Optimise client library performance with skeleton loaders (#9256)

* Treat empty string default values as nullish when considering whether field default values should be applied to the real field value

* Add flag to allow not loading data immediately upon creation of a data fetch object

* Use loading context inside forms to avoid wasted renders while form schema are loading

* Wait for parent data providers to finish loading before loading their own data

* Optimise data provider to reduce updates as much as possible

* Allow forms to render content immediately again, but use the loading context to inform data providers to wait for them

* Remove fetchOnCreation flag for data fetches as now redundant

* Fix issue with deleting the selected button action when the next action has no parameters

* Lint

packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte

@@ -74,8 +74,18 @@
   }
 
   const deleteAction = index => {
+    // Check if we're deleting the selected action
+    const selectedIndex = actions.indexOf(selectedAction)
+    const isSelected = index === selectedIndex
+
+    // Delete the action
     actions.splice(index, 1)
     actions = actions
+
+    // Select a new action if we deleted the selected one
+    if (isSelected) {
+      selectedAction = actions?.length ? actions[0] : null
+    }
   }
 
   const toggleActionList = () => {

packages/client/src/components/Component.svelte

@@ -171,6 +171,15 @@
   $: pad = pad || (interactive && hasChildren && inDndPath)
   $: $dndIsDragging, (pad = false)
 
+  // Determine whether we should render a skeleton loader for this component
+  $: showSkeleton =
+    $loading &&
+    definition.name !== "Screenslot" &&
+    children.length === 0 &&
+    !instance._blockElementHasChildren &&
+    !definition.block &&
+    definition.skeleton !== false
+
   // Update component context
   $: store.set({
     id,
@@ -473,14 +482,6 @@
       componentStore.actions.unregisterInstance(id)
     }
   })
