From 401bfa812cd9d464efe385e81d0bb02ec6bf7e97 Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 6 Jan 2025 11:45:53 +0000 Subject: [PATCH 01/34] Screen store to ts and test updates. Minor changes required for component store --- .../builder/src/stores/builder/components.ts | 39 ++- packages/builder/src/stores/builder/index.js | 2 +- .../stores/builder/{screens.js => screens.ts} | 169 ++++++---- .../src/stores/builder/tests/screens.test.js | 299 +++++++++--------- packages/frontend-core/src/utils/utils.js | 2 +- 5 files changed, 300 insertions(+), 211 deletions(-) rename packages/builder/src/stores/builder/{screens.js => screens.ts} (77%) diff --git a/packages/builder/src/stores/builder/components.ts b/packages/builder/src/stores/builder/components.ts index 9ad9a75f84..bce7fcb71d 100644 --- a/packages/builder/src/stores/builder/components.ts +++ b/packages/builder/src/stores/builder/components.ts @@ -33,7 +33,16 @@ import { Utils } from "@budibase/frontend-core" import { Component, FieldType, Screen, Table } from "@budibase/types" import { utils } from "@budibase/shared-core" -interface ComponentDefinition { +export interface ComponentState { + components: Record + customComponents: string[] + selectedComponentId?: string | null + componentToPaste?: Component | null + settingsCache: Record + selectedScreenId?: string | null +} + +export interface ComponentDefinition { component: string name: string friendlyName?: string @@ -41,9 +50,11 @@ interface ComponentDefinition { settings?: ComponentSetting[] features?: Record typeSupportPresets?: Record + legalDirectChildren: string[] + illegalChildren: string[] } -interface ComponentSetting { +export interface ComponentSetting { key: string type: string section?: string @@ -54,15 +65,6 @@ interface ComponentSetting { settings?: ComponentSetting[] } -interface ComponentState { - components: Record - customComponents: string[] - selectedComponentId: string | null - componentToPaste?: Component | null - settingsCache: Record - selectedScreenId?: string | null -} - export const INITIAL_COMPONENTS_STATE: ComponentState = { components: {}, customComponents: [], @@ -440,6 +442,11 @@ export class ComponentStore extends BudiStore { * @returns */ createInstance(componentName: string, presetProps: any, parent: any) { + const screen = get(selectedScreen) + if (!screen || !selectedScreen) { + throw "A valid screen must be selected" + } + const definition = this.getDefinition(componentName) if (!definition) { return null @@ -461,7 +468,7 @@ export class ComponentStore extends BudiStore { // Standard post processing this.enrichEmptySettings(instance, { parent, - screen: get(selectedScreen), + screen, useDefaultValues: true, }) @@ -481,7 +488,7 @@ export class ComponentStore extends BudiStore { // Add step name to form steps if (componentName.endsWith("/formstep")) { const parentForm = findClosestMatchingComponent( - get(selectedScreen).props, + screen.props, get(selectedComponent)._id, (component: Component) => component._component.endsWith("/form") ) @@ -841,6 +848,9 @@ export class ComponentStore extends BudiStore { const state = get(this.store) const componentId = state.selectedComponentId const screen = get(selectedScreen) + if (!screen) { + throw "A valid screen must be selected" + } const parent = findComponentParent(screen.props, componentId) const index = parent?._children.findIndex( (x: Component) => x._id === componentId @@ -890,6 +900,9 @@ export class ComponentStore extends BudiStore { const component = get(selectedComponent) const componentId = component?._id const screen = get(selectedScreen) + if (!screen) { + throw "A valid screen must be selected" + } const parent = findComponentParent(screen.props, componentId) const index = parent?._children.findIndex( (x: Component) => x._id === componentId diff --git a/packages/builder/src/stores/builder/index.js b/packages/builder/src/stores/builder/index.js index 08d87bebf5..0d4682b551 100644 --- a/packages/builder/src/stores/builder/index.js +++ b/packages/builder/src/stores/builder/index.js @@ -3,7 +3,7 @@ import { appStore } from "./app.js" import { componentStore, selectedComponent } from "./components" import { navigationStore } from "./navigation.js" import { themeStore } from "./theme.js" -import { screenStore, selectedScreen, sortedScreens } from "./screens.js" +import { screenStore, selectedScreen, sortedScreens } from "./screens" import { builderStore } from "./builder.js" import { hoverStore } from "./hover.js" import { previewStore } from "./preview.js" diff --git a/packages/builder/src/stores/builder/screens.js b/packages/builder/src/stores/builder/screens.ts similarity index 77% rename from packages/builder/src/stores/builder/screens.js rename to packages/builder/src/stores/builder/screens.ts index 8298a1469d..fd16cbfae8 100644 --- a/packages/builder/src/stores/builder/screens.js +++ b/packages/builder/src/stores/builder/screens.ts @@ -13,15 +13,32 @@ import { import { createHistoryStore } from "@/stores/builder/history" import { API } from "@/api" import { BudiStore } from "../BudiStore" +import { + FetchAppPackageResponse, + DeleteScreenResponse, + Screen, + Component, +} from "@budibase/types" +import { ComponentDefinition } from "./components" -export const INITIAL_SCREENS_STATE = { - screens: [], - selectedScreenId: null, +interface ScreenState { + screens: Screen[] + selectedScreenId?: string + selected?: Screen } -export class ScreenStore extends BudiStore { +export const initialScreenState: ScreenState = { + screens: [], +} + +// Review the nulls +export class ScreenStore extends BudiStore { + history: any + delete: any + save: any + constructor() { - super(INITIAL_SCREENS_STATE) + super(initialScreenState) // Bind scope this.select = this.select.bind(this) @@ -34,12 +51,15 @@ export class ScreenStore extends BudiStore { this.deleteScreen = this.deleteScreen.bind(this) this.syncScreenData = this.syncScreenData.bind(this) this.updateSetting = this.updateSetting.bind(this) + // TODO review this behaviour this.sequentialScreenPatch = this.sequentialScreenPatch.bind(this) this.removeCustomLayout = this.removeCustomLayout.bind(this) this.history = createHistoryStore({ - getDoc: id => get(this.store).screens?.find(screen => screen._id === id), + getDoc: (id: string) => + get(this.store).screens?.find(screen => screen._id === id), selectDoc: this.select, + beforeAction: () => {}, afterAction: () => { // Ensure a valid component is selected if (!get(selectedComponent)) { @@ -59,14 +79,14 @@ export class ScreenStore extends BudiStore { * Reset entire store back to base config */ reset() { - this.store.set({ ...INITIAL_SCREENS_STATE }) + this.store.set({ ...initialScreenState }) } /** * Replace ALL store screens with application package screens * @param {object} pkg */ - syncAppScreens(pkg) { + syncAppScreens(pkg: FetchAppPackageResponse) { this.update(state => ({ ...state, screens: [...pkg.screens], @@ -79,7 +99,7 @@ export class ScreenStore extends BudiStore { * @param {string} screenId * @returns */ - select(screenId) { + select(screenId: string) { // Check this screen exists const state = get(this.store) const screen = state.screens.find(screen => screen._id === screenId) @@ -107,14 +127,14 @@ export class ScreenStore extends BudiStore { * @throws Will throw an error containing the name of the component causing * the invalid screen state */ - validate(screen) { + validate(screen: Screen) { // Recursive function to find any illegal children in component trees const findIllegalChild = ( - component, - illegalChildren = [], - legalDirectChildren = [] - ) => { - const type = component._component + component: Component, + illegalChildren: string[] = [], + legalDirectChildren: string[] = [] + ): string | undefined => { + const type: string = component._component if (illegalChildren.includes(type)) { return type @@ -137,7 +157,13 @@ export class ScreenStore extends BudiStore { illegalChildren = [] } - const definition = componentStore.getDefinition(component._component) + const definition: ComponentDefinition | null = + componentStore.getDefinition(component._component) + + if (definition == null) { + throw `Invalid defintion ${component._component}` + } + // Reset whitelist for direct children legalDirectChildren = [] if (definition?.legalDirectChildren?.length) { @@ -172,7 +198,7 @@ export class ScreenStore extends BudiStore { const illegalChild = findIllegalChild(screen.props) if (illegalChild) { const def = componentStore.getDefinition(illegalChild) - throw `You can't place a ${def.name} here` + throw `You can't place a ${def?.name} here` } } @@ -183,7 +209,7 @@ export class ScreenStore extends BudiStore { * @param {object} screen * @returns {object} */ - async saveScreen(screen) { + async saveScreen(screen: Screen) { const appState = get(appStore) // Validate screen structure if the app supports it @@ -230,7 +256,7 @@ export class ScreenStore extends BudiStore { * After saving a screen, sync plugins and routes to the appStore * @param {object} savedScreen */ - async syncScreenData(savedScreen) { + async syncScreenData(savedScreen: Screen) { const appState = get(appStore) // If plugins changed we need to fetch the latest app metadata let usedPlugins = appState.usedPlugins @@ -256,28 +282,51 @@ export class ScreenStore extends BudiStore { * This is slightly better than just a traditional "patch" endpoint and this * supports deeply mutating the current doc rather than just appending data. */ - sequentialScreenPatch = Utils.sequential(async (patchFn, screenId) => { - const state = get(this.store) - const screen = state.screens.find(screen => screen._id === screenId) - if (!screen) { - return - } - let clone = cloneDeep(screen) - const result = patchFn(clone) + // sequentialScreenPatch = ( + // patchFn: (screen: Screen) => any, + // screenId: string + // ) => { + // return Utils.sequential(async () => { + // const state = get(this.store) + // const screen = state.screens.find(screen => screen._id === screenId) + // if (!screen) { + // return + // } + // let clone = cloneDeep(screen) + // const result = patchFn(clone) - // An explicit false result means skip this change - if (result === false) { - return + // // An explicit false result means skip this change + // if (result === false) { + // return + // } + // return this.save(clone) + // }) + // } + + sequentialScreenPatch = Utils.sequential( + async (patchFn: (screen: Screen) => any, screenId: string) => { + const state = get(this.store) + const screen = state.screens.find(screen => screen._id === screenId) + if (!screen) { + return + } + let clone = cloneDeep(screen) + const result = patchFn(clone) + + // An explicit false result means skip this change + if (result === false) { + return + } + return this.save(clone) } - return this.save(clone) - }) + ) /** * @param {function} patchFn * @param {string | null} screenId * @returns */ - async patch(patchFn, screenId) { + async patch(patchFn: (screen: Screen) => any, screenId?: string | null) { // Default to the currently selected screen if (!screenId) { const state = get(this.store) @@ -298,7 +347,7 @@ export class ScreenStore extends BudiStore { * @param {object} screen * @returns */ - async replace(screenId, screen) { + async replace(screenId: string, screen: Screen) { if (!screenId) { return } @@ -337,17 +386,25 @@ export class ScreenStore extends BudiStore { * @param {object | array} screens * @returns */ - async deleteScreen(screens) { + async deleteScreen(screens: Screen[]) { const screensToDelete = Array.isArray(screens) ? screens : [screens] // Build array of promises to speed up bulk deletions - let promises = [] - let deleteUrls = [] - screensToDelete.forEach(screen => { - // Delete the screen - promises.push(API.deleteScreen(screen._id, screen._rev)) - // Remove links to this screen - deleteUrls.push(screen.routing.route) - }) + let promises: Promise[] = [] + let deleteUrls: string[] = [] + + // In this instance _id will have been set + // Underline the expectation that _id and _rev will be set after filtering + screensToDelete + .filter( + (screen): screen is Screen & { _id: string; _rev: string } => + !!screen._id || !!screen._rev + ) + .forEach(screen => { + // Delete the screen + promises.push(API.deleteScreen(screen._id, screen._rev)) + // Remove links to this screen + deleteUrls.push(screen.routing.route) + }) await Promise.all(promises) await navigationStore.deleteLink(deleteUrls) const deletedIds = screensToDelete.map(screen => screen._id) @@ -359,8 +416,11 @@ export class ScreenStore extends BudiStore { }) // Deselect the current screen if it was deleted - if (deletedIds.includes(state.selectedScreenId)) { - state.selectedScreenId = null + if ( + state.selectedScreenId && + deletedIds.includes(state.selectedScreenId) + ) { + delete state.selectedScreenId componentStore.update(state => ({ ...state, selectedComponentId: null, @@ -389,13 +449,13 @@ export class ScreenStore extends BudiStore { * @param {any} value * @returns */ - async updateSetting(screen, name, value) { + async updateSetting(screen: Screen, name: string, value: any) { if (!screen || !name) { return } // Apply setting update - const patchFn = screen => { + const patchFn = (screen: Screen) => { if (!screen) { return false } @@ -422,7 +482,7 @@ export class ScreenStore extends BudiStore { ) }) if (otherHomeScreens.length && updatedScreen.routing.homeScreen) { - const patchFn = screen => { + const patchFn = (screen: Screen) => { screen.routing.homeScreen = false } for (let otherHomeScreen of otherHomeScreens) { @@ -432,11 +492,11 @@ export class ScreenStore extends BudiStore { } // Move to layouts store - async removeCustomLayout(screen) { + async removeCustomLayout(screen: Screen) { // Pull relevant settings from old layout, if required const layout = get(layoutStore).layouts.find(x => x._id === screen.layoutId) - const patchFn = screen => { - screen.layoutId = null + const patchFn = (screen: Screen) => { + delete screen.layoutId screen.showNavigation = layout?.props.navigation !== "None" screen.width = layout?.props.width || "Large" } @@ -448,9 +508,12 @@ export class ScreenStore extends BudiStore { * and up-to-date. Ensures stability after a product update. * @param {object} screen */ - async enrichEmptySettings(screen) { + async enrichEmptySettings(screen: Screen) { // Flatten the recursive component tree - const components = findAllMatchingComponents(screen.props, x => x) + const components = findAllMatchingComponents( + screen.props, + (x: Component) => x + ) // Iterate over all components and run checks components.forEach(component => { diff --git a/packages/builder/src/stores/builder/tests/screens.test.js b/packages/builder/src/stores/builder/tests/screens.test.js index 430605b77a..87eb03ea04 100644 --- a/packages/builder/src/stores/builder/tests/screens.test.js +++ b/packages/builder/src/stores/builder/tests/screens.test.js @@ -3,7 +3,7 @@ import { get, writable } from "svelte/store" import { API } from "@/api" import { Constants } from "@budibase/frontend-core" import { componentStore, appStore } from "@/stores/builder" -import { INITIAL_SCREENS_STATE, ScreenStore } from "@/stores/builder/screens" +import { initialScreenState, ScreenStore } from "@/stores/builder/screens" import { getScreenFixture, getComponentFixture, @@ -73,7 +73,7 @@ describe("Screens store", () => { vi.clearAllMocks() const screenStore = new ScreenStore() - ctx.test = { + ctx.bb = { get store() { return get(screenStore) }, @@ -81,74 +81,76 @@ describe("Screens store", () => { } }) - it("Create base screen store with defaults", ctx => { - expect(ctx.test.store).toStrictEqual(INITIAL_SCREENS_STATE) + it("Create base screen store with defaults", ({ bb }) => { + expect(bb.store).toStrictEqual(initialScreenState) }) - it("Syncs all screens from the app package", ctx => { - expect(ctx.test.store.screens.length).toBe(0) + it("Syncs all screens from the app package", ({ bb }) => { + expect(bb.store.screens.length).toBe(0) const screens = Array(2) .fill() .map(() => getScreenFixture().json()) - ctx.test.screenStore.syncAppScreens({ screens }) + bb.screenStore.syncAppScreens({ screens }) - expect(ctx.test.store.screens).toStrictEqual(screens) + expect(bb.store.screens).toStrictEqual(screens) }) - it("Reset the screen store back to the default state", ctx => { - expect(ctx.test.store.screens.length).toBe(0) + it("Reset the screen store back to the default state", ({ bb }) => { + expect(bb.store.screens.length).toBe(0) const screens = Array(2) .fill() .map(() => getScreenFixture().json()) - ctx.test.screenStore.syncAppScreens({ screens }) - expect(ctx.test.store.screens).toStrictEqual(screens) + bb.screenStore.syncAppScreens({ screens }) + expect(bb.store.screens).toStrictEqual(screens) - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, selectedScreenId: screens[0]._id, })) - ctx.test.screenStore.reset() + bb.screenStore.reset() - expect(ctx.test.store).toStrictEqual(INITIAL_SCREENS_STATE) + expect(bb.store).toStrictEqual(initialScreenState) }) - it("Marks a valid screen as selected", ctx => { + it("Marks a valid screen as selected", ({ bb }) => { const screens = Array(2) .fill() .map(() => getScreenFixture().json()) - ctx.test.screenStore.syncAppScreens({ screens }) - expect(ctx.test.store.screens.length).toBe(2) + bb.screenStore.syncAppScreens({ screens }) + expect(bb.store.screens.length).toBe(2) - ctx.test.screenStore.select(screens[0]._id) + bb.screenStore.select(screens[0]._id) - expect(ctx.test.store.selectedScreenId).toEqual(screens[0]._id) + expect(bb.store.selectedScreenId).toEqual(screens[0]._id) }) - it("Skip selecting a screen if it is not present", ctx => { + it("Skip selecting a screen if it is not present", ({ bb }) => { const screens = Array(2) .fill() .map(() => getScreenFixture().json()) - ctx.test.screenStore.syncAppScreens({ screens }) - expect(ctx.test.store.screens.length).toBe(2) + bb.screenStore.syncAppScreens({ screens }) + expect(bb.store.screens.length).toBe(2) - ctx.test.screenStore.select("screen_abc") + bb.screenStore.select("screen_abc") - expect(ctx.test.store.selectedScreenId).toBeNull() + expect(bb.store.selectedScreenId).toBeUndefined() }) - it("Approve a valid empty screen config", ctx => { + it("Approve a valid empty screen config", ({ bb }) => { const coreScreen = getScreenFixture() - ctx.test.screenStore.validate(coreScreen.json()) + bb.screenStore.validate(coreScreen.json()) }) - it("Approve a valid screen config with one component and no illegal children", ctx => { + it("Approve a valid screen config with one component and no illegal children", ({ + bb, + }) => { const coreScreen = getScreenFixture() const formBlock = getComponentFixture(`${COMP_PREFIX}/formblock`) @@ -157,12 +159,12 @@ describe("Screens store", () => { const defSpy = vi.spyOn(componentStore, "getDefinition") defSpy.mockReturnValueOnce(COMPONENT_DEFINITIONS.formblock) - ctx.test.screenStore.validate(coreScreen.json()) + bb.screenStore.validate(coreScreen.json()) expect(defSpy).toHaveBeenCalled() }) - it("Reject an attempt to nest invalid components", ctx => { + it("Reject an attempt to nest invalid components", ({ bb }) => { const coreScreen = getScreenFixture() const formOne = getComponentFixture(`${COMP_PREFIX}/form`) @@ -178,14 +180,14 @@ describe("Screens store", () => { return defMap[comp] }) - expect(() => ctx.test.screenStore.validate(coreScreen.json())).toThrowError( + expect(() => bb.screenStore.validate(coreScreen.json())).toThrowError( `You can't place a ${COMPONENT_DEFINITIONS.form.name} here` ) expect(defSpy).toHaveBeenCalled() }) - it("Reject an attempt to deeply nest invalid components", ctx => { + it("Reject an attempt to deeply nest invalid components", ({ bb }) => { const coreScreen = getScreenFixture() const formOne = getComponentFixture(`${COMP_PREFIX}/form`) @@ -210,14 +212,16 @@ describe("Screens store", () => { return defMap[comp] }) - expect(() => ctx.test.screenStore.validate(coreScreen.json())).toThrowError( + expect(() => bb.screenStore.validate(coreScreen.json())).toThrowError( `You can't place a ${COMPONENT_DEFINITIONS.form.name} here` ) expect(defSpy).toHaveBeenCalled() }) - it("Save a brand new screen and add it to the store. No validation", async ctx => { + it("Save a brand new screen and add it to the store. No validation", async ({ + bb, + }) => { const coreScreen = getScreenFixture() const formOne = getComponentFixture(`${COMP_PREFIX}/form`) @@ -225,7 +229,7 @@ describe("Screens store", () => { appStore.set({ features: { componentValidation: false } }) - expect(ctx.test.store.screens.length).toBe(0) + expect(bb.store.screens.length).toBe(0) const newDocId = getScreenDocId() const newDoc = { ...coreScreen.json(), _id: newDocId } @@ -235,15 +239,15 @@ describe("Screens store", () => { vi.spyOn(API, "fetchAppRoutes").mockResolvedValue({ routes: [], }) - await ctx.test.screenStore.save(coreScreen.json()) + await bb.screenStore.save(coreScreen.json()) expect(saveSpy).toHaveBeenCalled() - expect(ctx.test.store.screens.length).toBe(1) + expect(bb.store.screens.length).toBe(1) - expect(ctx.test.store.screens[0]).toStrictEqual(newDoc) + expect(bb.store.screens[0]).toStrictEqual(newDoc) - expect(ctx.test.store.selectedScreenId).toBe(newDocId) + expect(bb.store.selectedScreenId).toBe(newDocId) // The new screen should be selected expect(get(componentStore).selectedComponentId).toBe( @@ -251,7 +255,7 @@ describe("Screens store", () => { ) }) - it("Sync an updated screen to the screen store on save", async ctx => { + it("Sync an updated screen to the screen store on save", async ({ bb }) => { const existingScreens = Array(4) .fill() .map(() => { @@ -261,7 +265,7 @@ describe("Screens store", () => { return screenDoc }) - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, screens: existingScreens.map(screen => screen.json()), })) @@ -279,16 +283,18 @@ describe("Screens store", () => { }) // Saved the existing screen having modified it. - await ctx.test.screenStore.save(existingScreens[2].json()) + await bb.screenStore.save(existingScreens[2].json()) expect(routeSpy).toHaveBeenCalled() expect(saveSpy).toHaveBeenCalled() // On save, the screen is spliced back into the store with the saved content - expect(ctx.test.store.screens[2]).toStrictEqual(existingScreens[2].json()) + expect(bb.store.screens[2]).toStrictEqual(existingScreens[2].json()) }) - it("Sync API data to relevant stores on save. Updated plugins", async ctx => { + it("Sync API data to relevant stores on save. Updated plugins", async ({ + bb, + }) => { const coreScreen = getScreenFixture() const newDocId = getScreenDocId() @@ -318,7 +324,7 @@ describe("Screens store", () => { routes: [], }) - await ctx.test.screenStore.syncScreenData(newDoc) + await bb.screenStore.syncScreenData(newDoc) expect(routeSpy).toHaveBeenCalled() expect(appPackageSpy).toHaveBeenCalled() @@ -326,7 +332,9 @@ describe("Screens store", () => { expect(get(appStore).usedPlugins).toStrictEqual(plugins) }) - it("Sync API updates to relevant stores on save. Plugins unchanged", async ctx => { + it("Sync API updates to relevant stores on save. Plugins unchanged", async ({ + bb, + }) => { const coreScreen = getScreenFixture() const newDocId = getScreenDocId() @@ -343,7 +351,7 @@ describe("Screens store", () => { routes: [], }) - await ctx.test.screenStore.syncScreenData(newDoc) + await bb.screenStore.syncScreenData(newDoc) expect(routeSpy).toHaveBeenCalled() expect(appPackageSpy).not.toHaveBeenCalled() @@ -352,46 +360,48 @@ describe("Screens store", () => { expect(get(appStore).usedPlugins).toStrictEqual([plugin]) }) - it("Proceed to patch if appropriate config are supplied", async ctx => { - vi.spyOn(ctx.test.screenStore, "sequentialScreenPatch").mockImplementation( - () => { - return false - } - ) + it("Proceed to patch if appropriate config are supplied", async ({ bb }) => { + vi.spyOn(bb.screenStore, "sequentialScreenPatch").mockImplementation(() => { + return false + }) const noop = () => {} - await ctx.test.screenStore.patch(noop, "test") - expect(ctx.test.screenStore.sequentialScreenPatch).toHaveBeenCalledWith( + await bb.screenStore.patch(noop, "test") + expect(bb.screenStore.sequentialScreenPatch).toHaveBeenCalledWith( noop, "test" ) }) - it("Return from the patch if all valid config are not present", async ctx => { - vi.spyOn(ctx.test.screenStore, "sequentialScreenPatch") - await ctx.test.screenStore.patch() - expect(ctx.test.screenStore.sequentialScreenPatch).not.toBeCalled() + it("Return from the patch if all valid config are not present", async ({ + bb, + }) => { + vi.spyOn(bb.screenStore, "sequentialScreenPatch") + await bb.screenStore.patch() + expect(bb.screenStore.sequentialScreenPatch).not.toBeCalled() }) - it("Acquire the currently selected screen on patch, if not specified", async ctx => { - vi.spyOn(ctx.test.screenStore, "sequentialScreenPatch") - await ctx.test.screenStore.patch() + it("Acquire the currently selected screen on patch, if not specified", async ({ + bb, + }) => { + vi.spyOn(bb.screenStore, "sequentialScreenPatch") + await bb.screenStore.patch() const noop = () => {} - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, selectedScreenId: "screen_123", })) - await ctx.test.screenStore.patch(noop) - expect(ctx.test.screenStore.sequentialScreenPatch).toHaveBeenCalledWith( + await bb.screenStore.patch(noop) + expect(bb.screenStore.sequentialScreenPatch).toHaveBeenCalledWith( noop, "screen_123" ) }) // Used by the websocket - it("Ignore a call to replace if no screenId is provided", ctx => { + it("Ignore a call to replace if no screenId is provided", ({ bb }) => { const existingScreens = Array(4) .fill() .map(() => { @@ -400,14 +410,16 @@ describe("Screens store", () => { screenDoc._json._id = existingDocId return screenDoc.json() }) - ctx.test.screenStore.syncAppScreens({ screens: existingScreens }) + bb.screenStore.syncAppScreens({ screens: existingScreens }) - ctx.test.screenStore.replace() + bb.screenStore.replace() - expect(ctx.test.store.screens).toStrictEqual(existingScreens) + expect(bb.store.screens).toStrictEqual(existingScreens) }) - it("Remove a screen from the store if a single screenId is supplied", ctx => { + it("Remove a screen from the store if a single screenId is supplied", ({ + bb, + }) => { const existingScreens = Array(4) .fill() .map(() => { @@ -416,17 +428,17 @@ describe("Screens store", () => { screenDoc._json._id = existingDocId return screenDoc.json() }) - ctx.test.screenStore.syncAppScreens({ screens: existingScreens }) + bb.screenStore.syncAppScreens({ screens: existingScreens }) - ctx.test.screenStore.replace(existingScreens[1]._id) + bb.screenStore.replace(existingScreens[1]._id) const filtered = existingScreens.filter( screen => screen._id != existingScreens[1]._id ) - expect(ctx.test.store.screens).toStrictEqual(filtered) + expect(bb.store.screens).toStrictEqual(filtered) }) - it("Replace an existing screen with a new version of itself", ctx => { + it("Replace an existing screen with a new version of itself", ({ bb }) => { const existingScreens = Array(4) .fill() .map(() => { @@ -436,7 +448,7 @@ describe("Screens store", () => { return screenDoc }) - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, screens: existingScreens.map(screen => screen.json()), })) @@ -444,15 +456,14 @@ describe("Screens store", () => { const formBlock = getComponentFixture(`${COMP_PREFIX}/formblock`) existingScreens[2].addChild(formBlock) - ctx.test.screenStore.replace( - existingScreens[2]._id, - existingScreens[2].json() - ) + bb.screenStore.replace(existingScreens[2]._id, existingScreens[2].json()) - expect(ctx.test.store.screens.length).toBe(4) + expect(bb.store.screens.length).toBe(4) }) - it("Add a screen when attempting to replace one not present in the store", ctx => { + it("Add a screen when attempting to replace one not present in the store", ({ + bb, + }) => { const existingScreens = Array(4) .fill() .map(() => { @@ -462,7 +473,7 @@ describe("Screens store", () => { return screenDoc }) - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, screens: existingScreens.map(screen => screen.json()), })) @@ -470,13 +481,13 @@ describe("Screens store", () => { const newScreenDoc = getScreenFixture() newScreenDoc._json._id = getScreenDocId() - ctx.test.screenStore.replace(newScreenDoc._json._id, newScreenDoc.json()) + bb.screenStore.replace(newScreenDoc._json._id, newScreenDoc.json()) - expect(ctx.test.store.screens.length).toBe(5) - expect(ctx.test.store.screens[4]).toStrictEqual(newScreenDoc.json()) + expect(bb.store.screens.length).toBe(5) + expect(bb.store.screens[4]).toStrictEqual(newScreenDoc.json()) }) - it("Delete a single screen and remove it from the store", async ctx => { + it("Delete a single screen and remove it from the store", async ({ bb }) => { const existingScreens = Array(3) .fill() .map(() => { @@ -486,14 +497,14 @@ describe("Screens store", () => { return screenDoc }) - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, screens: existingScreens.map(screen => screen.json()), })) const deleteSpy = vi.spyOn(API, "deleteScreen") - await ctx.test.screenStore.delete(existingScreens[2].json()) + await bb.screenStore.delete(existingScreens[2].json()) vi.spyOn(API, "fetchAppRoutes").mockResolvedValue({ routes: [], @@ -501,13 +512,15 @@ describe("Screens store", () => { expect(deleteSpy).toBeCalled() - expect(ctx.test.store.screens.length).toBe(2) + expect(bb.store.screens.length).toBe(2) // Just confirm that the routes at are being initialised expect(get(appStore).routes).toEqual([]) }) - it("Upon delete, reset selected screen and component ids if the screen was selected", async ctx => { + it("Upon delete, reset selected screen and component ids if the screen was selected", async ({ + bb, + }) => { const existingScreens = Array(3) .fill() .map(() => { @@ -517,7 +530,7 @@ describe("Screens store", () => { return screenDoc }) - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, screens: existingScreens.map(screen => screen.json()), selectedScreenId: existingScreens[2]._json._id, @@ -528,14 +541,16 @@ describe("Screens store", () => { selectedComponentId: existingScreens[2]._json._id, })) - await ctx.test.screenStore.delete(existingScreens[2].json()) + await bb.screenStore.delete(existingScreens[2].json()) - expect(ctx.test.store.screens.length).toBe(2) + expect(bb.store.screens.length).toBe(2) expect(get(componentStore).selectedComponentId).toBeNull() - expect(ctx.test.store.selectedScreenId).toBeNull() + expect(bb.store.selectedScreenId).toBeUndefined() }) - it("Delete multiple is not supported and should leave the store unchanged", async ctx => { + it("Delete multiple is not supported and should leave the store unchanged", async ({ + bb, + }) => { const existingScreens = Array(3) .fill() .map(() => { @@ -547,7 +562,7 @@ describe("Screens store", () => { const storeScreens = existingScreens.map(screen => screen.json()) - ctx.test.screenStore.update(state => ({ + bb.screenStore.update(state => ({ ...state, screens: existingScreens.map(screen => screen.json()), })) @@ -556,42 +571,40 @@ describe("Screens store", () => { const deleteSpy = vi.spyOn(API, "deleteScreen") - await ctx.test.screenStore.delete(targets) + await bb.screenStore.delete(targets) expect(deleteSpy).not.toHaveBeenCalled() - expect(ctx.test.store.screens.length).toBe(3) - expect(ctx.test.store.screens).toStrictEqual(storeScreens) + expect(bb.store.screens.length).toBe(3) + expect(bb.store.screens).toStrictEqual(storeScreens) }) - it("Update a screen setting", async ctx => { + it("Update a screen setting", async ({ bb }) => { const screenDoc = getScreenFixture() const existingDocId = getScreenDocId() screenDoc._json._id = existingDocId - await ctx.test.screenStore.update(state => ({ + await bb.screenStore.update(state => ({ ...state, screens: [screenDoc.json()], })) const patchedDoc = screenDoc.json() const patchSpy = vi - .spyOn(ctx.test.screenStore, "patch") + .spyOn(bb.screenStore, "patch") .mockImplementation(async patchFn => { patchFn(patchedDoc) return }) - await ctx.test.screenStore.updateSetting( - patchedDoc, - "showNavigation", - false - ) + await bb.screenStore.updateSetting(patchedDoc, "showNavigation", false) expect(patchSpy).toBeCalled() expect(patchedDoc.showNavigation).toBe(false) }) - it("Ensure only one homescreen per role after updating setting. All screens same role", async ctx => { + it("Ensure only one homescreen per role after updating setting. All screens same role", async ({ + bb, + }) => { const existingScreens = Array(3) .fill() .map(() => { @@ -611,23 +624,21 @@ describe("Screens store", () => { // Set the 2nd screen as the home screen storeScreens[1].routing.homeScreen = true - await ctx.test.screenStore.update(state => ({ + await bb.screenStore.update(state => ({ ...state, screens: storeScreens, })) const patchSpy = vi - .spyOn(ctx.test.screenStore, "patch") + .spyOn(bb.screenStore, "patch") .mockImplementation(async (patchFn, screenId) => { - const target = ctx.test.store.screens.find( - screen => screen._id === screenId - ) + const target = bb.store.screens.find(screen => screen._id === screenId) patchFn(target) - await ctx.test.screenStore.replace(screenId, target) + await bb.screenStore.replace(screenId, target) }) - await ctx.test.screenStore.updateSetting( + await bb.screenStore.updateSetting( storeScreens[0], "routing.homeScreen", true @@ -637,13 +648,15 @@ describe("Screens store", () => { expect(patchSpy).toBeCalledTimes(2) // The new homescreen for BASIC - expect(ctx.test.store.screens[0].routing.homeScreen).toBe(true) + expect(bb.store.screens[0].routing.homeScreen).toBe(true) // The previous home screen for the BASIC role is now unset - expect(ctx.test.store.screens[1].routing.homeScreen).toBe(false) + expect(bb.store.screens[1].routing.homeScreen).toBe(false) }) - it("Ensure only one homescreen per role when updating screen setting. Multiple screen roles", async ctx => { + it("Ensure only one homescreen per role when updating screen setting. Multiple screen roles", async ({ + bb, + }) => { const expectedRoles = [ Constants.Roles.BASIC, Constants.Roles.POWER, @@ -675,30 +688,24 @@ describe("Screens store", () => { sorted[9].routing.homeScreen = true // Set screens state - await ctx.test.screenStore.update(state => ({ + await bb.screenStore.update(state => ({ ...state, screens: sorted, })) const patchSpy = vi - .spyOn(ctx.test.screenStore, "patch") + .spyOn(bb.screenStore, "patch") .mockImplementation(async (patchFn, screenId) => { - const target = ctx.test.store.screens.find( - screen => screen._id === screenId - ) + const target = bb.store.screens.find(screen => screen._id === screenId) patchFn(target) - await ctx.test.screenStore.replace(screenId, target) + await bb.screenStore.replace(screenId, target) }) // ADMIN homeScreen updated from 0 to 2 - await ctx.test.screenStore.updateSetting( - sorted[2], - "routing.homeScreen", - true - ) + await bb.screenStore.updateSetting(sorted[2], "routing.homeScreen", true) - const results = ctx.test.store.screens.reduce((acc, screen) => { + const results = bb.store.screens.reduce((acc, screen) => { if (screen.routing.homeScreen) { acc[screen.routing.roleId] = acc[screen.routing.roleId] || [] acc[screen.routing.roleId].push(screen) @@ -706,7 +713,7 @@ describe("Screens store", () => { return acc }, {}) - const screens = ctx.test.store.screens + const screens = bb.store.screens // Should still only be one of each homescreen expect(results[Constants.Roles.ADMIN].length).toBe(1) expect(screens[2].routing.homeScreen).toBe(true) @@ -724,74 +731,80 @@ describe("Screens store", () => { expect(patchSpy).toBeCalledTimes(2) }) - it("Sequential patch check. Exit if the screenId is not valid.", async ctx => { + it("Sequential patch check. Exit if the screenId is not valid.", async ({ + bb, + }) => { const screenDoc = getScreenFixture() const existingDocId = getScreenDocId() screenDoc._json._id = existingDocId const original = screenDoc.json() - await ctx.test.screenStore.update(state => ({ + await bb.screenStore.update(state => ({ ...state, screens: [original], })) const saveSpy = vi - .spyOn(ctx.test.screenStore, "save") + .spyOn(bb.screenStore, "save") .mockImplementation(async () => { return }) // A screen with this Id does not exist - await ctx.test.screenStore.sequentialScreenPatch(() => {}, "123") + await bb.screenStore.sequentialScreenPatch(() => {}, "123") expect(saveSpy).not.toBeCalled() }) - it("Sequential patch check. Exit if the patchFn result is false", async ctx => { + it("Sequential patch check. Exit if the patchFn result is false", async ({ + bb, + }) => { const screenDoc = getScreenFixture() const existingDocId = getScreenDocId() screenDoc._json._id = existingDocId const original = screenDoc.json() // Set screens state - await ctx.test.screenStore.update(state => ({ + await bb.screenStore.update(state => ({ ...state, screens: [original], })) const saveSpy = vi - .spyOn(ctx.test.screenStore, "save") + .spyOn(bb.screenStore, "save") .mockImplementation(async () => { return }) // Returning false from the patch will abort the save - await ctx.test.screenStore.sequentialScreenPatch(() => { + await bb.screenStore.sequentialScreenPatch(() => { return false }, "123") expect(saveSpy).not.toBeCalled() }) - it("Sequential patch check. Patch applied and save requested", async ctx => { + it("Sequential patch check. Patch applied and save requested", async ({ + bb, + }) => { const screenDoc = getScreenFixture() const existingDocId = getScreenDocId() screenDoc._json._id = existingDocId const original = screenDoc.json() - await ctx.test.screenStore.update(state => ({ + await bb.screenStore.update(state => ({ ...state, screens: [original], })) const saveSpy = vi - .spyOn(ctx.test.screenStore, "save") + .spyOn(bb.screenStore, "save") .mockImplementation(async () => { return }) - await ctx.test.screenStore.sequentialScreenPatch(screen => { + await bb.screenStore.sequentialScreenPatch(screen => { screen.name = "updated" }, existingDocId) diff --git a/packages/frontend-core/src/utils/utils.js b/packages/frontend-core/src/utils/utils.js index c424aea5b2..eeff561215 100644 --- a/packages/frontend-core/src/utils/utils.js +++ b/packages/frontend-core/src/utils/utils.js @@ -8,7 +8,7 @@ export const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)) * Utility to wrap an async function and ensure all invocations happen * sequentially. * @param fn the async function to run - * @return {Promise} a sequential version of the function + * @return {Function} a sequential version of the function */ export const sequential = fn => { let queue = [] From f1d57906b589642e90c457eaf956b6f48753ff3e Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 6 Jan 2025 14:25:13 +0000 Subject: [PATCH 02/34] Clean up jsdoc comments and remove unnecessary comments --- .../builder/src/stores/builder/screens.ts | 57 ++++++------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/packages/builder/src/stores/builder/screens.ts b/packages/builder/src/stores/builder/screens.ts index fd16cbfae8..a749ded1f9 100644 --- a/packages/builder/src/stores/builder/screens.ts +++ b/packages/builder/src/stores/builder/screens.ts @@ -18,6 +18,7 @@ import { DeleteScreenResponse, Screen, Component, + SaveScreenResponse, } from "@budibase/types" import { ComponentDefinition } from "./components" @@ -51,7 +52,6 @@ export class ScreenStore extends BudiStore { this.deleteScreen = this.deleteScreen.bind(this) this.syncScreenData = this.syncScreenData.bind(this) this.updateSetting = this.updateSetting.bind(this) - // TODO review this behaviour this.sequentialScreenPatch = this.sequentialScreenPatch.bind(this) this.removeCustomLayout = this.removeCustomLayout.bind(this) @@ -84,7 +84,7 @@ export class ScreenStore extends BudiStore { /** * Replace ALL store screens with application package screens - * @param {object} pkg + * @param {FetchAppPackageResponse} pkg */ syncAppScreens(pkg: FetchAppPackageResponse) { this.update(state => ({ @@ -123,7 +123,7 @@ export class ScreenStore extends BudiStore { * Recursively parses the entire screen doc and checks for components * violating illegal child configurations. * - * @param {object} screen + * @param {Screen} screen * @throws Will throw an error containing the name of the component causing * the invalid screen state */ @@ -206,8 +206,7 @@ export class ScreenStore extends BudiStore { * Core save method. If creating a new screen, the store will sync the target * screen id to ensure that it is selected in the builder * - * @param {object} screen - * @returns {object} + * @param {Screen} screen The screen being modified/created */ async saveScreen(screen: Screen) { const appState = get(appStore) @@ -254,7 +253,7 @@ export class ScreenStore extends BudiStore { /** * After saving a screen, sync plugins and routes to the appStore - * @param {object} savedScreen + * @param {Screen} savedScreen */ async syncScreenData(savedScreen: Screen) { const appState = get(appStore) @@ -282,27 +281,6 @@ export class ScreenStore extends BudiStore { * This is slightly better than just a traditional "patch" endpoint and this * supports deeply mutating the current doc rather than just appending data. */ - // sequentialScreenPatch = ( - // patchFn: (screen: Screen) => any, - // screenId: string - // ) => { - // return Utils.sequential(async () => { - // const state = get(this.store) - // const screen = state.screens.find(screen => screen._id === screenId) - // if (!screen) { - // return - // } - // let clone = cloneDeep(screen) - // const result = patchFn(clone) - - // // An explicit false result means skip this change - // if (result === false) { - // return - // } - // return this.save(clone) - // }) - // } - sequentialScreenPatch = Utils.sequential( async (patchFn: (screen: Screen) => any, screenId: string) => { const state = get(this.store) @@ -322,11 +300,13 @@ export class ScreenStore extends BudiStore { ) /** - * @param {function} patchFn + * @param {Function} patchFn the patch action to be applied * @param {string | null} screenId - * @returns */ - async patch(patchFn: (screen: Screen) => any, screenId?: string | null) { + async patch( + patchFn: (screen: Screen) => any, + screenId?: string | null + ): Promise { // Default to the currently selected screen if (!screenId) { const state = get(this.store) @@ -343,9 +323,9 @@ export class ScreenStore extends BudiStore { * the screen supplied. If no screen is provided, the target has * been removed by another user and will be filtered from the store. * Used to marshal updates for the websocket - * @param {string} screenId - * @param {object} screen - * @returns + * + * @param {string} screenId the target screen id + * @param {Screen} screen the replacement screen */ async replace(screenId: string, screen: Screen) { if (!screenId) { @@ -383,10 +363,9 @@ export class ScreenStore extends BudiStore { * Any deleted screens will then have their routes/links purged * * Wrapped by {@link delete} - * @param {object | array} screens - * @returns + * @param {Screen | Screen[]} screens */ - async deleteScreen(screens: Screen[]) { + async deleteScreen(screens: Screen | Screen[]) { const screensToDelete = Array.isArray(screens) ? screens : [screens] // Build array of promises to speed up bulk deletions let promises: Promise[] = [] @@ -435,7 +414,6 @@ export class ScreenStore extends BudiStore { return state }) - return null } /** @@ -444,10 +422,9 @@ export class ScreenStore extends BudiStore { * After a successful update, this method ensures that there is only * ONE home screen per user Role. * - * @param {object} screen + * @param {Screen} screen * @param {string} name e.g "routing.homeScreen" or "showNavigation" * @param {any} value - * @returns */ async updateSetting(screen: Screen, name: string, value: any) { if (!screen || !name) { @@ -506,7 +483,7 @@ export class ScreenStore extends BudiStore { /** * Parse the entire screen component tree and ensure settings are valid * and up-to-date. Ensures stability after a product update. - * @param {object} screen + * @param {Screen} screen */ async enrichEmptySettings(screen: Screen) { // Flatten the recursive component tree From 744b1d3dbcba03f6fd848c4d38dab4828f5fa5f8 Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 7 Jan 2025 09:32:02 +0000 Subject: [PATCH 03/34] Screen type fixes --- .../builder/src/stores/builder/componentTreeNodes.ts | 7 ++++++- packages/builder/src/stores/builder/websocket.ts | 9 ++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/stores/builder/componentTreeNodes.ts b/packages/builder/src/stores/builder/componentTreeNodes.ts index 420c540e37..cec4e3a4a0 100644 --- a/packages/builder/src/stores/builder/componentTreeNodes.ts +++ b/packages/builder/src/stores/builder/componentTreeNodes.ts @@ -49,7 +49,12 @@ export class ComponentTreeNodesStore extends BudiStore { // Will ensure all parents of a node are expanded so that it is visible in the tree makeNodeVisible(componentId: string) { - const selectedScreen: Screen = get(selectedScreenStore) + const selectedScreen: Screen | undefined = get(selectedScreenStore) + + if (!selectedScreen) { + console.error("Invalid node " + componentId) + return {} + } const path = findComponentPath(selectedScreen.props, componentId) diff --git a/packages/builder/src/stores/builder/websocket.ts b/packages/builder/src/stores/builder/websocket.ts index bd9e2c8d4d..a56fec2227 100644 --- a/packages/builder/src/stores/builder/websocket.ts +++ b/packages/builder/src/stores/builder/websocket.ts @@ -16,7 +16,14 @@ import { auth, appsStore } from "@/stores/portal" import { screenStore } from "./screens" import { SocketEvent, BuilderSocketEvent, helpers } from "@budibase/shared-core" import { notifications } from "@budibase/bbui" -import { Automation, Datasource, Role, Table, UIUser } from "@budibase/types" +import { + Automation, + Datasource, + Role, + Table, + UIUser, + Screen, +} from "@budibase/types" export const createBuilderWebsocket = (appId: string) => { const socket = createWebsocket("/socket/builder") From 1578c1a64b7508bd674d179296b57d7ce801baaf Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 20 Jan 2025 10:48:00 +0000 Subject: [PATCH 04/34] PR Feedback --- .../builder/src/stores/builder/components.ts | 18 ++++++++---------- packages/builder/src/stores/builder/screens.ts | 11 +++++------ .../src/stores/builder/tests/screens.test.js | 2 +- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/builder/src/stores/builder/components.ts b/packages/builder/src/stores/builder/components.ts index bce7fcb71d..7eb351e9da 100644 --- a/packages/builder/src/stores/builder/components.ts +++ b/packages/builder/src/stores/builder/components.ts @@ -36,8 +36,8 @@ import { utils } from "@budibase/shared-core" export interface ComponentState { components: Record customComponents: string[] - selectedComponentId?: string | null - componentToPaste?: Component | null + selectedComponentId?: string + componentToPaste?: Component settingsCache: Record selectedScreenId?: string | null } @@ -68,8 +68,6 @@ export interface ComponentSetting { export const INITIAL_COMPONENTS_STATE: ComponentState = { components: {}, customComponents: [], - selectedComponentId: null, - componentToPaste: null, settingsCache: {}, } @@ -443,7 +441,7 @@ export class ComponentStore extends BudiStore { */ createInstance(componentName: string, presetProps: any, parent: any) { const screen = get(selectedScreen) - if (!screen || !selectedScreen) { + if (!screen) { throw "A valid screen must be selected" } @@ -548,7 +546,7 @@ export class ComponentStore extends BudiStore { // Find the selected component let selectedComponentId = state.selectedComponentId if (selectedComponentId?.startsWith(`${screen._id}-`)) { - selectedComponentId = screen.props._id || null + selectedComponentId = screen.props._id } const currentComponent = findComponent( screen.props, @@ -659,7 +657,7 @@ export class ComponentStore extends BudiStore { // Determine the next component to select, and select it before deletion // to avoid an intermediate state of no component selection const state = get(this.store) - let nextId: string | null = "" + let nextId: string = "" if (state.selectedComponentId === component._id) { nextId = this.getNext() if (!nextId) { @@ -746,7 +744,7 @@ export class ComponentStore extends BudiStore { if (!state.componentToPaste) { return } - let newComponentId: string | null = "" + let newComponentId: string = "" // Remove copied component if cutting, regardless if pasting works let componentToPaste = cloneDeep(state.componentToPaste) @@ -1169,7 +1167,7 @@ export class ComponentStore extends BudiStore { } async handleEjectBlock(componentId: string, ejectedDefinition: Component) { - let nextSelectedComponentId: string | null = null + let nextSelectedComponentId: string | undefined await screenStore.patch((screen: Screen) => { const block = findComponent(screen.props, componentId) @@ -1205,7 +1203,7 @@ export class ComponentStore extends BudiStore { (x: Component) => x._id === componentId ) parent._children[index] = ejectedDefinition - nextSelectedComponentId = ejectedDefinition._id ?? null + nextSelectedComponentId = ejectedDefinition._id }, null) // Select new root component diff --git a/packages/builder/src/stores/builder/screens.ts b/packages/builder/src/stores/builder/screens.ts index a749ded1f9..5163c6a3ea 100644 --- a/packages/builder/src/stores/builder/screens.ts +++ b/packages/builder/src/stores/builder/screens.ts @@ -25,7 +25,6 @@ import { ComponentDefinition } from "./components" interface ScreenState { screens: Screen[] selectedScreenId?: string - selected?: Screen } export const initialScreenState: ScreenState = { @@ -65,7 +64,7 @@ export class ScreenStore extends BudiStore { if (!get(selectedComponent)) { this.update(state => ({ ...state, - selectedComponentId: get(this.store).selected?.props._id, + selectedComponentId: get(selectedScreen)?.props._id, })) } }, @@ -400,10 +399,10 @@ export class ScreenStore extends BudiStore { deletedIds.includes(state.selectedScreenId) ) { delete state.selectedScreenId - componentStore.update(state => ({ - ...state, - selectedComponentId: null, - })) + componentStore.update(state => { + delete state.selectedComponentId + return state + }) } // Update routing diff --git a/packages/builder/src/stores/builder/tests/screens.test.js b/packages/builder/src/stores/builder/tests/screens.test.js index 87eb03ea04..ea916c8d59 100644 --- a/packages/builder/src/stores/builder/tests/screens.test.js +++ b/packages/builder/src/stores/builder/tests/screens.test.js @@ -544,7 +544,7 @@ describe("Screens store", () => { await bb.screenStore.delete(existingScreens[2].json()) expect(bb.store.screens.length).toBe(2) - expect(get(componentStore).selectedComponentId).toBeNull() + expect(get(componentStore).selectedComponentId).toBeUndefined() expect(bb.store.selectedScreenId).toBeUndefined() }) From 51a98229e83fbf0858ad587b09d07c9aca74d544 Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 20 Jan 2025 11:05:17 +0000 Subject: [PATCH 05/34] Lint --- packages/builder/src/stores/builder/components.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/stores/builder/components.ts b/packages/builder/src/stores/builder/components.ts index 7eb351e9da..46d3e07eae 100644 --- a/packages/builder/src/stores/builder/components.ts +++ b/packages/builder/src/stores/builder/components.ts @@ -657,7 +657,7 @@ export class ComponentStore extends BudiStore { // Determine the next component to select, and select it before deletion // to avoid an intermediate state of no component selection const state = get(this.store) - let nextId: string = "" + let nextId = "" if (state.selectedComponentId === component._id) { nextId = this.getNext() if (!nextId) { @@ -744,7 +744,7 @@ export class ComponentStore extends BudiStore { if (!state.componentToPaste) { return } - let newComponentId: string = "" + let newComponentId = "" // Remove copied component if cutting, regardless if pasting works let componentToPaste = cloneDeep(state.componentToPaste) From 221021ae9b8051e3ca3a9257ef00c8bf33a0e08e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 21 Jan 2025 17:00:40 +0000 Subject: [PATCH 06/34] Support for our own vm-browserify implementation which re-uses the iframe for running JS in the frontend, to improve performance. --- packages/string-templates/package.json | 1 + packages/string-templates/src/index.ts | 6 +++--- yarn.lock | 18 +++++++++++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/string-templates/package.json b/packages/string-templates/package.json index 1d7a4507ab..74d9aaa85a 100644 --- a/packages/string-templates/package.json +++ b/packages/string-templates/package.json @@ -23,6 +23,7 @@ }, "dependencies": { "@budibase/handlebars-helpers": "^0.13.2", + "@budibase/vm-browserify": "^1.1.3", "dayjs": "^1.10.8", "handlebars": "^4.7.8", "lodash.clonedeep": "^4.5.0" diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts index 8dda8b71ab..bd008dd4d2 100644 --- a/packages/string-templates/src/index.ts +++ b/packages/string-templates/src/index.ts @@ -1,4 +1,4 @@ -import { createContext, runInNewContext } from "vm" +import vm from "@budibase/vm-browserify" import { create, TemplateDelegate } from "handlebars" import { registerAll, registerMinimum } from "./helpers/index" import { postprocess, postprocessWithLogs, preprocess } from "./processors" @@ -511,11 +511,11 @@ export function browserJSSetup() { * Use polyfilled vm to run JS scripts in a browser Env */ setJSRunner((js: string, context: Record) => { - createContext(context) + vm.createContext(context) const wrappedJs = frontendWrapJS(js) - const result = runInNewContext(wrappedJs, context, { timeout: 1000 }) + const result = vm.runInNewContext(wrappedJs, context) if (result.error) { throw new UserScriptError(result.error) } diff --git a/yarn.lock b/yarn.lock index 453dc45128..e25ff97747 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2131,9 +2131,9 @@ through2 "^2.0.0" "@budibase/pro@npm:@budibase/pro@latest": - version "3.2.44" - resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-3.2.44.tgz#90367bb2167aafd8c809e000a57d349e5dc4bb78" - integrity sha512-Zv2PBVUZUS6/psOpIRIDlW3jrOHWWPhpQXzCk00kIQJaqjkdcvuTXSedQ70u537sQmLu8JsSWbui9MdfF8ksVw== + version "3.2.47" + resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-3.2.47.tgz#150d7b16b14932d03c84bdb0e6d570d490c28a5c" + integrity sha512-UeTIq7yzMUK6w/akUsRafoD/Kif6PXv4d7K1arn8GTMjwFm9QYu2hg1YkQ+duNdwyZ/GEPlEAV5SYK+NDgtpdA== dependencies: "@anthropic-ai/sdk" "^0.27.3" "@budibase/backend-core" "*" @@ -2152,6 +2152,13 @@ scim-patch "^0.8.1" scim2-parse-filter "^0.2.8" +"@budibase/vm-browserify@^1.1.3": + version "1.1.3" + resolved "https://registry.yarnpkg.com/@budibase/vm-browserify/-/vm-browserify-1.1.3.tgz#63f7917671a0f0cb760e3aa37cfd5dfa32e997ed" + integrity sha512-CuoNb2xwS8TT2ZfG9YqC8QCTcG3ZPLwH4m00sfPDluwmdp3U3HGg/UKWRIqKC6Wv8Mywy1q6bxmSx6Vf40V52w== + dependencies: + indexof "^0.0.1" + "@bull-board/api@5.10.2": version "5.10.2" resolved "https://registry.yarnpkg.com/@bull-board/api/-/api-5.10.2.tgz#ae8ff6918b23897bf879a6ead3683f964374c4b3" @@ -11925,6 +11932,11 @@ indexes-of@^1.0.1: resolved "https://registry.yarnpkg.com/indexes-of/-/indexes-of-1.0.1.tgz#f30f716c8e2bd346c7b67d3df3915566a7c05607" integrity sha512-bup+4tap3Hympa+JBJUG7XuOsdNQ6fxt0MHyXMKuLBKn0OqsTfvUxkUrroEX1+B2VsSHvCjiIcZVxRtYa4nllA== +indexof@^0.0.1: + version "0.0.1" + resolved "https://registry.yarnpkg.com/indexof/-/indexof-0.0.1.tgz#82dc336d232b9062179d05ab3293a66059fd435d" + integrity sha512-i0G7hLJ1z0DE8dsqJa2rycj9dBmNKgXBvotXtZYXakU9oivfB9Uj2ZBC27qqef2U58/ZLwalxa1X/RDCdkHtVg== + infer-owner@^1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/infer-owner/-/infer-owner-1.0.4.tgz#c4cefcaa8e51051c2a40ba2ce8a3d27295af9467" From fbbe1738db7a4006e7bbe8dfea73de26e7b4c33d Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 22 Jan 2025 16:11:54 +0000 Subject: [PATCH 07/34] Using sandboxed iframe to limit to scripting only. --- packages/string-templates/package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/string-templates/package.json b/packages/string-templates/package.json index 74d9aaa85a..b1b4b9ef55 100644 --- a/packages/string-templates/package.json +++ b/packages/string-templates/package.json @@ -23,7 +23,7 @@ }, "dependencies": { "@budibase/handlebars-helpers": "^0.13.2", - "@budibase/vm-browserify": "^1.1.3", + "@budibase/vm-browserify": "^1.1.4", "dayjs": "^1.10.8", "handlebars": "^4.7.8", "lodash.clonedeep": "^4.5.0" diff --git a/yarn.lock b/yarn.lock index e25ff97747..a375c05ffd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2152,10 +2152,10 @@ scim-patch "^0.8.1" scim2-parse-filter "^0.2.8" -"@budibase/vm-browserify@^1.1.3": - version "1.1.3" - resolved "https://registry.yarnpkg.com/@budibase/vm-browserify/-/vm-browserify-1.1.3.tgz#63f7917671a0f0cb760e3aa37cfd5dfa32e997ed" - integrity sha512-CuoNb2xwS8TT2ZfG9YqC8QCTcG3ZPLwH4m00sfPDluwmdp3U3HGg/UKWRIqKC6Wv8Mywy1q6bxmSx6Vf40V52w== +"@budibase/vm-browserify@^1.1.4": + version "1.1.4" + resolved "https://registry.yarnpkg.com/@budibase/vm-browserify/-/vm-browserify-1.1.4.tgz#eecb001bd9521cb7647e26fb4d2d29d0a4dce262" + integrity sha512-/dyOLj+jQNKe6sVfLP6NdwA79OZxEWHCa41VGsjKJC9DYo6l2fEcL5BNXq2pATqrbgWmOlEbcRulfZ+7W0QRUg== dependencies: indexof "^0.0.1" From afb9f86bf7aa2f7b10814e215da1afc5ecf3993b Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 23 Jan 2025 17:23:08 +0000 Subject: [PATCH 08/34] Using node vm for string-template test cases. --- packages/string-templates/src/index.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts index bd008dd4d2..79b0aa0699 100644 --- a/packages/string-templates/src/index.ts +++ b/packages/string-templates/src/index.ts @@ -1,4 +1,5 @@ -import vm from "@budibase/vm-browserify" +import browserVM from "@budibase/vm-browserify" +import vm from "vm" import { create, TemplateDelegate } from "handlebars" import { registerAll, registerMinimum } from "./helpers/index" import { postprocess, postprocessWithLogs, preprocess } from "./processors" @@ -14,10 +15,10 @@ import { } from "./utilities" import { convertHBSBlock } from "./conversion" import { removeJSRunner, setJSRunner } from "./helpers/javascript" - import manifest from "./manifest.json" import { Log, ProcessOptions } from "./types" import { UserScriptError } from "./errors" +import { isTest } from "./environment" export type { Log, LogType } from "./types" export { setTestingBackendJS } from "./environment" @@ -507,15 +508,15 @@ export function convertToJS(hbs: string) { export { JsTimeoutError, UserScriptError } from "./errors" export function browserJSSetup() { - /** - * Use polyfilled vm to run JS scripts in a browser Env - */ + // tests are in jest - we need to use node VM for these + const jsSandbox = isTest() ? vm : browserVM + // Use polyfilled vm to run JS scripts in a browser Env setJSRunner((js: string, context: Record) => { - vm.createContext(context) + jsSandbox.createContext(context) const wrappedJs = frontendWrapJS(js) - const result = vm.runInNewContext(wrappedJs, context) + const result = jsSandbox.runInNewContext(wrappedJs, context) if (result.error) { throw new UserScriptError(result.error) } From bf05fea3edbeef0706dc6d28dbd74165ad87881b Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Fri, 24 Jan 2025 11:41:27 +0000 Subject: [PATCH 09/34] Removing while loop test - frontend JS cannot timeout. --- .../string-templates/test/javascript.spec.ts | 5 --- yarn.lock | 36 +++++-------------- 2 files changed, 8 insertions(+), 33 deletions(-) diff --git a/packages/string-templates/test/javascript.spec.ts b/packages/string-templates/test/javascript.spec.ts index 9134005acb..631fe828ae 100644 --- a/packages/string-templates/test/javascript.spec.ts +++ b/packages/string-templates/test/javascript.spec.ts @@ -125,11 +125,6 @@ describe("Javascript", () => { expect(processJS(`throw "Error"`)).toEqual("Error") }) - it("should timeout after one second", () => { - const output = processJS(`while (true) {}`) - expect(output).toBe("Timed out while executing JS") - }) - it("should prevent access to the process global", async () => { expect(processJS(`return process`)).toEqual( "ReferenceError: process is not defined" diff --git a/yarn.lock b/yarn.lock index a375c05ffd..8647f40b79 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11807,6 +11807,11 @@ husky@^8.0.3: resolved "https://registry.yarnpkg.com/husky/-/husky-8.0.3.tgz#4936d7212e46d1dea28fef29bb3a108872cd9184" integrity sha512-+dQSyqPh4x1hlO1swXBiNb2HzTDN1I2IGLQx1GrBuiqFJfoMrnZWwVmatvSiO+Iz8fBUnf+lekwNo4c2LlXItg== +husky@^9.1.4: + version "9.1.7" + resolved "https://registry.yarnpkg.com/husky/-/husky-9.1.7.tgz#d46a38035d101b46a70456a850ff4201344c0b2d" + integrity sha512-5gs5ytaNjBrh5Ow3zrvdUUY+0VxIuWVL4i9irt6friV+BqdCfmV11CQTWMiBYWHbXhco+J1kHfTOUkePhCDvMA== + ical-generator@4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/ical-generator/-/ical-generator-4.1.0.tgz#2a336c951864c5583a2aa715d16f2edcdfd2d90b" @@ -18658,16 +18663,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -18759,7 +18755,7 @@ stringify-object@^3.2.1: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -18773,13 +18769,6 @@ strip-ansi@^5.0.0, strip-ansi@^5.1.0, strip-ansi@^5.2.0: dependencies: ansi-regex "^4.1.0" -strip-ansi@^6.0.0, strip-ansi@^6.0.1: - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - strip-ansi@^7.0.1: version "7.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.0.1.tgz#61740a08ce36b61e50e65653f07060d000975fb2" @@ -20527,7 +20516,7 @@ worker-farm@1.7.0: dependencies: errno "~0.1.7" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -20545,15 +20534,6 @@ wrap-ansi@^5.1.0: string-width "^3.0.0" strip-ansi "^5.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From 8dd1366df27d82d15f3a05e71d4f6db57893408b Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Fri, 24 Jan 2025 13:02:00 +0000 Subject: [PATCH 10/34] Fixing yarn.lock (after removing account portal ref). --- yarn.lock | 5 ----- 1 file changed, 5 deletions(-) diff --git a/yarn.lock b/yarn.lock index 8647f40b79..60294cc5ab 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11807,11 +11807,6 @@ husky@^8.0.3: resolved "https://registry.yarnpkg.com/husky/-/husky-8.0.3.tgz#4936d7212e46d1dea28fef29bb3a108872cd9184" integrity sha512-+dQSyqPh4x1hlO1swXBiNb2HzTDN1I2IGLQx1GrBuiqFJfoMrnZWwVmatvSiO+Iz8fBUnf+lekwNo4c2LlXItg== -husky@^9.1.4: - version "9.1.7" - resolved "https://registry.yarnpkg.com/husky/-/husky-9.1.7.tgz#d46a38035d101b46a70456a850ff4201344c0b2d" - integrity sha512-5gs5ytaNjBrh5Ow3zrvdUUY+0VxIuWVL4i9irt6friV+BqdCfmV11CQTWMiBYWHbXhco+J1kHfTOUkePhCDvMA== - ical-generator@4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/ical-generator/-/ical-generator-4.1.0.tgz#2a336c951864c5583a2aa715d16f2edcdfd2d90b" From b8d38159d09659b4b7d04077385e1cbb4832b1ce Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 24 Jan 2025 14:48:27 +0100 Subject: [PATCH 11/34] Extract binding utils --- .../DataSourceSelect/DataSourceSelect.svelte | 43 ++-------------- packages/builder/src/helpers/bindings.ts | 50 +++++++++++++++++++ packages/builder/src/helpers/index.ts | 1 + packages/types/src/ui/bindings/binding.ts | 13 +++++ 4 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 packages/builder/src/helpers/bindings.ts diff --git a/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte b/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte index 1b9fdcdf10..6ccde7fede 100644 --- a/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte +++ b/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte @@ -31,6 +31,7 @@ import IntegrationQueryEditor from "@/components/integration/index.svelte" import { makePropSafe as safe } from "@budibase/string-templates" import { findAllComponents } from "@/helpers/components" + import { extractFields, extractRelationships } from "@/helpers/bindings" import ClientBindingPanel from "@/components/common/bindings/ClientBindingPanel.svelte" import DataSourceCategory from "@/components/design/settings/controls/DataSourceSelect/DataSourceCategory.svelte" import { API } from "@/api" @@ -81,46 +82,8 @@ value: `{{ literal ${safe(provider._id)} }}`, type: "provider", })) - $: links = bindings - // Get only link bindings - .filter(x => x.fieldSchema?.type === "link") - // Filter out bindings provided by forms - .filter(x => !x.component?.endsWith("/form")) - .map(binding => { - const { providerId, readableBinding, fieldSchema } = binding || {} - const { name, tableId } = fieldSchema || {} - const safeProviderId = safe(providerId) - return { - providerId, - label: readableBinding, - fieldName: name, - tableId, - type: "link", - // These properties will be enriched by the client library and provide - // details of the parent row of the relationship field, from context - rowId: `{{ ${safeProviderId}.${safe("_id")} }}`, - rowTableId: `{{ ${safeProviderId}.${safe("tableId")} }}`, - } - }) - $: fields = bindings - .filter( - x => - x.fieldSchema?.type === "attachment" || - (x.fieldSchema?.type === "array" && x.tableId) - ) - .map(binding => { - const { providerId, readableBinding, runtimeBinding } = binding - const { name, type, tableId } = binding.fieldSchema - return { - providerId, - label: readableBinding, - fieldName: name, - fieldType: type, - tableId, - type: "field", - value: `{{ literal ${runtimeBinding} }}`, - } - }) + $: links = extractRelationships(bindings) + $: fields = extractFields(bindings) $: jsonArrays = bindings .filter( x => diff --git a/packages/builder/src/helpers/bindings.ts b/packages/builder/src/helpers/bindings.ts new file mode 100644 index 0000000000..2d1410c76a --- /dev/null +++ b/packages/builder/src/helpers/bindings.ts @@ -0,0 +1,50 @@ +import { makePropSafe } from "@budibase/string-templates" +import { UIBinding } from "@budibase/types" + +export function extractRelationships(bindings: UIBinding[]) { + return ( + bindings + // Get only link bindings + .filter(x => x.fieldSchema?.type === "link") + // Filter out bindings provided by forms + .filter(x => !x.component?.endsWith("/form")) + .map(binding => { + const { providerId, readableBinding, fieldSchema } = binding || {} + const { name, tableId } = fieldSchema || {} + const safeProviderId = makePropSafe(providerId) + return { + providerId, + label: readableBinding, + fieldName: name, + tableId, + type: "link", + // These properties will be enriched by the client library and provide + // details of the parent row of the relationship field, from context + rowId: `{{ ${safeProviderId}.${makePropSafe("_id")} }}`, + rowTableId: `{{ ${safeProviderId}.${makePropSafe("tableId")} }}`, + } + }) + ) +} + +export function extractFields(bindings: UIBinding[]) { + return bindings + .filter( + x => + x.fieldSchema?.type === "attachment" || + (x.fieldSchema?.type === "array" && x.tableId) + ) + .map(binding => { + const { providerId, readableBinding, runtimeBinding } = binding + const { name, type, tableId } = binding.fieldSchema! + return { + providerId, + label: readableBinding, + fieldName: name, + fieldType: type, + tableId, + type: "field", + value: `{{ literal ${runtimeBinding} }}`, + } + }) +} diff --git a/packages/builder/src/helpers/index.ts b/packages/builder/src/helpers/index.ts index 81afc696a3..0e61eeb9c6 100644 --- a/packages/builder/src/helpers/index.ts +++ b/packages/builder/src/helpers/index.ts @@ -10,3 +10,4 @@ export { isBuilderInputFocused, } from "./helpers" export * as featureFlag from "./featureFlags" +export * as bindings from "./bindings" diff --git a/packages/types/src/ui/bindings/binding.ts b/packages/types/src/ui/bindings/binding.ts index 2cfb23ed2d..10e6968409 100644 --- a/packages/types/src/ui/bindings/binding.ts +++ b/packages/types/src/ui/bindings/binding.ts @@ -24,3 +24,16 @@ export type InsertAtPositionFn = (_: { value: string cursor?: { anchor: number } }) => void + +export interface UIBinding { + tableId?: string + fieldSchema?: { + name: string + tableId: string + type: string + } + component?: string + providerId: string + readableBinding?: string + runtimeBinding?: string +} From 5f3aaf458b593f0ecbab53553713249b4e60f7fc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 24 Jan 2025 14:50:06 +0100 Subject: [PATCH 12/34] Extract binding utils --- .../DataSourceSelect/DataSourceSelect.svelte | 28 ++++--------------- packages/builder/src/helpers/bindings.ts | 24 ++++++++++++++++ packages/types/src/ui/bindings/binding.ts | 2 ++ 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte b/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte index 6ccde7fede..f5028b1eea 100644 --- a/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte +++ b/packages/builder/src/components/design/settings/controls/DataSourceSelect/DataSourceSelect.svelte @@ -31,7 +31,11 @@ import IntegrationQueryEditor from "@/components/integration/index.svelte" import { makePropSafe as safe } from "@budibase/string-templates" import { findAllComponents } from "@/helpers/components" - import { extractFields, extractRelationships } from "@/helpers/bindings" + import { + extractFields, + extractJSONArrayFields, + extractRelationships, + } from "@/helpers/bindings" import ClientBindingPanel from "@/components/common/bindings/ClientBindingPanel.svelte" import DataSourceCategory from "@/components/design/settings/controls/DataSourceSelect/DataSourceCategory.svelte" import { API } from "@/api" @@ -84,27 +88,7 @@ })) $: links = extractRelationships(bindings) $: fields = extractFields(bindings) - $: jsonArrays = bindings - .filter( - x => - x.fieldSchema?.type === "jsonarray" || - (x.fieldSchema?.type === "json" && x.fieldSchema?.subtype === "array") - ) - .map(binding => { - const { providerId, readableBinding, runtimeBinding, tableId } = binding - const { name, type, prefixKeys, subtype } = binding.fieldSchema - return { - providerId, - label: readableBinding, - fieldName: name, - fieldType: type, - tableId, - prefixKeys, - type: type === "jsonarray" ? "jsonarray" : "queryarray", - subtype, - value: `{{ literal ${runtimeBinding} }}`, - } - }) + $: jsonArrays = extractJSONArrayFields(bindings) $: custom = { type: "custom", label: "JSON / CSV", diff --git a/packages/builder/src/helpers/bindings.ts b/packages/builder/src/helpers/bindings.ts index 2d1410c76a..66a13d9ba3 100644 --- a/packages/builder/src/helpers/bindings.ts +++ b/packages/builder/src/helpers/bindings.ts @@ -48,3 +48,27 @@ export function extractFields(bindings: UIBinding[]) { } }) } + +export function extractJSONArrayFields(bindings: UIBinding[]) { + return bindings + .filter( + x => + x.fieldSchema?.type === "jsonarray" || + (x.fieldSchema?.type === "json" && x.fieldSchema?.subtype === "array") + ) + .map(binding => { + const { providerId, readableBinding, runtimeBinding, tableId } = binding + const { name, type, prefixKeys, subtype } = binding.fieldSchema! + return { + providerId, + label: readableBinding, + fieldName: name, + fieldType: type, + tableId, + prefixKeys, + type: type === "jsonarray" ? "jsonarray" : "queryarray", + subtype, + value: `{{ literal ${runtimeBinding} }}`, + } + }) +} diff --git a/packages/types/src/ui/bindings/binding.ts b/packages/types/src/ui/bindings/binding.ts index 10e6968409..a770a25a3e 100644 --- a/packages/types/src/ui/bindings/binding.ts +++ b/packages/types/src/ui/bindings/binding.ts @@ -31,6 +31,8 @@ export interface UIBinding { name: string tableId: string type: string + subtype?: string + prefixKeys?: string } component?: string providerId: string From 08c4cfcec0a4c5ce3691b5dc87b243048d6a65e4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 10:21:31 +0100 Subject: [PATCH 13/34] Validate links --- .../src/stores/builder/screenComponent.ts | 32 +++++++++++++++---- packages/types/src/ui/datasource.ts | 8 ++++- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index d8169fdedb..aa6a854f77 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -6,7 +6,8 @@ import { findComponentsBySettingsType } from "@/helpers/screen" import { UIDatasourceType, Screen } from "@budibase/types" import { queries } from "./queries" import { views } from "./views" -import { featureFlag } from "@/helpers" +import { bindings, featureFlag } from "@/helpers" +import { screenComponentBindableProperties } from "./bindings" function reduceBy( key: TKey, @@ -31,14 +32,26 @@ const validationKeyByType: Record = { viewV2: "id", query: "_id", custom: null, + link: "rowId", } export const screenComponentErrors = derived( - [selectedScreen, tables, views, viewsV2, queries], - ([$selectedScreen, $tables, $views, $viewsV2, $queries]): Record< - string, - string[] - > => { + [ + selectedScreen, + tables, + views, + viewsV2, + queries, + screenComponentBindableProperties, + ], + ([ + $selectedScreen, + $tables, + $views, + $viewsV2, + $queries, + $screenComponentBindableProperties, + ]): Record => { if (!featureFlag.isEnabled("CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS")) { return {} } @@ -56,6 +69,9 @@ export const screenComponentErrors = derived( const type = componentSettings.type as UIDatasourceType const validationKey = validationKeyByType[type] + if (type === "link") { + debugger + } if (!validationKey) { continue } @@ -76,6 +92,10 @@ export const screenComponentErrors = derived( ...reduceBy("name", $views.list), ...reduceBy("id", $viewsV2.list), ...reduceBy("_id", $queries.list), + ...reduceBy( + "rowId", + bindings.extractRelationships($screenComponentBindableProperties) + ), } return getInvalidDatasources($selectedScreen, datasources) diff --git a/packages/types/src/ui/datasource.ts b/packages/types/src/ui/datasource.ts index 53740e8c4d..a121d929c8 100644 --- a/packages/types/src/ui/datasource.ts +++ b/packages/types/src/ui/datasource.ts @@ -1 +1,7 @@ -export type UIDatasourceType = "table" | "view" | "viewV2" | "query" | "custom" +export type UIDatasourceType = + | "table" + | "view" + | "viewV2" + | "query" + | "custom" + | "link" From 52330a30b84e8c231e92d03a020f05f5fc984eb9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 10:23:26 +0100 Subject: [PATCH 14/34] Remove debugger --- packages/builder/src/stores/builder/screenComponent.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index aa6a854f77..1af10e8a80 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -69,9 +69,6 @@ export const screenComponentErrors = derived( const type = componentSettings.type as UIDatasourceType const validationKey = validationKeyByType[type] - if (type === "link") { - debugger - } if (!validationKey) { continue } From 92606c6129458bd499320d6994b7ddae7f72cabc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 10:28:54 +0100 Subject: [PATCH 15/34] Validate in all screen --- .../src/stores/builder/screenComponent.ts | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 1af10e8a80..3afb96994f 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -7,12 +7,12 @@ import { UIDatasourceType, Screen } from "@budibase/types" import { queries } from "./queries" import { views } from "./views" import { bindings, featureFlag } from "@/helpers" -import { screenComponentBindableProperties } from "./bindings" +import { getBindableProperties } from "@/dataBinding" function reduceBy( key: TKey, list: TItem[] -) { +): Record { return list.reduce( (result, item) => ({ ...result, @@ -36,22 +36,11 @@ const validationKeyByType: Record = { } export const screenComponentErrors = derived( - [ - selectedScreen, - tables, - views, - viewsV2, - queries, - screenComponentBindableProperties, - ], - ([ - $selectedScreen, - $tables, - $views, - $viewsV2, - $queries, - $screenComponentBindableProperties, - ]): Record => { + [selectedScreen, tables, views, viewsV2, queries], + ([$selectedScreen, $tables, $views, $viewsV2, $queries]): Record< + string, + string[] + > => { if (!featureFlag.isEnabled("CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS")) { return {} } @@ -72,8 +61,21 @@ export const screenComponentErrors = derived( if (!validationKey) { continue } + + const componentBindings = getBindableProperties( + $selectedScreen, + component._id + ) + + const componentDatasources = { + ...reduceBy( + "rowId", + bindings.extractRelationships(componentBindings) + ), + } + const resourceId = componentSettings[validationKey] - if (!datasources[resourceId]) { + if (!{ ...datasources, ...componentDatasources }[resourceId]) { const friendlyTypeName = friendlyNameByType[type] ?? type result[component._id!] = [ `The ${friendlyTypeName} named "${label}" could not be found`, @@ -89,10 +91,6 @@ export const screenComponentErrors = derived( ...reduceBy("name", $views.list), ...reduceBy("id", $viewsV2.list), ...reduceBy("_id", $queries.list), - ...reduceBy( - "rowId", - bindings.extractRelationships($screenComponentBindableProperties) - ), } return getInvalidDatasources($selectedScreen, datasources) From f666b35bd181a9aa8f311f58e236344f3f91caef Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 11:48:00 +0100 Subject: [PATCH 16/34] Validate by field --- packages/builder/src/stores/builder/screenComponent.ts | 2 ++ packages/types/src/ui/datasource.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 3afb96994f..aecc27c4e5 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -33,6 +33,7 @@ const validationKeyByType: Record = { query: "_id", custom: null, link: "rowId", + field: "label", } export const screenComponentErrors = derived( @@ -72,6 +73,7 @@ export const screenComponentErrors = derived( "rowId", bindings.extractRelationships(componentBindings) ), + ...reduceBy("label", bindings.extractFields(componentBindings)), } const resourceId = componentSettings[validationKey] diff --git a/packages/types/src/ui/datasource.ts b/packages/types/src/ui/datasource.ts index a121d929c8..c6b1ed01d1 100644 --- a/packages/types/src/ui/datasource.ts +++ b/packages/types/src/ui/datasource.ts @@ -5,3 +5,4 @@ export type UIDatasourceType = | "query" | "custom" | "link" + | "field" From 73d7f985bfaa523715bb2eee0eb6b4120e594079 Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 27 Jan 2025 10:59:42 +0000 Subject: [PATCH 17/34] Do not proceed to validation if a screen hasn't been selected --- packages/builder/src/stores/builder/screenComponent.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index d8169fdedb..434fa27ae5 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -78,6 +78,11 @@ export const screenComponentErrors = derived( ...reduceBy("_id", $queries.list), } + if (!$selectedScreen) { + // Skip validation if a screen is not selected. + return {} + } + return getInvalidDatasources($selectedScreen, datasources) } ) From 1557b026826911ba2e87ec978698bbb04d91f5db Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 12:17:25 +0100 Subject: [PATCH 18/34] Use proper field --- packages/builder/src/stores/builder/screenComponent.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index aecc27c4e5..1c1070ad17 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -33,7 +33,7 @@ const validationKeyByType: Record = { query: "_id", custom: null, link: "rowId", - field: "label", + field: "value", } export const screenComponentErrors = derived( @@ -73,7 +73,7 @@ export const screenComponentErrors = derived( "rowId", bindings.extractRelationships(componentBindings) ), - ...reduceBy("label", bindings.extractFields(componentBindings)), + ...reduceBy("value", bindings.extractFields(componentBindings)), } const resourceId = componentSettings[validationKey] From f9dadf83a2914da94ea793a9fadf09b056d894e0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 12:16:07 +0100 Subject: [PATCH 19/34] Validate json arrays --- packages/builder/src/stores/builder/screenComponent.ts | 5 +++++ packages/types/src/ui/datasource.ts | 1 + 2 files changed, 6 insertions(+) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 1c1070ad17..4100e1e1fc 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -34,6 +34,7 @@ const validationKeyByType: Record = { custom: null, link: "rowId", field: "value", + jsonarray: "value", } export const screenComponentErrors = derived( @@ -74,6 +75,10 @@ export const screenComponentErrors = derived( bindings.extractRelationships(componentBindings) ), ...reduceBy("value", bindings.extractFields(componentBindings)), + ...reduceBy( + "value", + bindings.extractJSONArrayFields(componentBindings) + ), } const resourceId = componentSettings[validationKey] diff --git a/packages/types/src/ui/datasource.ts b/packages/types/src/ui/datasource.ts index c6b1ed01d1..8cc9b4ff94 100644 --- a/packages/types/src/ui/datasource.ts +++ b/packages/types/src/ui/datasource.ts @@ -6,3 +6,4 @@ export type UIDatasourceType = | "custom" | "link" | "field" + | "jsonarray" From 9c6ce76f682012dda63889c7b1a4a10f5278c390 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 13:45:35 +0100 Subject: [PATCH 20/34] Fix undefined reference --- packages/builder/src/stores/builder/screenComponent.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 3fdaf97660..226d233090 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -56,6 +56,9 @@ export const screenComponentErrors = derived( ["table", "dataSource"] )) { const componentSettings = component[setting.key] + if (!componentSettings) { + continue + } const { label } = componentSettings const type = componentSettings.type as UIDatasourceType From d4e63c07166b78efdd20752806ed5738a111c78e Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 27 Jan 2025 15:58:53 +0000 Subject: [PATCH 21/34] Type Checkbox.svelte. --- packages/bbui/src/Form/Core/Checkbox.svelte | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/bbui/src/Form/Core/Checkbox.svelte b/packages/bbui/src/Form/Core/Checkbox.svelte index 21fe00e864..0af9be87d9 100644 --- a/packages/bbui/src/Form/Core/Checkbox.svelte +++ b/packages/bbui/src/Form/Core/Checkbox.svelte @@ -1,22 +1,23 @@ -