From 1e033c3618c688728c6b293df98f458a05a115ac Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 1 Feb 2024 14:45:16 +0000 Subject: [PATCH 1/4] Remove proxying of context changes up the chain --- .../src/components/context/Provider.svelte | 4 +- packages/client/src/stores/context.js | 53 +++++-------------- 2 files changed, 15 insertions(+), 42 deletions(-) diff --git a/packages/client/src/components/context/Provider.svelte b/packages/client/src/components/context/Provider.svelte index 1b6a073512..ad5b580c4f 100644 --- a/packages/client/src/components/context/Provider.svelte +++ b/packages/client/src/components/context/Provider.svelte @@ -33,7 +33,7 @@ const provideData = newData => { const dataKey = JSON.stringify(newData) if (dataKey !== lastDataKey) { - context.actions.provideData(providerKey, newData, scope) + context.actions.provideData(providerKey, newData) lastDataKey = dataKey } } @@ -43,7 +43,7 @@ if (actionsKey !== lastActionsKey) { lastActionsKey = actionsKey newActions?.forEach(({ type, callback, metadata }) => { - context.actions.provideAction(providerKey, type, callback, scope) + context.actions.provideAction(providerKey, type, callback) // Register any "refresh datasource" actions with a singleton store // so we can easily refresh data at all levels for any datasource diff --git a/packages/client/src/stores/context.js b/packages/client/src/stores/context.js index e54c773591..c1ec18ef13 100644 --- a/packages/client/src/stores/context.js +++ b/packages/client/src/stores/context.js @@ -1,5 +1,4 @@ import { writable, derived } from "svelte/store" -import { ContextScopes } from "constants" export const createContextStore = parentContext => { const context = writable({}) @@ -20,60 +19,34 @@ export const createContextStore = parentContext => { } // Provide some data in context - const provideData = (providerId, data, scope = ContextScopes.Global) => { + const provideData = (providerId, data) => { if (!providerId || data === undefined) { return } - // Proxy message up the chain if we have a parent and are providing global - // context - if (scope === ContextScopes.Global && parentContext) { - parentContext.actions.provideData(providerId, data, scope) - } - // Otherwise this is either the context root, or we're providing a local // context override, so we need to update the local context instead - else { - context.update(state => { - state[providerId] = data - return state - }) - broadcastChange(providerId) - } + context.update(state => { + state[providerId] = data + return state + }) + broadcastChange(providerId) } // Provides some action in context - const provideAction = ( - providerId, - actionType, - callback, - scope = ContextScopes.Global - ) => { + const provideAction = (providerId, actionType, callback) => { if (!providerId || !actionType) { return } - // Proxy message up the chain if we have a parent and are providing global - // context - if (scope === ContextScopes.Global && parentContext) { - parentContext.actions.provideAction( - providerId, - actionType, - callback, - scope - ) - } - // Otherwise this is either the context root, or we're providing a local // context override, so we need to update the local context instead - else { - const key = `${providerId}_${actionType}` - context.update(state => { - state[key] = callback - return state - }) - broadcastChange(key) - } + const key = `${providerId}_${actionType}` + context.update(state => { + state[key] = callback + return state + }) + broadcastChange(key) } const observeChanges = callback => { From d6bf33bce753081ba72c5312bf4dd9b6c800fd06 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Fri, 2 Feb 2024 14:59:45 +0000 Subject: [PATCH 2/4] Update data binding generation to match how context is provided by components, respecting branching due to local context --- .../src/builderStore/componentUtils.js | 61 +++++++++++++++++-- .../builder/src/builderStore/dataBinding.js | 43 +++++++++---- packages/client/manifest.json | 3 +- .../client/src/components/app/Repeater.svelte | 7 +-- .../src/components/context/Provider.svelte | 4 +- packages/client/src/constants.js | 5 -- packages/client/src/sdk.js | 6 +- packages/frontend-core/src/constants.js | 5 ++ 8 files changed, 102 insertions(+), 32 deletions(-) diff --git a/packages/builder/src/builderStore/componentUtils.js b/packages/builder/src/builderStore/componentUtils.js index 733ce0948e..8a7cd93d0f 100644 --- a/packages/builder/src/builderStore/componentUtils.js +++ b/packages/builder/src/builderStore/componentUtils.js @@ -7,6 +7,9 @@ import { findHBSBlocks, } from "@budibase/string-templates" import { capitalise } from "helpers" +import { Constants } from "@budibase/frontend-core" + +const { ContextScopes } = Constants /** * Recursively searches for a specific component ID @@ -263,11 +266,59 @@ export const getComponentName = component => { if (component == null) { return "" } - const components = get(store)?.components || {} const componentDefinition = components[component._component] || {} - const name = - componentDefinition.friendlyName || componentDefinition.name || "" - - return name + return componentDefinition.friendlyName || componentDefinition.name || "" +} + +/** + * Recurses through the component tree and builds a tree of contexts provided + * by components. + */ +export const buildContextTree = ( + rootComponent, + tree = { root: [] }, + currentBranch = "root" +) => { + // Sanity check + if (!rootComponent) { + return tree + } + + // Process this component's contexts + const def = store.actions.components.getDefinition(rootComponent._component) + if (def?.context) { + tree[currentBranch].push(rootComponent._id) + const contexts = Array.isArray(def.context) ? def.context : [def.context] + + // If we provide local context, start a new branch for our children + if (contexts.some(context => context.scope === ContextScopes.Local)) { + currentBranch = rootComponent._id + tree[rootComponent._id] = [] + } + } + + // Process children + if (rootComponent._children) { + rootComponent._children.forEach(child => { + buildContextTree(child, tree, currentBranch) + }) + } + + return tree +} + +/** + * Generates a lookup map of which content branch all components in a component + * tree are inside. + */ +export const buildContextTreeLookupMap = rootComponent => { + const tree = buildContextTree(rootComponent) + let map = {} + Object.entries(tree).forEach(([branch, ids]) => { + ids.forEach(id => { + map[id] = branch + }) + }) + return map } diff --git a/packages/builder/src/builderStore/dataBinding.js b/packages/builder/src/builderStore/dataBinding.js index 86aecd466f..951d615e7a 100644 --- a/packages/builder/src/builderStore/dataBinding.js +++ b/packages/builder/src/builderStore/dataBinding.js @@ -1,6 +1,8 @@ import { cloneDeep } from "lodash/fp" import { get } from "svelte/store" import { + buildContextTree, + buildContextTreeLookupMap, findAllComponents, findAllMatchingComponents, findComponent, @@ -20,11 +22,13 @@ import { encodeJSBinding, } from "@budibase/string-templates" import { TableNames } from "../constants" -import { JSONUtils } from "@budibase/frontend-core" +import { JSONUtils, Constants } from "@budibase/frontend-core" import ActionDefinitions from "components/design/settings/controls/ButtonActionEditor/manifest.json" import { environment, licensing } from "stores/portal" import { convertOldFieldFormat } from "components/design/settings/controls/FieldConfiguration/utils" +const { ContextScopes } = Constants + // Regex to match all instances of template strings const CAPTURE_VAR_INSIDE_TEMPLATE = /{{([^}]+)}}/g const CAPTURE_VAR_INSIDE_JS = /\$\("([^")]+)"\)/g @@ -214,20 +218,27 @@ export const getComponentContexts = ( return [] } let map = {} + const componentPath = findComponentPath(asset.props, componentId) + const componentPathIds = componentPath.map(component => component._id) + const contextTreeLookupMap = buildContextTreeLookupMap(asset.props) // Processes all contexts exposed by a component const processContexts = scope => component => { - const def = store.actions.components.getDefinition(component._component) + // Sanity check + const def = store.actions.components.getDefinition(component?._component) if (!def?.context) { return } - if (!map[component._id]) { - map[component._id] = { - component, - definition: def, - contexts: [], - } + + // Filter out global contexts not in the same branch. + // Global contexts are only valid if their branch root is an ancestor of + // this component. + const branch = contextTreeLookupMap[component._id] + if (branch !== "root" && !componentPathIds.includes(branch)) { + return } + + // Process all contexts provided by this component const contexts = Array.isArray(def.context) ? def.context : [def.context] contexts.forEach(context => { // Ensure type matches @@ -235,7 +246,7 @@ export const getComponentContexts = ( return } // Ensure scope matches - let contextScope = context.scope || "global" + let contextScope = context.scope || ContextScopes.Global if (contextScope !== scope) { return } @@ -243,17 +254,23 @@ export const getComponentContexts = ( if (!isContextCompatibleWithComponent(context, component)) { return } + if (!map[component._id]) { + map[component._id] = { + component, + definition: def, + contexts: [], + } + } map[component._id].contexts.push(context) }) } // Process all global contexts const allComponents = findAllComponents(asset.props) - allComponents.forEach(processContexts("global")) + allComponents.forEach(processContexts(ContextScopes.Global)) - // Process all local contexts - const localComponents = findComponentPath(asset.props, componentId) - localComponents.forEach(processContexts("local")) + // Process all local contexts in the immediate tree + componentPath.forEach(processContexts(ContextScopes.Local)) // Exclude self if required if (!options?.includeSelf) { diff --git a/packages/client/manifest.json b/packages/client/manifest.json index 5bbf465766..febc03085e 100644 --- a/packages/client/manifest.json +++ b/packages/client/manifest.json @@ -4720,7 +4720,8 @@ } ], "context": { - "type": "schema" + "type": "schema", + "scope": "local" } }, "daterangepicker": { diff --git a/packages/client/src/components/app/Repeater.svelte b/packages/client/src/components/app/Repeater.svelte index 8796078311..2d07342cf5 100644 --- a/packages/client/src/components/app/Repeater.svelte +++ b/packages/client/src/components/app/Repeater.svelte @@ -2,7 +2,9 @@ import { getContext } from "svelte" import Placeholder from "./Placeholder.svelte" import Container from "./Container.svelte" - import { ContextScopes } from "constants" + + const { Provider, ContextScopes } = getContext("sdk") + const component = getContext("component") export let dataProvider export let noRowsMessage @@ -12,9 +14,6 @@ export let gap export let scope = ContextScopes.Local - const { Provider } = getContext("sdk") - const component = getContext("component") - $: rows = dataProvider?.rows ?? [] $: loaded = dataProvider?.loaded ?? true diff --git a/packages/client/src/components/context/Provider.svelte b/packages/client/src/components/context/Provider.svelte index ad5b580c4f..fe4463821e 100644 --- a/packages/client/src/components/context/Provider.svelte +++ b/packages/client/src/components/context/Provider.svelte @@ -1,9 +1,11 @@