Skip to content

Commit c3ac4d6

Browse files
TrySoundTrySound
authored
fix: preserve global variables when duplicate page (#4875)
Missed the case. Global variables were recreated and got invalid state. Now global variables are reused or added without changes.
1 parent 8ed3fbb commit c3ac4d6

File tree

3 files changed

+156
-9
lines changed

3 files changed

+156
-9
lines changed

‎apps/builder/app/shared/instance-utils.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -770,11 +770,6 @@ export const insertWebstudioFragmentCopy = ({
770770
}
771771
}
772772

773-
const fragmentDataSources: DataSources = new Map();
774-
for (const dataSource of fragment.dataSources) {
775-
fragmentDataSources.set(dataSource.id, dataSource);
776-
}
777-
778773
const {
779774
assets,
780775
instances,
@@ -962,6 +957,16 @@ export const insertWebstudioFragmentCopy = ({
962957
const newResourceIds = new Map<Resource["id"], Resource["id"]>();
963958
for (let dataSource of fragment.dataSources) {
964959
const { scopeInstanceId } = dataSource;
960+
if (scopeInstanceId === ROOT_INSTANCE_ID) {
961+
// add global variable only if not exist already
962+
if (
963+
dataSources.has(dataSource.id) === false &&
964+
maskedIdByName.has(dataSource.name) === false
965+
) {
966+
dataSources.set(dataSource.id, dataSource);
967+
}
968+
continue;
969+
}
965970
// insert only data sources within portal content
966971
if (fragmentInstanceIds.has(scopeInstanceId)) {
967972
const newDataSourceId = nanoid();

‎apps/builder/app/shared/page-utils.test.ts‎ renamed to ‎apps/builder/app/shared/page-utils.test.tsx‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { describe, expect, test } from "vitest";
22
import type { Project } from "@webstudio-is/project";
33
import {
44
ROOT_FOLDER_ID,
5+
ROOT_INSTANCE_ID,
56
encodeDataSourceVariable,
7+
encodeDataVariableId,
68
type DataSource,
79
type Instance,
810
type WebstudioData,
@@ -13,6 +15,14 @@ import {
1315
} from "@webstudio-is/project-build";
1416
import { $project } from "./nano-states";
1517
import { insertPageCopyMutable } from "./page-utils";
18+
import {
19+
$,
20+
expression,
21+
renderData,
22+
Variable,
23+
ws,
24+
} from "@webstudio-is/template";
25+
import { nanoid } from "nanoid";
1626

1727
const toMap = <T extends { id: string }>(list: T[]) =>
1828
new Map(list.map((item) => [item.id, item]));
@@ -401,4 +411,130 @@ describe("insert page copy", () => {
401411
expect(data.pages.pages[2].name).toEqual(`Name`);
402412
expect(data.pages.pages[3].name).toEqual(`Name (1)`);
403413
});
414+
415+
test("preserve global variables when duplicate page", () => {
416+
const globalVariable = new Variable("globalVariable", "global value");
417+
const data = {
418+
pages: createDefaultPages({
419+
rootInstanceId: "bodyId",
420+
systemDataSourceId: "",
421+
}),
422+
...renderData(
423+
<ws.root ws:id={ROOT_INSTANCE_ID} vars={expression`${globalVariable}`}>
424+
<$.Body ws:id="bodyId">
425+
<$.Box ws:id="boxId">{expression`${globalVariable}`}</$.Box>
426+
</$.Body>
427+
</ws.root>
428+
),
429+
};
430+
data.instances.delete(ROOT_INSTANCE_ID);
431+
insertPageCopyMutable({
432+
source: { data, pageId: data.pages.homePage.id },
433+
target: { data, folderId: ROOT_FOLDER_ID },
434+
});
435+
expect(data.dataSources.size).toEqual(1);
436+
const [globalVariableId] = data.dataSources.keys();
437+
expect(Array.from(data.instances.values())).toEqual([
438+
expect.objectContaining({ component: "Body", id: "bodyId" }),
439+
expect.objectContaining({ component: "Box", id: "boxId" }),
440+
expect.objectContaining({ component: "Body" }),
441+
expect.objectContaining({ component: "Box" }),
442+
]);
443+
const newBox = Array.from(data.instances.values()).at(-1);
444+
expect(newBox?.children).toEqual([
445+
{ type: "expression", value: encodeDataVariableId(globalVariableId) },
446+
]);
447+
});
448+
449+
test("preserve existing global variables by name", () => {
450+
const globalVariable = new Variable("globalVariable", "global value");
451+
const sourceData = {
452+
pages: createDefaultPages({
453+
rootInstanceId: "bodyId",
454+
systemDataSourceId: "",
455+
}),
456+
...renderData(
457+
<ws.root ws:id={ROOT_INSTANCE_ID} vars={expression`${globalVariable}`}>
458+
<$.Body ws:id="bodyId">
459+
<$.Box ws:id="boxId">{expression`${globalVariable}`}</$.Box>
460+
</$.Body>
461+
</ws.root>,
462+
// generate different ids in source and data projects
463+
nanoid
464+
),
465+
};
466+
sourceData.instances.delete(ROOT_INSTANCE_ID);
467+
const targetData = {
468+
pages: createDefaultPages({
469+
rootInstanceId: "anotherBodyId",
470+
systemDataSourceId: "",
471+
}),
472+
...renderData(
473+
<ws.root ws:id={ROOT_INSTANCE_ID} vars={expression`${globalVariable}`}>
474+
<$.Body ws:id="anotherBodyId"></$.Body>
475+
</ws.root>,
476+
// generate different ids in source and data projects
477+
nanoid
478+
),
479+
};
480+
targetData.instances.delete(ROOT_INSTANCE_ID);
481+
insertPageCopyMutable({
482+
source: { data: sourceData, pageId: sourceData.pages.homePage.id },
483+
target: { data: targetData, folderId: ROOT_FOLDER_ID },
484+
});
485+
expect(targetData.dataSources.size).toEqual(1);
486+
const [globalVariableId] = targetData.dataSources.keys();
487+
expect(Array.from(targetData.instances.values())).toEqual([
488+
expect.objectContaining({ component: "Body", id: "anotherBodyId" }),
489+
expect.objectContaining({ component: "Body" }),
490+
expect.objectContaining({ component: "Box" }),
491+
]);
492+
const newBox = Array.from(targetData.instances.values()).at(-1);
493+
expect(newBox?.children).toEqual([
494+
{ type: "expression", value: encodeDataVariableId(globalVariableId) },
495+
]);
496+
});
497+
498+
test("restore newly added global variable by name", () => {
499+
const globalVariable = new Variable("globalVariable", "global value");
500+
const sourceData = {
501+
pages: createDefaultPages({
502+
rootInstanceId: "bodyId",
503+
systemDataSourceId: "",
504+
}),
505+
...renderData(
506+
<ws.root ws:id={ROOT_INSTANCE_ID} vars={expression`${globalVariable}`}>
507+
<$.Body ws:id="bodyId">
508+
<$.Box ws:id="boxId">{expression`${globalVariable}`}</$.Box>
509+
</$.Body>
510+
</ws.root>,
511+
// generate different ids in source and data projects
512+
nanoid
513+
),
514+
};
515+
sourceData.instances.delete(ROOT_INSTANCE_ID);
516+
const targetData = {
517+
pages: createDefaultPages({
518+
rootInstanceId: "anotherBodyId",
519+
systemDataSourceId: "",
520+
}),
521+
// generate different ids in source and data projects
522+
...renderData(<$.Body ws:id="anotherBodyId"></$.Body>, nanoid),
523+
};
524+
insertPageCopyMutable({
525+
source: { data: sourceData, pageId: sourceData.pages.homePage.id },
526+
target: { data: targetData, folderId: ROOT_FOLDER_ID },
527+
});
528+
expect(targetData.dataSources.size).toEqual(1);
529+
const [globalVariableId] = targetData.dataSources.keys();
530+
expect(Array.from(targetData.instances.values())).toEqual([
531+
expect.objectContaining({ component: "Body", id: "anotherBodyId" }),
532+
expect.objectContaining({ component: "Body" }),
533+
expect.objectContaining({ component: "Box" }),
534+
]);
535+
const newBox = Array.from(targetData.instances.values()).at(-1);
536+
expect(newBox?.children).toEqual([
537+
{ type: "expression", value: encodeDataVariableId(globalVariableId) },
538+
]);
539+
});
404540
});

‎packages/template/src/jsx.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,10 @@ const traverseJsx = (
142142
return result;
143143
};
144144

145-
export const renderTemplate = (root: JSX.Element): WebstudioFragment => {
145+
export const renderTemplate = (
146+
root: JSX.Element,
147+
generateId?: () => string
148+
): WebstudioFragment => {
146149
let lastId = -1;
147150
const instances: Instance[] = [];
148151
const props: Prop[] = [];
@@ -157,7 +160,7 @@ export const renderTemplate = (root: JSX.Element): WebstudioFragment => {
157160
let id = ids.get(key);
158161
if (id === undefined) {
159162
lastId += 1;
160-
id = lastId.toString();
163+
id = generateId?.() ?? lastId.toString();
161164
ids.set(key, id);
162165
}
163166
return id;
@@ -370,7 +373,10 @@ export const renderTemplate = (root: JSX.Element): WebstudioFragment => {
370373
};
371374
};
372375

373-
export const renderData = (root: JSX.Element): Omit<WebstudioData, "pages"> => {
376+
export const renderData = (
377+
root: JSX.Element,
378+
generateId?: () => string
379+
): Omit<WebstudioData, "pages"> => {
374380
const {
375381
instances,
376382
props,
@@ -381,7 +387,7 @@ export const renderData = (root: JSX.Element): Omit<WebstudioData, "pages"> => {
381387
dataSources,
382388
resources,
383389
assets,
384-
} = renderTemplate(root);
390+
} = renderTemplate(root, generateId);
385391
return {
386392
instances: new Map(instances.map((item) => [item.id, item])),
387393
props: new Map(props.map((item) => [item.id, item])),

0 commit comments

Comments
 (0)