From 9acd30a4cb05a74752a4515ac52df28a4eec3d74 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Jan 2021 17:57:38 +0000 Subject: [PATCH] Adding error checking to our handlebars syntax inputs as well as making all handlebars helpers available due to space pre-processor being removed. --- .../builder/src/builderStore/dataBinding.js | 5 ++- .../store/screenTemplates/rowDetailScreen.js | 7 ++-- .../SetupPanel/GenericBindingPopover.svelte | 24 +++++++++++- .../components/common/BindableInput.svelte | 2 +- .../PropertiesPanel/BindingPanel.svelte | 33 ++++++++++++++++ packages/string-templates/src/index.js | 28 ++++++++++--- .../string-templates/src/processors/index.js | 10 ++++- .../src/processors/preprocessor.js | 39 ++----------------- packages/string-templates/test/basic.spec.js | 19 +++++++++ .../string-templates/test/escapes.spec.js | 17 ++++++-- 10 files changed, 130 insertions(+), 54 deletions(-) diff --git a/packages/builder/src/builderStore/dataBinding.js b/packages/builder/src/builderStore/dataBinding.js index 004c7170d3..48c09d0b1a 100644 --- a/packages/builder/src/builderStore/dataBinding.js +++ b/packages/builder/src/builderStore/dataBinding.js @@ -2,6 +2,7 @@ import { cloneDeep } from "lodash/fp" import { get } from "svelte/store" import { backendUiStore, store } from "builderStore" import { findAllMatchingComponents, findComponentPath } from "./storeUtils" +import { makePropSafe } from "@budibase/string-templates" // Regex to match all instances of template strings const CAPTURE_VAR_INSIDE_TEMPLATE = /{{([^}]+)}}/g @@ -105,7 +106,7 @@ export const getContextBindings = (rootComponent, componentId) => { contextBindings.push({ type: "context", - runtimeBinding: `${component._id}.${runtimeBoundKey}`, + runtimeBinding: `${makePropSafe(component._id)}.${makePropSafe(runtimeBoundKey)}`, readableBinding: `${component._instanceName}.${table.name}.${key}`, fieldSchema, providerId: component._id, @@ -135,7 +136,7 @@ export const getComponentBindings = rootComponent => { return { type: "instance", providerId: component._id, - runtimeBinding: `${component._id}`, + runtimeBinding: `${makePropSafe(component._id)}`, readableBinding: `${component._instanceName}`, } }) diff --git a/packages/builder/src/builderStore/store/screenTemplates/rowDetailScreen.js b/packages/builder/src/builderStore/store/screenTemplates/rowDetailScreen.js index 0a9148eaf8..fd54405875 100644 --- a/packages/builder/src/builderStore/store/screenTemplates/rowDetailScreen.js +++ b/packages/builder/src/builderStore/store/screenTemplates/rowDetailScreen.js @@ -2,6 +2,7 @@ import sanitizeUrl from "./utils/sanitizeUrl" import { rowListUrl } from "./rowListScreen" import { Screen } from "./utils/Screen" import { Component } from "./utils/Component" +import { makePropSafe } from "@budibase/string-templates" import { makeMainContainer, makeBreadcrumbContainer, @@ -12,7 +13,7 @@ import { export default function(tables) { return tables.map(table => { const heading = table.primaryDisplay - ? `{{ data.${table.primaryDisplay} }}` + ? `{{ data.${makePropSafe(table.primaryDisplay)} }}` : null return { name: `${table.name} - Detail`, @@ -60,8 +61,8 @@ function generateTitleContainer(table, title, providerId) { onClick: [ { parameters: { - rowId: `{{ ${providerId}._id }}`, - revId: `{{ ${providerId}._rev }}`, + rowId: `{{ ${makePropSafe(providerId)}._id }}`, + revId: `{{ ${makePropSafe(providerId)}._rev }}`, tableId: table._id, }, "##eventHandlerType": "Delete Row", diff --git a/packages/builder/src/components/automation/SetupPanel/GenericBindingPopover.svelte b/packages/builder/src/components/automation/SetupPanel/GenericBindingPopover.svelte index a326f60a08..a48748f7d5 100644 --- a/packages/builder/src/components/automation/SetupPanel/GenericBindingPopover.svelte +++ b/packages/builder/src/components/automation/SetupPanel/GenericBindingPopover.svelte @@ -10,6 +10,7 @@ Popover, } from "@budibase/bbui" import { createEventDispatcher } from "svelte" + import { isValid } from "@budibase/string-templates" const dispatch = createEventDispatcher() export let value = "" @@ -19,8 +20,10 @@ export let popover = null let getCaretPosition + let validity = true $: categories = Object.entries(groupBy("category", bindings)) + $: value && checkValid() function onClickBinding(binding) { const position = getCaretPosition() @@ -34,6 +37,10 @@ value += toAdd } } + + function checkValid() { + validity = isValid(value) + } @@ -70,11 +77,16 @@ bind:getCaretPosition bind:value placeholder="Add options from the left, type text, or do both" /> + {#if !validity} +

