Merge branch 'master' into grid-column-formatting

eslint-local-rules/index.js

@@ -41,12 +41,11 @@ module.exports = {
           if (
             /^@budibase\/[^/]+\/.*$/.test(importPath) &&
             importPath !== "@budibase/backend-core/tests" &&
-            importPath !== "@budibase/string-templates/test/utils" &&
-            importPath !== "@budibase/client/manifest.json"
+            importPath !== "@budibase/string-templates/test/utils"
           ) {
             context.report({
               node,
-              message: `Importing from @budibase is not allowed, except for @budibase/backend-core/tests, @budibase/string-templates/test/utils and @budibase/client/manifest.json.`,
+              message: `Importing from @budibase is not allowed, except for @budibase/backend-core/tests and @budibase/string-templates/test/utils.`,
             })
           }
         },

lerna.json

@@ -1,6 +1,6 @@
 {
   "$schema": "node_modules/lerna/schemas/lerna-schema.json",
-  "version": "3.3.5",
+  "version": "3.3.6",
   "npmClient": "yarn",
   "concurrency": 20,
   "command": {

packages/builder/src/helpers/duplicate.ts

@@ -76,13 +76,15 @@ export const getSequentialName = <T extends any>(
   {
     getName,
     numberFirstItem,
+    separator = "",
   }: {
     getName?: (item: T) => string
     numberFirstItem?: boolean
+    separator?: string
   } = {}
 ) => {
   if (!prefix?.length) {
-    return null
+    return ""
   }
   const trimmedPrefix = prefix.trim()
   const firstName = numberFirstItem ? `${prefix}1` : trimmedPrefix
@@ -107,5 +109,5 @@ export const getSequentialName = <T extends any>(
       max = num
     }
   })
-  return max === 0 ? firstName : `${prefix}${max + 1}`
+  return max === 0 ? firstName : `${prefix}${separator}${max + 1}`
 }

packages/builder/src/helpers/screen.ts

@@ -1,46 +0,0 @@
-import { Component, Screen, ScreenProps } from "@budibase/types"
-import clientManifest from "@budibase/client/manifest.json"
-
-export function findComponentsBySettingsType(
-  screen: Screen,
-  type: string | string[]
-) {
-  const typesArray = Array.isArray(type) ? type : [type]
-
-  const result: {
-    component: Component
-    setting: {
-      type: string
-      key: string
-    }
-  }[] = []
-  function recurseFieldComponentsInChildren(component: ScreenProps) {
-    if (!component) {
-      return
-    }
-
-    const definition = getManifestDefinition(component)
-    const setting =
-      "settings" in definition &&
-      definition.settings.find((s: any) => typesArray.includes(s.type))
-    if (setting && "type" in setting) {
-      result.push({
-        component,
-        setting: { type: setting.type!, key: setting.key! },
-      })
-    }
-    component._children?.forEach(child => {
-      recurseFieldComponentsInChildren(child)
-    })
-  }
-
-  recurseFieldComponentsInChildren(screen?.props)
-  return result
-}
-
-function getManifestDefinition(component: Component) {
-  const componentType = component._component.split("/").slice(-1)[0]
-  const definition =
-    clientManifest[componentType as keyof typeof clientManifest]
-  return definition
-}

packages/builder/src/helpers/tests/duplicate.test.ts

