From 401bfa812cd9d464efe385e81d0bb02ec6bf7e97 Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 6 Jan 2025 11:45:53 +0000 Subject: [PATCH 1/6] 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} | 167 +++++++--- .../src/stores/builder/tests/screens.test.js | 299 +++++++++--------- packages/frontend-core/src/utils/utils.js | 2 +- 5 files changed, 299 insertions(+), 210 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 9ad9a75f84b..bce7fcb71d5 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 08d87bebf5b..0d4682b5516 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 8298a1469da..fd16cbfae8e 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" + +interface ScreenState { + screens: Screen[] + selectedScreenId?: string + selected?: Screen +} -export const INITIAL_SCREENS_STATE = { +export const initialScreenState: ScreenState = { screens: [], - selectedScreenId: null, } -export class ScreenStore extends BudiStore { +// 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 + // } + // 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 + // 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 430605b77af..87eb03ea046 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 c424aea5b20..eeff5612155 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 2/6] 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 fd16cbfae8e..a749ded1f90 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 3/6] 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 420c540e377..cec4e3a4a08 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 bd9e2c8d4da..a56fec2227d 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 4/6] 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 bce7fcb71d5..7eb351e9da2 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 a749ded1f90..5163c6a3ea3 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 87eb03ea046..ea916c8d59e 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 5/6] 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 7eb351e9da2..46d3e07eaea 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 73d7f985bfaa523715bb2eee0eb6b4120e594079 Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 27 Jan 2025 10:59:42 +0000 Subject: [PATCH 6/6] 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 d8169fdedb0..434fa27ae5f 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) } )