Skip to content

Commit

Permalink
fix: preserve global variables when duplicate page (#4875)
Browse files Browse the repository at this point in the history 
Missed the case. Global variables were recreated and got invalid state.
Now global variables are reused or added without changes.
  • Loading branch information
@TrySound
TrySound authored Feb 14, 2025
1 parent 8ed3fbb commit c3ac4d6
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 9 deletions.
15 changes: 10 additions & 5 deletions apps/builder/app/shared/instance-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -770,11 +770,6 @@ export const insertWebstudioFragmentCopy = ({
}
}

const fragmentDataSources: DataSources = new Map();
for (const dataSource of fragment.dataSources) {
fragmentDataSources.set(dataSource.id, dataSource);
}

const {
assets,
instances,
Expand Down Expand Up @@ -962,6 +957,16 @@ export const insertWebstudioFragmentCopy = ({
const newResourceIds = new Map<Resource["id"], Resource["id"]>();
for (let dataSource of fragment.dataSources) {
const { scopeInstanceId } = dataSource;
if (scopeInstanceId === ROOT_INSTANCE_ID) {
// add global variable only if not exist already
if (
dataSources.has(dataSource.id) === false &&
maskedIdByName.has(dataSource.name) === false
) {
dataSources.set(dataSource.id, dataSource);
}
continue;
}
// insert only data sources within portal content
if (fragmentInstanceIds.has(scopeInstanceId)) {
const newDataSourceId = nanoid();
Expand Down
136 changes: 136 additions & 0 deletions apps/builder/app/shared/page-utils.test.ts → apps/builder/app/shared/page-utils.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import { describe, expect, test } from "vitest";
import type { Project } from "@webstudio-is/project";
import {
ROOT_FOLDER_ID,
ROOT_INSTANCE_ID,
encodeDataSourceVariable,
encodeDataVariableId,
type DataSource,
type Instance,
type WebstudioData,
Expand All @@ -13,6 +15,14 @@ import {
} from "@webstudio-is/project-build";
import { $project } from "./nano-states";
import { insertPageCopyMutable } from "./page-utils";
import {
$,
expression,
renderData,
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]));
Expand Down Expand Up @@ -401,4 +411,130 @@ describe("insert page copy", () => {
expect(data.pages.pages[2].name).toEqual(`Name`);
expect(data.pages.pages[3].name).toEqual(`Name (1)`);
});

test("preserve global variables when duplicate page", () => {
const globalVariable = new Variable("globalVariable", "global value");
const data = {
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>
),
};
data.instances.delete(ROOT_INSTANCE_ID);
insertPageCopyMutable({
source: { data, pageId: data.pages.homePage.id },
target: { data, folderId: ROOT_FOLDER_ID },
});
expect(data.dataSources.size).toEqual(1);
const [globalVariableId] = data.dataSources.keys();
expect(Array.from(data.instances.values())).toEqual([
expect.objectContaining({ component: "Body", id: "bodyId" }),
expect.objectContaining({ component: "Box", id: "boxId" }),
expect.objectContaining({ component: "Body" }),
expect.objectContaining({ component: "Box" }),
]);
const newBox = Array.from(data.instances.values()).at(-1);
expect(newBox?.children).toEqual([
{ 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) },
]);
});
});
14 changes: 10 additions & 4 deletions packages/template/src/jsx.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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])),
Expand Down

0 comments on commit c3ac4d6

Please sign in to comment.