@@ -49,7 +49,7 @@ describe("getSequentialName", () => {
 
   it("handles nullish prefix", async () => {
     const name = getSequentialName([], null)
-    expect(name).toBe(null)
+    expect(name).toBe("")
   })
 
   it("handles just the prefix", async () => {

packages/builder/src/stores/builder/components.ts

@@ -20,6 +20,7 @@ import {
   previewStore,
   tables,
   componentTreeNodesStore,
+  screenComponents,
 } from "@/stores/builder"
 import { buildFormSchema, getSchemaForDatasource } from "@/dataBinding"
 import {
@@ -37,6 +38,7 @@ import {
   Table,
 } from "@budibase/types"
 import { utils } from "@budibase/shared-core"
+import { getSequentialName } from "@/helpers/duplicate"
 
 interface Component extends ComponentType {
   _id: string
@@ -452,7 +454,7 @@ export class ComponentStore extends BudiStore<ComponentState> {
    * @returns
    */
   createInstance(
-    componentName: string,
+    componentType: string,
     presetProps: any,
     parent: any
   ): Component | null {
@@ -461,11 +463,20 @@ export class ComponentStore extends BudiStore<ComponentState> {
       throw "A valid screen must be selected"
     }
 
-    const definition = this.getDefinition(componentName)
+    const definition = this.getDefinition(componentType)
     if (!definition) {
       return null
     }
 
+    const componentName = getSequentialName(
+      get(screenComponents),
+      `New ${definition.friendlyName || definition.name}`,
+      {
+        getName: c => c._instanceName,
+        separator: " ",
+      }
+    )
+
     // Generate basic component structure
     let instance: Component = {
       _id: Helpers.uuid(),
@@ -475,7 +486,7 @@ export class ComponentStore extends BudiStore<ComponentState> {
         hover: {},
         active: {},
       },
-      _instanceName: `New ${definition.friendlyName || definition.name}`,
+      _instanceName: componentName,
       ...presetProps,
     }
 
@@ -500,7 +511,7 @@ export class ComponentStore extends BudiStore<ComponentState> {
     }
 
     // Add step name to form steps
-    if (componentName.endsWith("/formstep")) {
+    if (componentType.endsWith("/formstep")) {
       const parentForm = findClosestMatchingComponent(
         screen.props,
         get(selectedComponent)?._id,
@@ -529,14 +540,14 @@ export class ComponentStore extends BudiStore<ComponentState> {
    * @returns
    */
   async create(
-    componentName: string,
+    componentType: string,
     presetProps: any,
     parent: Component,
     index: number
   ) {
     const state = get(this.store)
     const componentInstance = this.createInstance(
-      componentName,
+      componentType,
       presetProps,
       parent
     )

packages/builder/src/stores/builder/index.js

@@ -16,7 +16,7 @@ import { userStore, userSelectedResourceMap, isOnlyUser } from "./users.js"
 import { deploymentStore } from "./deployments.js"
 import { contextMenuStore } from "./contextMenu.js"
 import { snippets } from "./snippets"
-import { screenComponentErrors } from "./screenComponent"
+import { screenComponents, screenComponentErrors } from "./screenComponent"
 
 // Backend
 import { tables } from "./tables"
@@ -68,6 +68,7 @@ export {
   snippets,
   rowActions,
   appPublished,
+  screenComponents,
   screenComponentErrors,
 }
 

packages/builder/src/stores/builder/screenComponent.ts

@@ -2,12 +2,18 @@ import { derived } from "svelte/store"
 import { tables } from "./tables"
 import { selectedScreen } from "./screens"
 import { viewsV2 } from "./viewsV2"
-import { findComponentsBySettingsType } from "@/helpers/screen"
-import { UIDatasourceType, Screen } from "@budibase/types"
+import {
+  UIDatasourceType,
+  Screen,
+  Component,
+  ScreenProps,
+} from "@budibase/types"
 import { queries } from "./queries"
 import { views } from "./views"
 import { bindings, featureFlag } from "@/helpers"
 import { getBindableProperties } from "@/dataBinding"
+import { componentStore, ComponentDefinition } from "./components"
+import { findAllComponents } from "@/helpers/components"
 
 function reduceBy<TItem extends {}, TKey extends keyof TItem>(
   key: TKey,
@@ -38,12 +44,16 @@ const validationKeyByType: Record<UIDatasourceType, string | null> = {
 }
 
 export const screenComponentErrors = derived(
-  [selectedScreen, tables, views, viewsV2, queries],
-  ([$selectedScreen, $tables, $views, $viewsV2, $queries]): Record<
-    string,
-    string[]
-  > => {
-    if (!featureFlag.isEnabled("CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS")) {
+  [selectedScreen, tables, views, viewsV2, queries, componentStore],
+  ([
+    $selectedScreen,
+    $tables,
+    $views,
+    $viewsV2,
+    $queries,
+    $componentStore,
+  ]): Record<string, string[]> => {
+    if (!featureFlag.isEnabled("CHECK_COMPONENT_SETTINGS_ERRORS")) {
       return {}
     }
     function getInvalidDatasources(
@@ -51,9 +61,11 @@ export const screenComponentErrors = derived(
       datasources: Record<string, any>
     ) {
       const result: Record<string, string[]> = {}
+
       for (const { component, setting } of findComponentsBySettingsType(
         screen,
-        ["table", "dataSource"]
+        ["table", "dataSource"],
+        $componentStore.components
       )) {
         const componentSettings = component[setting.key]
         if (!componentSettings) {
@@ -111,3 +123,53 @@ export const screenComponentErrors = derived(
     return getInvalidDatasources($selectedScreen, datasources)
   }
 )
+
+function findComponentsBySettingsType(
+  screen: Screen,
+  type: string | string[],
+  definitions: Record<string, ComponentDefinition>
+) {
+  const typesArray = Array.isArray(type) ? type : [type]
+
+  const result: {
+    component: Component
+    setting: {
+      type: string
+      key: string
+    }
+  }[] = []
+
+  function recurseFieldComponentsInChildren(component: ScreenProps) {
+    if (!component) {
+      return
+    }
+
+    const definition = definitions[component._component]
+
+    const setting = definition?.settings?.find((s: any) =>
+      typesArray.includes(s.type)
+    )
+    if (setting && "type" in setting) {
+      result.push({
+        component,
+        setting: { type: setting.type!, key: setting.key! },
+      })
+    }
+    component._children?.forEach(child => {
+      recurseFieldComponentsInChildren(child)
+    })
+  }
+
+  recurseFieldComponentsInChildren(screen?.props)
+  return result
+}
+
+export const screenComponents = derived(
+  [selectedScreen],
+  ([$selectedScreen]) => {
+    if (!$selectedScreen) {
+      return []
+    }
+    return findAllComponents($selectedScreen.props) as Component[]
+  }
+)

packages/server/src/jsRunner/tests/isolatedVM.spec.ts

@@ -107,4 +107,15 @@ describe("Test isolated vm directly", () => {
     )
     expect(result).toEqual([])
   })
+
+  it("should ensure error results are cleared between runs", () => {
+    const context = {}
+    // throw error
+    // Ensure the first execution throws an error
+    expect(() => runJSWithIsolatedVM(`test.foo.bar = 123`, context)).toThrow()
+
+    // Ensure the error is not persisted across VMs
+    const secondResult = runJSWithIsolatedVM(`return {}`, context)
+    expect(secondResult).toEqual({})
+  })
 })

packages/server/src/jsRunner/vm/isolated-vm.ts

@@ -186,6 +186,7 @@ export class IsolatedVM implements VM {
 
     code = `
       try {
+        results = {}
         results['${this.runResultKey}']=${this.codeWrapper(code)}
       } catch (e) {
         results['${this.runErrorKey}']=e

packages/server/src/utilities/csv.ts

@@ -1,22 +1,52 @@
 import csv from "csvtojson"
 
 export async function jsonFromCsvString(csvString: string) {
-  const castedWithEmptyValues = await csv({ ignoreEmpty: true }).fromString(
-    csvString
-  )
+  const possibleDelimiters = [",", ";", ":", "|", "~", "\t", " "]
 
-  // By default the csvtojson library casts empty values as empty strings. This
-  // is causing issues on conversion.  ignoreEmpty will remove the key completly
-  // if empty, so creating this empty object will ensure we return the values
-  // with the keys but empty values
-  const result = await csv({ ignoreEmpty: false }).fromString(csvString)
-  result.forEach((r, i) => {
-    for (const [key] of Object.entries(r).filter(([, value]) => value === "")) {
-      if (castedWithEmptyValues[i][key] === undefined) {
-        r[key] = null
+  for (let i = 0; i < possibleDelimiters.length; i++) {
+    let headers: string[] | undefined = undefined
+    let headerMismatch = false
+
+    try {
+      // By default the csvtojson library casts empty values as empty strings. This
+      // is causing issues on conversion.  ignoreEmpty will remove the key completly
+      // if empty, so creating this empty object will ensure we return the values
+      // with the keys but empty values
+      const result = await csv({
+        ignoreEmpty: false,
+        delimiter: possibleDelimiters[i],
+      }).fromString(csvString)
+      for (const [, row] of result.entries()) {
+        // The purpose of this is to find rows that have been split
+        // into the wrong number of columns - Any valid .CSV file will have
+        // the same number of colums in each row
+        // If the number of columms in each row is different to
+        // the number of headers, this isn't the right delimiter
+        const columns = Object.keys(row)
+        if (headers == null) {
+          headers = columns
+        }
+        if (headers.length === 1 || headers.length !== columns.length) {
+          headerMismatch = true
+          break
+        }
+
+        for (const header of headers) {
+          if (row[header] === undefined || row[header] === "") {
+            row[header] = null
+          }
+        }
       }
+      if (headerMismatch) {
+        continue
+      } else {
+        return result
+      }
+    } catch (err) {
+      // Splitting on the wrong delimiter sometimes throws CSV parsing error
+      // (eg unterminated strings), which tells us we've picked the wrong delimiter
+      continue
     }
-  })
-
-  return result
+  }
+  throw new Error("Unable to determine delimiter")
 }

packages/server/src/utilities/tests/csv.spec.ts

@@ -29,5 +29,34 @@ describe("csv", () => {
         expect(Object.keys(r)).toEqual(["id", "optional", "title"])
       )
     })
+
+    const possibleDelimeters = [",", ";", ":", "|", "~", "\t", " "]
+
+    const csvArray = [
+      ["id", "title"],
+      ["1", "aaa"],
+      ["2", "bbb"],
+      ["3", "c ccc"],
+      ["", ""],
+      [":5", "eee5:e"],
+    ]
+
+    test.each(possibleDelimeters)(
+      "Should parse with delimiter %s",
+      async delimiter => {
+        const csvString = csvArray
+          .map(row => row.map(col => `"${col}"`).join(delimiter))
+          .join("\n")
+        const result = await jsonFromCsvString(csvString)
+
+        expect(result).toEqual([
+          { id: "1", title: "aaa" },
+          { id: "2", title: "bbb" },
+          { id: "3", title: "c ccc" },
+          { id: null, title: null },
+          { id: ":5", title: "eee5:e" },
+        ])
+      }
+    )
   })
 })

packages/types/src/sdk/featureFlag.ts

@@ -1,6 +1,6 @@
 export enum FeatureFlag {
   USE_ZOD_VALIDATOR = "USE_ZOD_VALIDATOR",
-  CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS = "CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS",
+  CHECK_COMPONENT_SETTINGS_ERRORS = "CHECK_COMPONENT_SETTINGS_ERRORS",
 
   // Account-portal
   DIRECT_LOGIN_TO_ACCOUNT_PORTAL = "DIRECT_LOGIN_TO_ACCOUNT_PORTAL",
@@ -8,7 +8,7 @@ export enum FeatureFlag {
 
 export const FeatureFlagDefaults = {
   [FeatureFlag.USE_ZOD_VALIDATOR]: false,
-  [FeatureFlag.CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS]: false,
+  [FeatureFlag.CHECK_COMPONENT_SETTINGS_ERRORS]: false,
 
   // Account-portal
   [FeatureFlag.DIRECT_LOGIN_TO_ACCOUNT_PORTAL]: false,