From 10ea732ee27251fe6c3fbe4c127cae030cc45f67 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Mon, 13 Feb 2023 12:42:02 +0300 Subject: [PATCH] Resolve props assets on client (#1005) Before we checked looked for all asset ids in props, loaded assets and replaced ids with resolved assets. This is all on server. We faced a few problems with it. - requires to maintain two versions of types: stored in db and resolved for client - easy to break project by writing to db wrong version Here I provide to sdk both props and assets and resolve on client with almost same amount of code. Also replaced store setter with context, see https://github.com/webstudio-is/webstudio-designer/issues/213 --- apps/designer/app/canvas/canvas.tsx | 16 ++-- .../designer/features/props-panel/control.tsx | 37 ++++++--- .../app/shared/css-utils/generate-css-text.ts | 10 ++- .../app/shared/nano-states/nano-states.ts | 5 +- packages/project-build/src/schema/props.ts | 27 ++---- packages/project/src/db/props.ts | 82 ++----------------- packages/project/src/db/tree.ts | 7 +- .../project/src/shared/styles/global-rules.ts | 12 ++- packages/react-sdk/src/context.tsx | 11 +++ packages/react-sdk/src/index.ts | 1 - packages/react-sdk/src/props.ts | 40 ++++----- .../src/tree/create-elements-tree.tsx | 13 ++- packages/react-sdk/src/tree/root.ts | 10 +-- 13 files changed, 112 insertions(+), 159 deletions(-) create mode 100644 packages/react-sdk/src/context.tsx diff --git a/apps/designer/app/canvas/canvas.tsx b/apps/designer/app/canvas/canvas.tsx index 1e81d1e5b6..c051298c37 100644 --- a/apps/designer/app/canvas/canvas.tsx +++ b/apps/designer/app/canvas/canvas.tsx @@ -9,7 +9,6 @@ import { customComponentsMeta, setParams, type OnChangeChildren, - setPropsByInstanceIdStore, } from "@webstudio-is/react-sdk"; import { publish } from "~/shared/pubsub"; import { registerContainers, useCanvasStore } from "~/shared/sync"; @@ -21,6 +20,7 @@ import { useTrackSelectedElement } from "./shared/use-track-selected-element"; import { WrapperComponentDev } from "./features/wrapper-component"; import { propsIndexStore, + assetsStore, rootInstanceContainer, useBreakpoints, useRootInstance, @@ -45,6 +45,11 @@ import { useHoveredInstanceConnector } from "./hovered-instance-connector"; registerContainers(); +const propsByInstanceIdStore = computed( + propsIndexStore, + (propsIndex) => propsIndex.propsByInstanceId +); + const useElementsTree = () => { const [rootInstance] = useRootInstance(); const [breakpoints] = useBreakpoints(); @@ -76,6 +81,8 @@ const useElementsTree = () => { return createElementsTree({ sandbox: true, instance: rootInstance, + propsByInstanceIdStore, + assetsStore, Component: WrapperComponentDev, onChangeChildren, }); @@ -101,11 +108,6 @@ type CanvasProps = { data: CanvasData; }; -const propsByInstanceIdStore = computed( - propsIndexStore, - (propsIndex) => propsIndex.propsByInstanceId -); - export const Canvas = ({ data }: CanvasProps): JSX.Element | null => { if (data.build === null) { throw new Error("Build is null"); @@ -118,8 +120,6 @@ export const Canvas = ({ data }: CanvasProps): JSX.Element | null => { useSetAssets(data.assets); useSetBreakpoints(data.build.breakpoints); useSetProps(data.tree.props); - // inject props store to sdk - setPropsByInstanceIdStore(propsByInstanceIdStore); useSetStyles(data.build.styles); useSetStyleSources(data.build.styleSources); useSetStyleSourceSelections(data.tree.styleSourceSelections); diff --git a/apps/designer/app/designer/features/props-panel/control.tsx b/apps/designer/app/designer/features/props-panel/control.tsx index 71ebd592bd..1730ab6fce 100644 --- a/apps/designer/app/designer/features/props-panel/control.tsx +++ b/apps/designer/app/designer/features/props-panel/control.tsx @@ -1,19 +1,23 @@ +import { computed } from "nanostores"; +import warnOnce from "warn-once"; +import { useMemo } from "react"; +import { useStore } from "@nanostores/react"; import type { Instance, Prop } from "@webstudio-is/project-build"; import { getComponentMetaProps } from "@webstudio-is/react-sdk"; -import warnOnce from "warn-once"; +import type { Asset } from "@webstudio-is/asset-uploader"; import { Flex, DeprecatedLabel, + Checkbox, Radio, RadioGroup, Select, Switch, TextField, } from "@webstudio-is/design-system"; -import { FloatingPanel } from "~/designer/shared/floating-panel/"; +import { assetsStore } from "~/shared/nano-states"; +import { FloatingPanel } from "~/designer/shared/floating-panel"; import { ImageManager } from "~/designer/shared/image-manager"; -import { Checkbox } from "@webstudio-is/design-system"; -import { Asset } from "@webstudio-is/asset-uploader"; import type { UserPropValue } from "./use-props-logic"; import { type SetProperty } from "../style-panel/shared/use-style-data"; @@ -143,11 +147,20 @@ const BooleanControl = ({ ); type ImageControlProps = { - asset: Asset | null; + assetId: undefined | Asset["id"]; onChange: (asset: Asset) => void; }; -const ImageControl = ({ asset, onChange }: ImageControlProps) => { +const ImageControl = ({ assetId, onChange }: ImageControlProps) => { + const assetStore = useMemo(() => { + return computed(assetsStore, (assets) => { + if (assetId === undefined) { + return undefined; + } + return assets.get(assetId); + }); + }, [assetId]); + const asset = useStore(assetStore); return ( { - onChangePropValue({ type: "asset", value }); + assetId={assetId} + onChange={(asset) => { + onChangePropValue({ type: "asset", value: asset.id }); - const { meta } = value; + const { meta } = asset; if ("width" in meta && "height" in meta) { // @todo: change on own type, pass width/hight separately const aspectRatio = meta.width / meta.height; @@ -243,7 +256,7 @@ export function Control({ if (userProp.type === "asset") { throw new Error( - `userProp with id "${userProp.value.id}" has asset but no control exists to process it` + `userProp with id "${userProp.value}" has asset but no control exists to process it` ); } diff --git a/apps/designer/app/shared/css-utils/generate-css-text.ts b/apps/designer/app/shared/css-utils/generate-css-text.ts index faa298883c..3af2ea2f69 100644 --- a/apps/designer/app/shared/css-utils/generate-css-text.ts +++ b/apps/designer/app/shared/css-utils/generate-css-text.ts @@ -1,15 +1,16 @@ import { json } from "@remix-run/node"; import { db } from "@webstudio-is/project/server"; import { addGlobalRules, getStyleRules } from "@webstudio-is/project"; -import { loadCanvasData } from "~/shared/db"; import { createCssEngine } from "@webstudio-is/css-engine"; import { getComponentMeta, getComponentNames, idAttribute, } from "@webstudio-is/react-sdk"; -import type { BuildParams } from "../router-utils"; import type { AppContext } from "@webstudio-is/trpc-interface/server"; +import type { Asset } from "@webstudio-is/asset-uploader"; +import { loadCanvasData } from "~/shared/db"; +import type { BuildParams } from "../router-utils"; export const generateCssText = async ( buildParams: BuildParams, @@ -37,7 +38,10 @@ export const generateCssText = async ( const engine = createCssEngine({ name: "ssr" }); - addGlobalRules(engine, canvasData); + const assets = new Map( + canvasData.assets.map((asset) => [asset.id, asset]) + ); + addGlobalRules(engine, { assets }); for (const breakpoint of canvasData.build?.breakpoints ?? []) { engine.addMediaRule(breakpoint.id, breakpoint); diff --git a/apps/designer/app/shared/nano-states/nano-states.ts b/apps/designer/app/shared/nano-states/nano-states.ts index 8890cf2593..fbac8eb89b 100644 --- a/apps/designer/app/shared/nano-states/nano-states.ts +++ b/apps/designer/app/shared/nano-states/nano-states.ts @@ -208,14 +208,15 @@ export const assetContainersStore = atom< >([]); export const assetsStore = computed(assetContainersStore, (assetContainers) => { - const assets: Asset[] = []; + const assets = new Map(); for (const assetContainer of assetContainers) { if (assetContainer.status === "uploaded") { - assets.push(assetContainer.asset); + assets.set(assetContainer.asset.id, assetContainer.asset); } } return assets; }); +export const assetsIndex = computed; export const useSetAssets = (assets: Asset[]) => { useSyncInitializeOnce(() => { diff --git a/packages/project-build/src/schema/props.ts b/packages/project-build/src/schema/props.ts index 5e26bdb370..8dd8ac6182 100644 --- a/packages/project-build/src/schema/props.ts +++ b/packages/project-build/src/schema/props.ts @@ -1,5 +1,4 @@ import { z } from "zod"; -import { Asset } from "@webstudio-is/asset-uploader"; const PropId = z.string(); @@ -10,7 +9,7 @@ const baseProp = { required: z.optional(z.boolean()), }; -const SharedProp = z.union([ +export const Prop = z.union([ z.object({ ...baseProp, type: z.literal("number"), @@ -26,35 +25,19 @@ const SharedProp = z.union([ type: z.literal("boolean"), value: z.boolean(), }), -]); - -export const StoredProp = z.union([ - SharedProp, z.object({ ...baseProp, type: z.literal("asset"), - // asset.id is stored in database value: z.string(), }), ]); -export type StoredProp = z.infer; - -export const StoredProps = z.array(StoredProp); - -export type StoredProps = z.infer; - -export const Prop = z.union([ - SharedProp, - z.object({ - ...baseProp, - type: z.literal("asset"), - value: Asset, - }), -]); - export type Prop = z.infer; +export const PropsList = z.array(Prop); + +export type PropsList = z.infer; + export const Props = z.map(PropId, Prop); export type Props = z.infer; diff --git a/packages/project/src/db/props.ts b/packages/project/src/db/props.ts index c519575eeb..4e5778ffd2 100644 --- a/packages/project/src/db/props.ts +++ b/packages/project/src/db/props.ts @@ -1,5 +1,4 @@ -import warnOnce from "warn-once"; -import { type Tree, StoredProps, Props } from "@webstudio-is/project-build"; +import { type Tree, Props, PropsList } from "@webstudio-is/project-build"; import { applyPatches, type Patch } from "immer"; import { prisma } from "@webstudio-is/prisma-client"; import type { Project } from "../shared/schema"; @@ -7,77 +6,15 @@ import { authorizeProject, type AppContext, } from "@webstudio-is/trpc-interface/server"; -import { loadByIds } from "@webstudio-is/asset-uploader/server"; -export const parseProps = async ( - props: { propsString: string; projectId: Project["id"] }, - context: AppContext -) => { - const storedProps = StoredProps.parse(JSON.parse(props.propsString)); - - // find all asset ids in all props - const assetIds: string[] = []; - - for (const prop of storedProps) { - if (prop.type === "asset") { - assetIds.push(prop.value); - } - } - - // load all assets - const assets = await loadByIds( - { ids: assetIds, projectId: props.projectId }, - context - ); - - const assetsMap = new Map(assets.map((asset) => [asset.id, asset])); - - const convertedProps: Props = new Map(); - for (const prop of storedProps) { - if (prop.type === "asset") { - const assetId = prop.value; - const asset = assetsMap.get(assetId); - - if (asset) { - convertedProps.set(prop.id, { - id: prop.id, - instanceId: prop.instanceId, - name: prop.name, - required: prop.required, - type: prop.type, - value: asset, - }); - - continue; - } - - warnOnce(true, `Asset with assetId "${assetId}" not found`); - continue; - } - - convertedProps.set(prop.id, prop); - } - - return convertedProps; +export const parseProps = (propsString: string): Props => { + const propsList = PropsList.parse(JSON.parse(propsString)); + return new Map(propsList.map((prop) => [prop.id, prop])); }; export const serializeProps = (props: Props) => { - const storedProps: StoredProps = []; - for (const prop of props.values()) { - if (prop.type === "asset") { - storedProps.push({ - id: prop.id, - instanceId: prop.instanceId, - name: prop.name, - required: prop.required, - type: prop.type, - value: prop.value.id, - }); - continue; - } - storedProps.push(prop); - } - return JSON.stringify(storedProps); + const propsList: PropsList = Array.from(props.values()); + return JSON.stringify(propsList); }; export const patch = async ( @@ -102,11 +39,8 @@ export const patch = async ( if (tree === null) { return; } - const props = await parseProps( - { propsString: tree.props, projectId }, - context - ); - const patchedProps = applyPatches(props, patches); + const props = parseProps(tree.props); + const patchedProps = Props.parse(applyPatches(props, patches)); await prisma.tree.update({ data: { diff --git a/packages/project/src/db/tree.ts b/packages/project/src/db/tree.ts index 00f884f341..caa77cc033 100644 --- a/packages/project/src/db/tree.ts +++ b/packages/project/src/db/tree.ts @@ -126,7 +126,7 @@ const denormalizeTree = (instances: z.infer) => { export const loadById = async ( { projectId, treeId }: { projectId: Project["id"]; treeId: Tree["id"] }, - context: AppContext, + _context: AppContext, client: Prisma.TransactionClient = prisma ): Promise => { const tree = await client.tree.findUnique({ @@ -141,10 +141,7 @@ export const loadById = async ( const instances = Instances.parse(JSON.parse(tree.instances)); const root = Instance.parse(denormalizeTree(instances)); - const props = await parseProps( - { propsString: tree.props, projectId }, - context - ); + const props = parseProps(tree.props); const styleSourceSelections = parseStyleSourceSelections( tree.styleSelections diff --git a/packages/project/src/shared/styles/global-rules.ts b/packages/project/src/shared/styles/global-rules.ts index 5c13dd0729..2323e6736d 100644 --- a/packages/project/src/shared/styles/global-rules.ts +++ b/packages/project/src/shared/styles/global-rules.ts @@ -8,16 +8,20 @@ import { export const addGlobalRules = ( engine: CssEngine, - { assets = [] }: { assets?: Array } + { assets }: { assets: Map } ) => { // @todo we need to figure out all global resets while keeping // the engine aware of all of them. // Ideally, the user is somehow aware and in control of the reset engine.addPlaintextRule("html {margin: 0; height: 100%}"); - const fontAssets = assets.filter((asset) => - FONT_FORMATS.has(asset.format as FontFormat) - ) as Array; + const fontAssets: Array = []; + for (const asset of assets.values()) { + if (FONT_FORMATS.has(asset.format as FontFormat)) { + fontAssets.push(asset as FontAsset); + } + } + const fontFaces = getFontFaces(fontAssets); for (const fontFace of fontFaces) { engine.addFontFaceRule(fontFace); diff --git a/packages/react-sdk/src/context.tsx b/packages/react-sdk/src/context.tsx new file mode 100644 index 0000000000..2b1e62b81d --- /dev/null +++ b/packages/react-sdk/src/context.tsx @@ -0,0 +1,11 @@ +import { type ReadableAtom, atom } from "nanostores"; +import { createContext } from "react"; +import type { Assets, PropsByInstanceId } from "./props"; + +export const ReactSdkContext = createContext<{ + propsByInstanceIdStore: ReadableAtom; + assetsStore: ReadableAtom; +}>({ + propsByInstanceIdStore: atom(new Map()), + assetsStore: atom(new Map()), +}); diff --git a/packages/react-sdk/src/index.ts b/packages/react-sdk/src/index.ts index 3a0c921dff..e4013477ee 100644 --- a/packages/react-sdk/src/index.ts +++ b/packages/react-sdk/src/index.ts @@ -8,4 +8,3 @@ export { customComponentsMeta, } from "./app/custom-components"; export type { MetaProps, WsComponentMeta } from "./components/component-type"; -export { setPropsByInstanceIdStore } from "./props"; diff --git a/packages/react-sdk/src/props.ts b/packages/react-sdk/src/props.ts index ccd7026bb1..b69ac51fc0 100644 --- a/packages/react-sdk/src/props.ts +++ b/packages/react-sdk/src/props.ts @@ -1,22 +1,13 @@ -import { useMemo } from "react"; -import { atom, computed, type ReadableAtom } from "nanostores"; +import { useContext, useMemo } from "react"; +import { computed } from "nanostores"; import { useStore } from "@nanostores/react"; import type { Instance, Prop, Props } from "@webstudio-is/project-build"; import type { Asset } from "@webstudio-is/asset-uploader"; +import { ReactSdkContext } from "./context"; -type PropsByInstanceId = Map; +export type PropsByInstanceId = Map; -type PropsByInstanceIdStore = ReadableAtom; - -let propsByInstanceIdStore: PropsByInstanceIdStore = atom(new Map()); - -export const setPropsByInstanceIdStore = (store: PropsByInstanceIdStore) => { - propsByInstanceIdStore = store; -}; - -export const getPropsByInstanceIdStore = () => { - return propsByInstanceIdStore; -}; +export type Assets = Map; export const getPropsByInstanceId = (props: Props) => { const propsByInstanceId: PropsByInstanceId = new Map(); @@ -34,6 +25,7 @@ export const getPropsByInstanceId = (props: Props) => { // this utility is be used only for preview with static props // so there is no need to use computed to optimize rerenders export const useInstanceProps = (instanceId: Instance["id"]) => { + const { propsByInstanceIdStore } = useContext(ReactSdkContext); const propsByInstanceId = useStore(propsByInstanceIdStore); const instanceProps = propsByInstanceId.get(instanceId); const instancePropsObject: Record = {}; @@ -50,20 +42,24 @@ export const useInstanceProps = (instanceId: Instance["id"]) => { // this utility is be used for image component in both designer and preview // so need to optimize rerenders with computed export const usePropAsset = (instanceId: Instance["id"], name: string) => { + const { propsByInstanceIdStore, assetsStore } = useContext(ReactSdkContext); const assetStore = useMemo(() => { - return computed(propsByInstanceIdStore, (propsByInstanceId) => { - const instanceProps = propsByInstanceId.get(instanceId); - let asset: undefined | Asset = undefined; - if (instanceProps) { + return computed( + [propsByInstanceIdStore, assetsStore], + (propsByInstanceId, assets) => { + const instanceProps = propsByInstanceId.get(instanceId); + if (instanceProps === undefined) { + return undefined; + } for (const prop of instanceProps) { if (prop.type === "asset" && prop.name === name) { - asset = prop.value; + const assetId = prop.value; + return assets.get(assetId); } } } - return asset; - }); - }, [instanceId, name]); + ); + }, [propsByInstanceIdStore, assetsStore, instanceId, name]); const asset = useStore(assetStore); return asset; }; diff --git a/packages/react-sdk/src/tree/create-elements-tree.tsx b/packages/react-sdk/src/tree/create-elements-tree.tsx index fb029e2939..c4e59aa32d 100644 --- a/packages/react-sdk/src/tree/create-elements-tree.tsx +++ b/packages/react-sdk/src/tree/create-elements-tree.tsx @@ -1,6 +1,9 @@ import { type ComponentProps, Fragment } from "react"; +import { ReadableAtom } from "nanostores"; import { Scripts, ScrollRestoration } from "@remix-run/react"; import type { Instance } from "@webstudio-is/project-build"; +import { ReactSdkContext } from "../context"; +import { Assets, PropsByInstanceId } from "../props"; import { WrapperComponent } from "./wrapper-component"; import { SessionStoragePolyfill } from "./session-storage-polyfill"; @@ -25,11 +28,15 @@ export type OnChangeChildren = (change: { export const createElementsTree = ({ sandbox, instance, + propsByInstanceIdStore, + assetsStore, Component, onChangeChildren, }: { sandbox?: boolean; instance: Instance; + propsByInstanceIdStore: ReadableAtom; + assetsStore: ReadableAtom; Component: (props: ComponentProps) => JSX.Element; onChangeChildren?: OnChangeChildren; }) => { @@ -50,7 +57,11 @@ export const createElementsTree = ({ , ], }); - return body; + return ( + + {body} + + ); }; const createInstanceChildrenElements = ({ diff --git a/packages/react-sdk/src/tree/root.ts b/packages/react-sdk/src/tree/root.ts index 0d959df45f..6ff98d6ad3 100644 --- a/packages/react-sdk/src/tree/root.ts +++ b/packages/react-sdk/src/tree/root.ts @@ -7,7 +7,7 @@ import { WrapperComponent } from "./wrapper-component"; import { registerComponents } from "../components"; import { customComponents as defaultCustomComponents } from "../app/custom-components"; import { setParams, type Params } from "../app/params"; -import { getPropsByInstanceId, setPropsByInstanceIdStore } from "../props"; +import { getPropsByInstanceId } from "../props"; export type Data = { tree: Tree | null; @@ -30,16 +30,16 @@ export const InstanceRoot = ({ throw new Error("Tree is null"); } - setPropsByInstanceIdStore( - atom(getPropsByInstanceId(new Map(data.tree.props))) - ); - setParams(data.params ?? null); registerComponents(customComponents); return createElementsTree({ instance: data.tree.root, + propsByInstanceIdStore: atom( + getPropsByInstanceId(new Map(data.tree.props)) + ), + assetsStore: atom(new Map(data.assets.map((asset) => [asset.id, asset]))), Component: Component ?? WrapperComponent, }); };