From 7c514df39ddcc80ba1f99d3d26c377cab7c479cd Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Mon, 31 Jan 2022 18:54:04 +0000
Subject: [PATCH] Cache the determination of client component settings and
 simplify initialisationlogic

---
 .../client/src/components/Component.svelte    | 109 ++++++++++++------
 packages/client/src/utils/buttonActions.js    |   2 +
 packages/client/src/utils/componentProps.js   |  67 +++++++----
 3 files changed, 117 insertions(+), 61 deletions(-)

diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte
index 71e08f121..9ebcb2afd 100644
--- a/packages/client/src/components/Component.svelte
+++ b/packages/client/src/components/Component.svelte
@@ -1,10 +1,15 @@
 <script context="module">
+  // Cache the definition of settings for each component type
   let SettingsDefinitionCache = {}
+
+  // Cache the settings of each component ID.
+  // This speeds up remounting as well as repeaters.
+  let InstanceSettingsCache = {}
 </script>
 
 <script>
   import { getContext, setContext } from "svelte"
-  import { writable } from "svelte/store"
+  import { writable, get } from "svelte/store"
   import * as AppComponents from "components/app"
   import Router from "./Router.svelte"
   import { enrichProps, propsAreSame } from "utils/componentProps"
@@ -37,8 +42,6 @@
   let staticSettings
   let nestedSettings
 
-  // The context keys that
-
   // The enriched component settings
   let enrichedSettings
 
@@ -70,10 +73,12 @@
   const componentStore = writable({})
   setContext("component", componentStore)
 
+  //
+  let constructor
+  let definition
+  $: initialise(instance)
+
   // Extract component instance info
-  $: constructor = getComponentConstructor(instance._component)
-  $: definition = getComponentDefinition(instance._component)
-  $: settingsDefinition = getSettingsDefinition(definition)
   $: children = instance._children || []
   $: id = instance._id
   $: name = instance._instanceName
@@ -107,13 +112,6 @@
   $: empty = interactive && !children.length && hasChildren
   $: emptyState = empty && showEmptyState
 
-  // Raw settings are all settings excluding internal props and children
-  $: rawSettings = getRawSettings(instance)
-  $: instanceKey = Helpers.hashString(JSON.stringify(rawSettings))
-
-  // Parse and split component settings into categories
-  $: updateSettings(rawSettings, instanceKey, settingsDefinition)
-
   // Enrich component settings
   $: enrichComponentSettings(dynamicSettings, $context)
 
@@ -147,15 +145,48 @@
     editing,
   })
 
-  // Extracts all settings from the component instance
-  const getRawSettings = instance => {
-    let validSettings = {}
-    Object.entries(instance)
-      .filter(([name]) => name === "_conditions" || !name.startsWith("_"))
-      .forEach(([key, value]) => {
-        validSettings[key] = value
-      })
-    return validSettings
+  const initialise = instance => {
+    if (instance == null) {
+      return
+    }
+
+    // Ensure we're processing a new instance
+    const instanceKey = Helpers.hashString(JSON.stringify(instance))
+    if (instanceKey === lastInstanceKey) {
+      return
+    } else {
+      lastInstanceKey = instanceKey
+    }
+
+    // Pull definition and constructor
+    constructor = getComponentConstructor(instance._component)
+    definition = getComponentDefinition(instance._component)
+    if (!definition) {
+      return
+    }
+
+    // Get the settings definition for this component, and cache it
+    let settingsDefinition
+    if (SettingsDefinitionCache[definition.name]) {
+      settingsDefinition = SettingsDefinitionCache[definition.name]
+    } else {
+      settingsDefinition = getSettingsDefinition(definition)
+      SettingsDefinitionCache[definition.name] = settingsDefinition
+    }
+
+    // Parse the instance settings, and cache them
+    let instanceSettings
+    if (InstanceSettingsCache[instanceKey]) {
+      instanceSettings = InstanceSettingsCache[instanceKey]
+    } else {
+      instanceSettings = getInstanceSettings(instance, settingsDefinition)
+      InstanceSettingsCache[instanceKey] = instanceSettings
+    }
+
+    // Update the settings type in the component
+    staticSettings = instanceSettings.staticSettings
+    dynamicSettings = instanceSettings.dynamicSettings
+    nestedSettings = instanceSettings.nestedSettings
   }
 
   // Gets the component constructor for the specified component
@@ -180,9 +211,6 @@
     if (!definition) {
       return []
     }
-    if (SettingsDefinitionCache[definition.name]) {
-      return SettingsDefinitionCache[definition.name]
-    }
     let settings = []
     definition.settings?.forEach(setting => {
       if (setting.section) {
@@ -191,18 +219,17 @@
         settings.push(setting)
       }
     })
-    SettingsDefinitionCache[definition] = settings
     return settings
   }
 