-
-  $: showSkeleton =
-    $loading &&
-    definition.name !== "Screenslot" &&
-    children.length === 0 &&
-    !instance._blockElementHasChildren &&
-    !definition.block &&
-    definition.skeleton !== false
 </script>
 
 {#if showSkeleton}

packages/client/src/components/app/DataProvider.svelte

@@ -11,20 +11,23 @@
   export let limit
   export let paginate
 
-  const loading = writable(false)
-
   const { styleable, Provider, ActionTypes, API } = getContext("sdk")
   const component = getContext("component")
 
+  // Update loading state
+  const parentLoading = getContext("loading")
+  const loading = writable(true)
+  setContext("loading", loading)
+
   // We need to manage our lucene query manually as we want to allow components
   // to extend it
   let queryExtensions = {}
   $: defaultQuery = LuceneUtils.buildLuceneQuery(filter)
   $: query = extendQuery(defaultQuery, queryExtensions)
 
-  // Keep our data fetch instance up to date
-  $: fetch = createFetch(dataSource)
-  $: fetch.update({
+  // Fetch data and refresh when needed
+  $: fetch = createFetch(dataSource, $parentLoading)
+  $: updateFetch({
     query,
     sortColumn,
     sortOrder,
@@ -32,6 +35,9 @@
     paginate,
   })
 
+  // Keep loading context updated
+  $: loading.set($parentLoading || !$fetch.loaded)
+
   // Build our action context
   $: actions = [
     {
@@ -80,14 +86,21 @@
       sortColumn: $fetch.sortColumn,
       sortOrder: $fetch.sortOrder,
     },
-    limit: limit,
+    limit,
   }
 
-  const parentLoading = getContext("loading")
-  setContext("loading", loading)
-  $: loading.set($parentLoading || !$fetch.loaded)
+  const createFetch = (datasource, parentLoading) => {
+    // Return a dummy fetch if parent is still loading. We do this so that we
+    // can still properly subscribe to a valid fetch object and check all
+    // properties, but we want to avoid fetching the real data until all parents
+    // have finished loading.
+    // This logic is only needed due to skeleton loaders, as previously we
+    // simply blocked component rendering until data was ready.
+    if (parentLoading) {
+      return fetchData({ API })
+    }
 
-  const createFetch = datasource => {
+    // Otherwise return the real thing
     return fetchData({
       API,
       datasource,
@@ -101,6 +114,14 @@
     })
   }
 
+  const updateFetch = opts => {
+    // Only update fetch if parents have stopped loading. Otherwise we will
+    // trigger a fetch of the real data before parents are ready.
+    if (!$parentLoading) {
+      fetch.update(opts)
+    }
+  }
+
   const addQueryExtension = (key, extension) => {
     if (!key || !extension) {
       return

packages/client/src/components/app/forms/Form.svelte

@@ -1,7 +1,8 @@
 <script>
-  import { getContext } from "svelte"
+  import { getContext, setContext } from "svelte"
   import InnerForm from "./InnerForm.svelte"
   import { Helpers } from "@budibase/bbui"
+  import { writable } from "svelte/store"
 
   export let dataSource
   export let theme
@@ -20,10 +21,17 @@
   const context = getContext("context")
   const { API, fetchDatasourceSchema } = getContext("sdk")
 
+  // Forms also use loading context as they require loading a schema
+  const parentLoading = getContext("loading")
+  const loading = writable(true)
+  setContext("loading", loading)
+
+  let loaded = false
   let schema
   let table
 
   $: fetchSchema(dataSource)
+  $: loading.set($parentLoading || !loaded)
 
   // Returns the closes data context which isn't a built in context
   const getInitialValues = (type, dataSource, context) => {
@@ -55,11 +63,14 @@
     }
     const res = await fetchDatasourceSchema(dataSource)
     schema = res || {}
+    if (!loaded) {
+      loaded = true
+    }
   }
 
   $: initialValues = getInitialValues(actionType, dataSource, $context)
   $: resetKey = Helpers.hashString(
-    !!schema +
+    loaded +
       JSON.stringify(initialValues) +
       JSON.stringify(dataSource) +
       disabled

packages/client/src/components/app/forms/InnerForm.svelte

@@ -128,21 +128,15 @@
     return fields.find(field => get(field).name === name)
   }
 
-  const getDefault = (defaultValue, schema, type) => {
-    // Remove any values not present in the field schema
-    // Convert any values supplied to string
-    if (Array.isArray(defaultValue) && type == "array" && schema) {
-      return defaultValue.reduce((acc, entry) => {
-        let processedOption = String(entry)
-        let schemaOptions = schema.constraints.inclusion
-        if (schemaOptions.indexOf(processedOption) > -1) {
-          acc.push(processedOption)
-        }
-        return acc
-      }, [])
-    } else {
-      return defaultValue
+  // Sanitises a value by ensuring it doesn't contain any invalid data
+  const sanitiseValue = (value, schema, type) => {
+    // Check arrays - remove any values not present in the field schema and
+    // convert any values supplied to strings
+    if (Array.isArray(value) && type === "array" && schema) {
+      const options = schema?.constraints.inclusion || []
+      return value.map(opt => String(opt)).filter(opt => options.includes(opt))
     }
+    return value
   }
 
   const formApi = {
@@ -160,7 +154,6 @@
 
       // Create validation function based on field schema
       const schemaConstraints = schema?.[field]?.constraints
-
       const validator = disableValidation
         ? null
         : createValidatorFromConstraints(
@@ -170,10 +163,11 @@
             table
           )
 
-      const parsedDefault = getDefault(defaultValue, schema?.[field], type)
+      // Sanitise the default value to ensure it doesn't contain invalid data
+      defaultValue = sanitiseValue(defaultValue, schema?.[field], type)
 
       // If we've already registered this field then keep some existing state
-      let initialValue = Helpers.deepGet(initialValues, field) ?? parsedDefault
+      let initialValue = Helpers.deepGet(initialValues, field) ?? defaultValue
       let initialError = null
       let fieldId = `id-${Helpers.uuid()}`
       const existingField = getField(field)
@@ -183,7 +177,9 @@
 
         // Determine the initial value for this field, reusing the current
         // value if one exists
-        initialValue = fieldState.value ?? initialValue
+        if (fieldState.value != null && fieldState.value !== "") {
+          initialValue = fieldState.value
+        }
 
         // If this field has already been registered and we previously had an
         // error set, then re-run the validator to see if we can unset it
@@ -206,11 +202,11 @@
           error: initialError,
           disabled:
             disabled || fieldDisabled || (isAutoColumn && !editAutoColumns),
-          defaultValue: parsedDefault,
+          defaultValue,
           validator,
           lastUpdate: Date.now(),
         },
-        fieldApi: makeFieldApi(field, parsedDefault),
+        fieldApi: makeFieldApi(field),
         fieldSchema: schema?.[field] ?? {},
       })
 
@@ -225,18 +221,9 @@
       return fieldInfo
     },
     validate: () => {
-      let valid = true
-      let validationFields = fields
-
-      validationFields = fields.filter(f => get(f).step === get(currentStep))
-
-      // Validate fields and check if any are invalid
-      validationFields.forEach(field => {
-        if (!get(field).fieldApi.validate()) {
-          valid = false
-        }
-      })
-      return valid
+      return fields
+        .filter(field => get(field).step === get(currentStep))
+        .every(field => get(field).fieldApi.validate())
     },
     reset: () => {
       // Reset the form by resetting each individual field