Adding error checking to our handlebars syntax inputs as well as making all handlebars helpers available due to space pre-processor being removed.

5 years ago · 9acd30a4cb
10 changed files with 130 additions and 54 deletions
--- 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}`,
    }
  })
--- 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",
--- 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)
+  }
 </script>

 <Popover {anchor} {align} bind:this={popover}>
@ -70,11 +77,16 @@
        bind:getCaretPosition
        bind:value
        placeholder="Add options from the left, type text, or do both" />
+      {#if !validity}
+        <p class="syntax-error">Current Handlebars syntax is invalid, please check the guide
+          <a href="https://handlebarsjs.com/guide/">here</a> for more details.
+        </p>
+      {/if}
      <div class="controls">
        <a href="https://docs.budibase.com/design/binding">
          <Body small>Learn more about binding</Body>
        </a>
-        <Button on:click={popover.hide} primary>Done</Button>
+        <Button on:click={popover.hide} disabled={!validity} primary>Done</Button>
      </div>
    </div>
  </div>
@ -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;
+  }
 </style>
--- 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;
--- 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}
+        <p class="syntax-error">Current Handlebars syntax is invalid, please check the guide
+          <a href="https://handlebarsjs.com/guide/">here</a> for more details.
+        </p>
+      {/if}
    </div>
  </div>
 </div>
@ -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;
+  }
 </style>
--- 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.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.errorEvents = handler => {
-  hbsInstance.registerHelper("helperMissing", handler)
+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
+  }
 }
--- 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 => {
--- 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
--- 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]")
+  })
 })
--- 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")
+  })
 })