-  // Updates and enriches component settings when raw settings change
-  const updateSettings = (settings, key, settingsDefinition) => {
-    const instanceChanged = key !== lastInstanceKey
-    if (!instanceChanged) {
-      return
-    } else {
-      lastInstanceKey = key
-    }
+  const getInstanceSettings = (instance, settingsDefinition) => {
+    // Get raw settings
+    let settings = {}
+    Object.entries(instance)
+      .filter(([name]) => name === "_conditions" || !name.startsWith("_"))
+      .forEach(([key, value]) => {
+        settings[key] = value
+      })
 
     // Derive static, dynamic and nested settings if the instance changed
     let newStaticSettings = { ...settings }
@@ -217,11 +244,15 @@
 
         // This is a non-nested setting, but we need to find out if it is
         // static or dynamic
-        const value = rawSettings[setting.key]
+        const value = settings[setting.key]
         if (value == null) {
           delete newDynamicSettings[setting.key]
         } else if (typeof value === "string" && value.includes("{{")) {
           delete newStaticSettings[setting.key]
+        } else if (value[0]?.["##eventHandlerType"] != null) {
+          console.log(value)
+          // Always treat button actions as dynamic
+          delete newStaticSettings[setting.key]
         } else if (typeof value === "object") {
           const stringified = JSON.stringify(value)
           if (stringified.includes("{{")) {
@@ -235,9 +266,11 @@
       }
     })
 
-    staticSettings = newStaticSettings
-    dynamicSettings = newDynamicSettings
-    nestedSettings = newNestedSettings
+    return {
+      staticSettings: newStaticSettings,
+      dynamicSettings: newDynamicSettings,
+      nestedSettings: newNestedSettings,
+    }
   }
 
   // Enriches any string component props using handlebars
diff --git a/packages/client/src/utils/buttonActions.js b/packages/client/src/utils/buttonActions.js
index 72c8f9c08..f2f455179 100644
--- a/packages/client/src/utils/buttonActions.js
+++ b/packages/client/src/utils/buttonActions.js
@@ -283,6 +283,7 @@ export const enrichButtonActions = (actions, context) => {
   const handlers = actions.map(def => handlerMap[def["##eventHandlerType"]])
   return async () => {
     for (let i = 0; i < handlers.length; i++) {
+      let start = Date.now()
       try {
         // Skip any non-existent action definitions
         if (!handlers[i]) {
@@ -344,6 +345,7 @@ export const enrichButtonActions = (actions, context) => {
         // Stop executing further actions on error
         return
       }
+      console.log("action", i, "took", Date.now() - start, "ms")
     }
   }
 }
diff --git a/packages/client/src/utils/componentProps.js b/packages/client/src/utils/componentProps.js
index b6b2cf7a9..0727b5943 100644
--- a/packages/client/src/utils/componentProps.js
+++ b/packages/client/src/utils/componentProps.js
@@ -1,5 +1,6 @@
 import { enrichDataBindings } from "./enrichDataBinding"
 import { enrichButtonActions } from "./buttonActions"
+import { decodeJSBinding } from "@budibase/string-templates"
 
 /**
  * Deeply compares 2 props using JSON.stringify.
@@ -43,9 +44,9 @@ export const enrichProps = (props, context) => {
     }
   })
 
-  // Handle conditional UI separately after normal settings
-  let conditions = normalProps._conditions
-  delete normalProps._conditions
+  // Store the original conditions so that we can restore parts of them after
+  // enrichment
+  let rawConditions = normalProps._conditions
 
   // Enrich all props except button actions
   let enrichedProps = enrichDataBindings(normalProps, totalContext)
@@ -58,31 +59,51 @@ export const enrichProps = (props, context) => {
   })
 
   // Conditions
-  if (conditions?.length) {
-    let enrichedConditions = []
-    conditions.forEach(condition => {
+  if (enrichedProps._conditions?.length) {
+    enrichedProps._conditions.forEach((condition, idx) => {
       if (condition.setting?.toLowerCase().includes("onclick")) {
-        // Copy and remove the setting value from the condition as it needs
-        // enriched separately
-        let toEnrich = { ...condition }
-        delete toEnrich.settingValue
+        // Use the original condition action value to enrich it to a button
+        // action
+        condition.settingValue = enrichButtonActions(
+          rawConditions[idx].settingValue,
+          totalContext
+        )
 
-        // Join the condition back together
-        enrichedConditions.push({
-          ...enrichDataBindings(toEnrich, totalContext),
-          settingValue: enrichButtonActions(
-            condition.settingValue,
-            totalContext
-          ),
-          rand: Math.random(),
-        })
-      } else {
-        // Normal condition
-        enrichedConditions.push(enrichDataBindings(condition, totalContext))
+        // Since we can't compare functions, we need to assume that conditions
+        // change after every enrichment
+        condition.rand = Math.random()
       }
     })
-    enrichedProps._conditions = enrichedConditions
   }
 
   return enrichedProps
 }
+
+/**
+ * Checks if a props object references a particular context binding.
+ * e.g. if props are { foo: "My name is {{ person.name }}" }, and we search for" +
+ * "person", then this function wil return true - the props do a context key
+ * called "person".
+ * @param props the props object to search
+ * @param bindingKey the key to search for
+ */
+export const propsUseBinding = (props, bindingKey) => {
+  if (!Object.keys(props || {}).length) {
+    return false
+  }
+  const string = JSON.stringify(props)
+  const usedInHBS = string.includes(`[${bindingKey}]`)
+  if (usedInHBS) {
+    return true
+  }
+  const jsBindingRegex = new RegExp("{{ js [^}]+ }}", "g")
+  const jsBindings = [...string.matchAll(jsBindingRegex)]
+  for (let jsBinding of jsBindings) {
+    const encoded = jsBinding[0]
+    const js = decodeJSBinding(encoded)
+    if (js?.includes(`$("[${bindingKey}]`)) {
+      return true
+    }
+  }
+  return false
+}