From 02bf5974a9684cd1fe1752d2b6573da7e01c8463 Mon Sep 17 00:00:00 2001 From: Ivan Starkov Date: Wed, 9 Oct 2024 15:04:48 +0300 Subject: [PATCH] fix: Variable bindings on Class don't show on live site (#4253) ## Description closes #3683 ## Steps for reproduction Tests and fixtures ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file --- .../.webstudio/data.json | 71 ++++++++++++++- .../__generated__/$resources.sitemap.xml.ts | 12 +-- .../__generated__/[class-names]._index.tsx | 6 +- .../app/__generated__/index.css | 3 + fixtures/webstudio-remix-vercel/package.json | 2 +- .../src/component-generator.test.tsx | 89 ++++++++++++++++++- packages/react-sdk/src/component-generator.ts | 17 ++-- 7 files changed, 181 insertions(+), 19 deletions(-) diff --git a/fixtures/webstudio-remix-vercel/.webstudio/data.json b/fixtures/webstudio-remix-vercel/.webstudio/data.json index ef47814702..9cd945532a 100644 --- a/fixtures/webstudio-remix-vercel/.webstudio/data.json +++ b/fixtures/webstudio-remix-vercel/.webstudio/data.json @@ -1,10 +1,10 @@ { "build": { - "id": "40962d82-ea2d-4536-b94f-dece81312190", + "id": "6876de3a-942a-466b-9761-ad7cb2fa2c21", "projectId": "cddc1d44-af37-4cb6-a430-d300cf6f932d", - "version": 315, - "createdAt": "2024-09-30T13:22:48.689+00:00", - "updatedAt": "2024-09-30T13:22:48.689+00:00", + "version": 321, + "createdAt": "2024-10-08T17:47:04.557+00:00", + "updatedAt": "2024-10-08T17:47:04.557+00:00", "pages": { "meta": { "siteName": "KittyGuardedZone", @@ -1540,6 +1540,19 @@ "value": 300 } } + ], + [ + "g6Z7dAxi6LYB-XtI6SaRR:UoTkWyaFuTYJihS3MFYK5:marginTop:", + { + "breakpointId": "UoTkWyaFuTYJihS3MFYK5", + "styleSourceId": "g6Z7dAxi6LYB-XtI6SaRR", + "property": "marginTop", + "value": { + "type": "unit", + "value": 12, + "unit": "px" + } + } ] ], "styleSources": [ @@ -1717,6 +1730,13 @@ "type": "local", "id": "o9VRyqmvIarF6HrGMccU8" } + ], + [ + "g6Z7dAxi6LYB-XtI6SaRR", + { + "type": "local", + "id": "g6Z7dAxi6LYB-XtI6SaRR" + } ] ], "styleSourceSelections": [ @@ -1894,6 +1914,13 @@ "instanceId": "drCx7m8q_gnNQLrhPDA-g", "values": ["o9VRyqmvIarF6HrGMccU8"] } + ], + [ + "T-rqx2b12pRrWcafRRoIG", + { + "instanceId": "T-rqx2b12pRrWcafRRoIG", + "values": ["g6Z7dAxi6LYB-XtI6SaRR"] + } ] ], "props": [ @@ -2297,6 +2324,16 @@ "type": "string", "value": "\"broken'with`symbols" } + ], + [ + "I4cM2PzBX49KWxsKdGMd1", + { + "id": "I4cM2PzBX49KWxsKdGMd1", + "instanceId": "T-rqx2b12pRrWcafRRoIG", + "name": "className", + "type": "expression", + "value": "`${$ws$dataSource$FG6camSaR89vSBREAV9PT} class_3`" + } ] ], "dataSources": [ @@ -2463,6 +2500,19 @@ "scopeInstanceId": "mdyCS8QnKx3fL1MLXifmy", "name": "system" } + ], + [ + "FG6camSaR89vSBREAV9PT", + { + "type": "variable", + "id": "FG6camSaR89vSBREAV9PT", + "scopeInstanceId": "mdyCS8QnKx3fL1MLXifmy", + "name": "classVar", + "value": { + "type": "string", + "value": "varClass" + } + } ] ], "resources": [ @@ -3581,6 +3631,10 @@ { "type": "id", "value": "drCx7m8q_gnNQLrhPDA-g" + }, + { + "type": "id", + "value": "T-rqx2b12pRrWcafRRoIG" } ] } @@ -3593,6 +3647,15 @@ "component": "Box", "children": [] } + ], + [ + "T-rqx2b12pRrWcafRRoIG", + { + "type": "instance", + "id": "T-rqx2b12pRrWcafRRoIG", + "component": "Box", + "children": [] + } ] ], "deployment": { diff --git a/fixtures/webstudio-remix-vercel/app/__generated__/$resources.sitemap.xml.ts b/fixtures/webstudio-remix-vercel/app/__generated__/$resources.sitemap.xml.ts index 0422d75129..069d453c16 100644 --- a/fixtures/webstudio-remix-vercel/app/__generated__/$resources.sitemap.xml.ts +++ b/fixtures/webstudio-remix-vercel/app/__generated__/$resources.sitemap.xml.ts @@ -1,26 +1,26 @@ export const sitemap = [ { path: "/", - lastModified: "2024-09-30", + lastModified: "2024-10-08", }, { path: "/_route_with_symbols_", - lastModified: "2024-09-30", + lastModified: "2024-10-08", }, { path: "/form", - lastModified: "2024-09-30", + lastModified: "2024-10-08", }, { path: "/heading-with-id", - lastModified: "2024-09-30", + lastModified: "2024-10-08", }, { path: "/resources", - lastModified: "2024-09-30", + lastModified: "2024-10-08", }, { path: "/nested/nested-page", - lastModified: "2024-09-30", + lastModified: "2024-10-08", }, ]; diff --git a/fixtures/webstudio-remix-vercel/app/__generated__/[class-names]._index.tsx b/fixtures/webstudio-remix-vercel/app/__generated__/[class-names]._index.tsx index 0c101c31ef..40d2f6f44f 100644 --- a/fixtures/webstudio-remix-vercel/app/__generated__/[class-names]._index.tsx +++ b/fixtures/webstudio-remix-vercel/app/__generated__/[class-names]._index.tsx @@ -27,12 +27,16 @@ export const pageFontAssets: FontAsset[] = []; export const pageBackgroundImageAssets: ImageAsset[] = []; const Page = ({}: { system: any }) => { + let [classVar, set$classVar] = useVariableState("varClass"); return ( + ); }; diff --git a/fixtures/webstudio-remix-vercel/app/__generated__/index.css b/fixtures/webstudio-remix-vercel/app/__generated__/index.css index 4b2a3a1dab..ccce94d86f 100644 --- a/fixtures/webstudio-remix-vercel/app/__generated__/index.css +++ b/fixtures/webstudio-remix-vercel/app/__generated__/index.css @@ -385,4 +385,7 @@ .cm16yxw { width: 300px; } + .ctm310 { + margin-top: 12px; + } } diff --git a/fixtures/webstudio-remix-vercel/package.json b/fixtures/webstudio-remix-vercel/package.json index 1658c4e032..38ee34594f 100644 --- a/fixtures/webstudio-remix-vercel/package.json +++ b/fixtures/webstudio-remix-vercel/package.json @@ -6,7 +6,7 @@ "typecheck": "tsc", "cli": "NODE_OPTIONS='--conditions=webstudio --import=tsx' webstudio", "fixtures:link": "pnpm cli link --link https://p-cddc1d44-af37-4cb6-a430-d300cf6f932d-dot-${BUILDER_HOST:-main.development.webstudio.is}'?authToken=1cdc6026-dd5b-4624-b89b-9bd45e9bcc3d'", - "fixtures:sync": "pnpm cli sync --buildId 40962d82-ea2d-4536-b94f-dece81312190 && pnpm prettier --write ./.webstudio/", + "fixtures:sync": "pnpm cli sync --buildId 6876de3a-942a-466b-9761-ad7cb2fa2c21 && pnpm prettier --write ./.webstudio/", "fixtures:build": "pnpm cli build --template vercel --template internal --preview && pnpm prettier --write ./app/ ./package.json ./tsconfig.json" }, "private": true, diff --git a/packages/react-sdk/src/component-generator.test.tsx b/packages/react-sdk/src/component-generator.test.tsx index 96287bd94d..32cb58e08b 100644 --- a/packages/react-sdk/src/component-generator.test.tsx +++ b/packages/react-sdk/src/component-generator.test.tsx @@ -567,6 +567,30 @@ test("generate component with variables and actions", () => { ); }); +test("merge classes if no className", () => { + expect( + generateWebstudioComponent({ + classesMap: new Map([["body", ["cls1"]]]), + scope: createScope(), + name: "Page", + rootInstanceId: "body", + parameters: [], + dataSources: new Map(), + indexesWithinAncestors: new Map(), + ...renderJsx(<$.Body ws:id="body">), + }) + ).toEqual( + validateJSX( + clear(` + const Page = () => { + return + } + `) + ) + ); +}); + test("add classes and merge classes", () => { expect( generateWebstudioComponent({ @@ -584,7 +608,70 @@ test("add classes and merge classes", () => { clear(` const Page = () => { return + className={"cls1" + " " + "cls2 \\"cls3\\""} /> + } + `) + ) + ); +}); + +test("add classes", () => { + expect( + generateWebstudioComponent({ + classesMap: new Map(), + scope: createScope(), + name: "Page", + rootInstanceId: "body", + parameters: [], + dataSources: new Map(), + indexesWithinAncestors: new Map(), + ...renderJsx(<$.Body ws:id="body" className='cls2 "cls3"'>), + }) + ).toEqual( + validateJSX( + clear(` + const Page = () => { + return + } + `) + ) + ); +}); + +test("add bind classes and merge classes", () => { + expect( + generateWebstudioComponent({ + classesMap: new Map([["body", ["cls1"]]]), + scope: createScope(), + name: "Page", + rootInstanceId: "body", + parameters: [], + dataSources: toMap([ + { + type: "variable", + id: "variableId", + name: "variableName", + value: { type: "string", value: "cls3" }, + }, + ]), + indexesWithinAncestors: new Map(), + ...renderJsx( + <$.Body + ws:id="body" + className={ + new ExpressionValue(`'cls2' + ' ' + $ws$dataSource$variableId`) + } + > + ), + }) + ).toEqual( + validateJSX( + clear(` + const Page = () => { + let [variableName, set$variableName] = useVariableState("cls3") + return } `) ) diff --git a/packages/react-sdk/src/component-generator.ts b/packages/react-sdk/src/component-generator.ts index 1f7bf8846b..50f5fb9c2d 100644 --- a/packages/react-sdk/src/component-generator.ts +++ b/packages/react-sdk/src/component-generator.ts @@ -170,11 +170,17 @@ export const generateJsxElement = ({ let collectionDataValue: undefined | string; let collectionItemValue: undefined | string; - const classes = Array.from(classesMap?.get(instance.id) ?? []); + const classMapArray = classesMap?.get(instance.id); + const classes = + classMapArray !== undefined + ? [JSON.stringify(classMapArray.join(" "))] + : []; + for (const prop of props.values()) { if (prop.instanceId !== instance.id) { continue; } + const propValue = generatePropValue({ scope, prop, @@ -210,10 +216,9 @@ export const generateJsxElement = ({ continue; } // We need to merge atomic classes with user-defined className prop. - if (prop.name === "className") { - if (prop.type === "string") { - classes.push(prop.value); - } + if (prop.name === "className" && propValue !== undefined) { + classes.push(propValue); + continue; } if (propValue !== undefined) { @@ -222,7 +227,7 @@ export const generateJsxElement = ({ } if (classes.length !== 0) { - generatedProps += `\nclassName={${JSON.stringify(classes.join(" "))}}`; + generatedProps += `\nclassName={${classes.join(` + " " + `)}}`; } let generatedElement = "";