Current Handlebars syntax is invalid, please check the guide + here for more details. +

+ {/if}
Learn more about binding - +
@@ -152,4 +164,14 @@ align-items: center; margin-top: var(--spacing-m); } + + .syntax-error { + color: var(--red); + font-size: 12px; + } + + .syntax-error a { + color: var(--red); + text-decoration: underline; + } diff --git a/packages/builder/src/components/common/BindableInput.svelte b/packages/builder/src/components/common/BindableInput.svelte index 0e2cc2174f..7b8831902c 100644 --- a/packages/builder/src/components/common/BindableInput.svelte +++ b/packages/builder/src/components/common/BindableInput.svelte @@ -41,7 +41,7 @@ .icon { right: 2px; - top: 2px; + top: 26px; bottom: 2px; position: absolute; align-items: center; diff --git a/packages/builder/src/components/design/PropertiesPanel/BindingPanel.svelte b/packages/builder/src/components/design/PropertiesPanel/BindingPanel.svelte index 612dc9caa5..f064449657 100644 --- a/packages/builder/src/components/design/PropertiesPanel/BindingPanel.svelte +++ b/packages/builder/src/components/design/PropertiesPanel/BindingPanel.svelte @@ -2,12 +2,29 @@ import groupBy from "lodash/fp/groupBy" import { Button, TextArea, Drawer, Heading, Spacer } from "@budibase/bbui" import { createEventDispatcher } from "svelte" + import { isValid } from "@budibase/string-templates" + import {getBindableProperties, readableToRuntimeBinding} from "builderStore/dataBinding" + import { currentAsset, store } from "../../../builderStore" const dispatch = createEventDispatcher() export let bindableProperties export let value = "" export let bindingDrawer + let validity = true + + $: value && checkValid() + $: bindableProperties = getBindableProperties( + $currentAsset.props, + $store.selectedComponentId + ) + + function checkValid() { + // TODO: need to convert the value to the runtime binding + const runtimeBinding = readableToRuntimeBinding(bindableProperties, value) + validity = isValid(runtimeBinding) + } + function addToText(readableBinding) { value = `${value || ""}{{ ${readableBinding} }}` } @@ -55,6 +72,11 @@ bind:value placeholder="Add text, or click the objects on the left to add them to the textbox." /> + {#if !validity} +

Current Handlebars syntax is invalid, please check the guide + here for more details. +

+ {/if} @@ -108,4 +130,15 @@ height: 40vh; overflow-y: auto; } + + .syntax-error { + padding-top: var(--spacing-m); + color: var(--red); + font-size: 12px; + } + + .syntax-error a { + color: var(--red); + text-decoration: underline; + } diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 2636752488..0bb3abf806 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -87,6 +87,7 @@ module.exports.processStringSync = (string, context) => { if (typeof string !== "string") { throw "Cannot process non-string types." } + let template string = processors.preprocess(string) // this does not throw an error when template can't be fulfilled, have to try correct beforehand @@ -95,11 +96,26 @@ module.exports.processStringSync = (string, context) => { } /** - * Errors can occur if a user of this library attempts to use a helper that has not been added to the system, these errors - * can be captured to alert the user of the mistake. - * @param {function} handler a function which will be called every time an error occurs when processing a handlebars - * statement. + * Simple utility function which makes sure that a templating property has been wrapped in literal specifiers correctly. + * @param {string} property The property which is to be wrapped. + * @returns {string} The wrapped property ready to be added to a templating string. */ -module.exports.errorEvents = handler => { - hbsInstance.registerHelper("helperMissing", handler) +module.exports.makePropSafe = property => { + return `[${property}]`.replace("[[", "[").replace("]]", "]") +} + +/** + * Checks whether or not a template string contains totally valid syntax (simply tries running it) + * @param string The string to test for valid syntax - this may contain no templates and will be considered valid. + * @returns {boolean} Whether or not the input string is valid. + */ +module.exports.isValid = string => { + // don't really need a real context to check if its valid + const context = {} + try { + hbsInstance.compile(processors.preprocess(string, false))(context) + return true + } catch (err) { + return false + } } diff --git a/packages/string-templates/src/processors/index.js b/packages/string-templates/src/processors/index.js index 225a707c60..a867b003db 100644 --- a/packages/string-templates/src/processors/index.js +++ b/packages/string-templates/src/processors/index.js @@ -17,8 +17,14 @@ function process(string, processors) { return string } -module.exports.preprocess = string => { - return process(string, preprocessor.processors) +module.exports.preprocess = (string, finalise = true) => { + let processors = preprocessor.processors + // the pre-processor finalisation stops handlebars from ever throwing an error + // might want to pre-process for other benefits but still want to see errors + if (!finalise) { + processors = processors.filter(processor => processor.name !== preprocessor.PreprocessorNames.FINALISE) + } + return process(string, processors) } module.exports.postprocess = string => { diff --git a/packages/string-templates/src/processors/preprocessor.js b/packages/string-templates/src/processors/preprocessor.js index c984d61298..a7c5891f58 100644 --- a/packages/string-templates/src/processors/preprocessor.js +++ b/packages/string-templates/src/processors/preprocessor.js @@ -37,42 +37,7 @@ module.exports.processors = [ return statement }), - new Preprocessor(PreprocessorNames.HANDLE_SPACES, statement => { - // exclude helpers and brackets, regex will only find double brackets - const exclusions = HelperNames() - // find all the parts split by spaces - const splitBySpaces = statement - .split(" ") - .filter(el => el !== "{{" && el !== "}}") - // remove braces if they are found and weren't spaced out - splitBySpaces[0] = splitBySpaces[0].replace("{", "") - splitBySpaces[splitBySpaces.length - 1] = splitBySpaces[ - splitBySpaces.length - 1 - ].replace("}", "") - // remove the excluded elements - const propertyParts = splitBySpaces.filter( - part => exclusions.indexOf(part) === -1 - ) - // rebuild to get the full property - const fullProperty = propertyParts.join(" ") - // now work out the dot notation layers and split them up - const propertyLayers = fullProperty.split(".") - // find the layers which need to be wrapped and wrap them - for (let layer of propertyLayers) { - if (layer.indexOf(" ") !== -1) { - statement = swapStrings( - statement, - statement.indexOf(layer), - layer.length, - `[${layer}]` - ) - } - } - // remove the edge case of double brackets being entered (in-case user already has specified) - return statement.replace(/\[\[/g, "[").replace(/]]/g, "]") - }), - - new Preprocessor(Preprocessor.FINALISE, statement => { + new Preprocessor(PreprocessorNames.FINALISE, statement => { let insideStatement = statement.slice(2, statement.length - 2) if (insideStatement.charAt(0) === " ") { insideStatement = insideStatement.slice(1) @@ -87,3 +52,5 @@ module.exports.processors = [ return `{{ all ${insideStatement} }}` }), ] + +module.exports.PreprocessorNames = PreprocessorNames diff --git a/packages/string-templates/test/basic.spec.js b/packages/string-templates/test/basic.spec.js index 55dc63a995..6fe57d67b3 100644 --- a/packages/string-templates/test/basic.spec.js +++ b/packages/string-templates/test/basic.spec.js @@ -1,6 +1,8 @@ const { processObject, processString, + isValid, + makePropSafe, } = require("../src/index") describe("Test that the string processing works correctly", () => { @@ -81,4 +83,21 @@ describe("Test that the object processing works correctly", () => { } expect(error).not.toBeNull() }) +}) + +describe("check the utility functions", () => { + it("should return false for an invalid template string", () => { + const valid = isValid("{{ table1.thing prop }}") + expect(valid).toBe(false) + }) + + it("should state this template is valid", () => { + const valid = isValid("{{ thing }}") + expect(valid).toBe(true) + }) + + it("should make a property safe", () => { + const property = makePropSafe("thing") + expect(property).toEqual("[thing]") + }) }) \ No newline at end of file diff --git a/packages/string-templates/test/escapes.spec.js b/packages/string-templates/test/escapes.spec.js index 39df8719d6..de03c08fe6 100644 --- a/packages/string-templates/test/escapes.spec.js +++ b/packages/string-templates/test/escapes.spec.js @@ -18,14 +18,14 @@ describe("Handling context properties with spaces in their name", () => { }) it("should be able to handle a property with a space in its name", async () => { - const output = await processString("hello my name is {{ person name }}", { + const output = await processString("hello my name is {{ [person name] }}", { "person name": "Mike", }) expect(output).toBe("hello my name is Mike") }) it("should be able to handle an object with layers that requires escaping", async () => { - const output = await processString("testcase {{ testing.test case }}", { + const output = await processString("testcase {{ testing.[test case] }}", { testing: { "test case": 1 } @@ -44,8 +44,19 @@ describe("attempt some complex problems", () => { }, }, } - const hbs = "{{ New Repeater.Get Actors.first_name }} {{ New Repeater.Get Actors.last_name }}" + const hbs = "{{ [New Repeater].[Get Actors].[first_name] }} {{ [New Repeater].[Get Actors].[last_name] }}" const output = await processString(hbs, context) expect(output).toBe("Bob Bobert") }) + + it("should be able to process an odd string produced by builder", async () => { + const context = { + "c306d140d7e854f388bae056db380a0eb": { + "test prop": "test", + } + } + const hbs = "null{{ [c306d140d7e854f388bae056db380a0eb].[test prop] }}" + const output = await processString(hbs, context) + expect(output).toBe("nulltest") + }) }) \ No newline at end of file