Deduplicate global variable by name

webstudio-is · Feb 14, 2025 · d0e05d5 · d0e05d5
1 parent 9e7aad0
commit d0e05d5
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 5 deletions.
diff --git a/apps/builder/app/shared/instance-utils.ts b/apps/builder/app/shared/instance-utils.ts
@@ -959,7 +959,10 @@ export const insertWebstudioFragmentCopy = ({
     const { scopeInstanceId } = dataSource;
     if (scopeInstanceId === ROOT_INSTANCE_ID) {
       // add global variable only if not exist already
-      if (dataSources.has(dataSource.id) === false) {
+      if (
+        dataSources.has(dataSource.id) === false &&
+        maskedIdByName.has(dataSource.name) === false
+      ) {
         dataSources.set(dataSource.id, dataSource);
       }
       continue;

diff --git a/apps/builder/app/shared/page-utils.test.tsx b/apps/builder/app/shared/page-utils.test.tsx
@@ -22,6 +22,7 @@ import {
   Variable,
   ws,
 } from "@webstudio-is/template";
+import { nanoid } from "nanoid";
 
 const toMap = <T extends { id: string }>(list: T[]) =>
   new Map(list.map((item) => [item.id, item]));
@@ -444,4 +445,96 @@ describe("insert page copy", () => {
       { type: "expression", value: encodeDataVariableId(globalVariableId) },
     ]);
   });
+
+  test("preserve existing global variables by name", () => {
+    const globalVariable = new Variable("globalVariable", "global value");
+    const sourceData = {
+      pages: createDefaultPages({
+        rootInstanceId: "bodyId",
+        systemDataSourceId: "",
+      }),
+      ...renderData(
+        <ws.root ws:id={ROOT_INSTANCE_ID} vars={expression`${globalVariable}`}>
+          <$.Body ws:id="bodyId">
+            <$.Box ws:id="boxId">{expression`${globalVariable}`}</$.Box>
+          </$.Body>
+        </ws.root>,
+        // generate different ids in source and data projects
+        nanoid
+      ),
+    };
+    sourceData.instances.delete(ROOT_INSTANCE_ID);
+    const targetData = {
+      pages: createDefaultPages({
+        rootInstanceId: "anotherBodyId",
+        systemDataSourceId: "",
+      }),
+      ...renderData(
+        <ws.root ws:id={ROOT_INSTANCE_ID} vars={expression`${globalVariable}`}>
+          <$.Body ws:id="anotherBodyId"></$.Body>
+        </ws.root>,
+        // generate different ids in source and data projects
+        nanoid
+      ),
+    };
+    targetData.instances.delete(ROOT_INSTANCE_ID);
+    insertPageCopyMutable({
+      source: { data: sourceData, pageId: sourceData.pages.homePage.id },
+      target: { data: targetData, folderId: ROOT_FOLDER_ID },
+    });
+    expect(targetData.dataSources.size).toEqual(1);
+    const [globalVariableId] = targetData.dataSources.keys();
+    expect(Array.from(targetData.instances.values())).toEqual([
+      expect.objectContaining({ component: "Body", id: "anotherBodyId" }),
+      expect.objectContaining({ component: "Body" }),
+      expect.objectContaining({ component: "Box" }),
+    ]);
+    const newBox = Array.from(targetData.instances.values()).at(-1);
+    expect(newBox?.children).toEqual([
+      { type: "expression", value: encodeDataVariableId(globalVariableId) },
+    ]);
+  });
+
+  test("restore newly added global variable by name", () => {
+    const globalVariable = new Variable("globalVariable", "global value");
+    const sourceData = {
+      pages: createDefaultPages({
+        rootInstanceId: "bodyId",
+        systemDataSourceId: "",
+      }),
+      ...renderData(
+        <ws.root ws:id={ROOT_INSTANCE_ID} vars={expression`${globalVariable}`}>
+          <$.Body ws:id="bodyId">
+            <$.Box ws:id="boxId">{expression`${globalVariable}`}</$.Box>
+          </$.Body>
+        </ws.root>,
+        // generate different ids in source and data projects
+        nanoid
+      ),
+    };
+    sourceData.instances.delete(ROOT_INSTANCE_ID);
+    const targetData = {
+      pages: createDefaultPages({
+        rootInstanceId: "anotherBodyId",
+        systemDataSourceId: "",
+      }),
+      // generate different ids in source and data projects
+      ...renderData(<$.Body ws:id="anotherBodyId"></$.Body>, nanoid),
+    };
+    insertPageCopyMutable({
+      source: { data: sourceData, pageId: sourceData.pages.homePage.id },
+      target: { data: targetData, folderId: ROOT_FOLDER_ID },
+    });
+    expect(targetData.dataSources.size).toEqual(1);
+    const [globalVariableId] = targetData.dataSources.keys();
+    expect(Array.from(targetData.instances.values())).toEqual([
+      expect.objectContaining({ component: "Body", id: "anotherBodyId" }),
+      expect.objectContaining({ component: "Body" }),
+      expect.objectContaining({ component: "Box" }),
+    ]);
+    const newBox = Array.from(targetData.instances.values()).at(-1);
+    expect(newBox?.children).toEqual([
+      { type: "expression", value: encodeDataVariableId(globalVariableId) },
+    ]);
+  });
 });
diff --git a/packages/template/src/jsx.ts b/packages/template/src/jsx.ts
@@ -142,7 +142,10 @@ const traverseJsx = (
   return result;
 };
 
-export const renderTemplate = (root: JSX.Element): WebstudioFragment => {
+export const renderTemplate = (
+  root: JSX.Element,
+  generateId?: () => string
+): WebstudioFragment => {
   let lastId = -1;
   const instances: Instance[] = [];
   const props: Prop[] = [];
@@ -157,7 +160,7 @@ export const renderTemplate = (root: JSX.Element): WebstudioFragment => {
     let id = ids.get(key);
     if (id === undefined) {
       lastId += 1;
-      id = lastId.toString();
+      id = generateId?.() ?? lastId.toString();
       ids.set(key, id);
     }
     return id;
@@ -370,7 +373,10 @@ export const renderTemplate = (root: JSX.Element): WebstudioFragment => {
   };
 };
 
-export const renderData = (root: JSX.Element): Omit<WebstudioData, "pages"> => {
+export const renderData = (
+  root: JSX.Element,
+  generateId?: () => string
+): Omit<WebstudioData, "pages"> => {
   const {
     instances,
     props,
@@ -381,7 +387,7 @@ export const renderData = (root: JSX.Element): Omit<WebstudioData, "pages"> => {
     dataSources,
     resources,
     assets,
-  } = renderTemplate(root);
+  } = renderTemplate(root, generateId);
   return {
     instances: new Map(instances.map((item) => [item.id, item])),
     props: new Map(props.map((item) => [item.id, item])),