Skip to content

feat: add global system variable in builder #4896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 22, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions apps/builder/app/builder/features/pages/page-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ import {
$assets,
$instances,
$pages,
$dataSources,
$publishedOrigin,
$project,
$userPlanFeatures,
Expand Down Expand Up @@ -1365,36 +1364,26 @@ const NewPageSettingsView = ({

const createPage = (pageId: Page["id"], values: Values) => {
serverSyncStore.createTransaction(
[$pages, $instances, $dataSources],
(pages, instances, dataSources) => {
[$pages, $instances],
(pages, instances) => {
if (pages === undefined) {
return;
}
const rootInstanceId = nanoid();
const systemDataSourceId = nanoid();
pages.pages.push({
id: pageId,
name: values.name,
path: values.path,
title: values.title,
rootInstanceId,
systemDataSourceId,
meta: {},
});

instances.set(rootInstanceId, {
type: "instance",
id: rootInstanceId,
component: "Body",
children: [],
});
dataSources.set(systemDataSourceId, {
id: systemDataSourceId,
scopeInstanceId: rootInstanceId,
name: "system",
type: "parameter",
});

registerFolderChildMutable(pages.folders, pageId, values.parentFolderId);
selectInstance(undefined);
}
Expand Down
35 changes: 24 additions & 11 deletions apps/builder/app/builder/features/pages/page-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type Folder,
ROOT_FOLDER_ID,
type Page,
SYSTEM_VARIABLE_ID,
} from "@webstudio-is/sdk";
import {
cleanupChildRefsMutable,
Expand All @@ -31,10 +32,15 @@ import { updateCurrentSystem } from "~/shared/system";
setEnv("*");
registerContainers();

const initialSystem = {
origin: "https://undefined.wstd.work",
params: {},
search: {},
};

const createPages = () => {
const data = createDefaultPages({
rootInstanceId: "rootInstanceId",
systemDataSourceId: "systemDataSourceId",
homePageId: "homePageId",
});

Expand Down Expand Up @@ -65,7 +71,6 @@ const createPages = () => {
name: id,
path,
rootInstanceId: "rootInstanceId",
systemDataSourceId: "systemDataSourceId",
title: `"${id}"`,
};
return page;
Expand Down Expand Up @@ -108,7 +113,6 @@ describe("reparentOrphansMutable", () => {
name: "Page",
path: "/page",
rootInstanceId: "rootInstanceId",
systemDataSourceId: "systemDataSourceId",
title: `"Page"`,
});
pages.folders.push({
Expand Down Expand Up @@ -402,7 +406,6 @@ test("page root scope should rely on selected page", () => {
const pages = createDefaultPages({
rootInstanceId: "homeRootId",
homePageId: "homePageId",
systemDataSourceId: "system",
});
pages.pages.push({
id: "pageId",
Expand All @@ -411,7 +414,6 @@ test("page root scope should rely on selected page", () => {
path: "/",
title: `"My Title"`,
meta: {},
systemDataSourceId: "system",
});
$pages.set(pages);
$awareness.set({ pageId: "pageId" });
Expand All @@ -434,9 +436,18 @@ test("page root scope should rely on selected page", () => {
])
);
expect($pageRootScope.get()).toEqual({
aliases: new Map([["$ws$dataSource$2", "page variable"]]),
scope: { $ws$dataSource$2: "" },
variableValues: new Map([["2", ""]]),
aliases: new Map([
["$ws$system", "system"],
["$ws$dataSource$2", "page variable"],
]),
scope: {
$ws$system: initialSystem,
$ws$dataSource$2: "",
},
variableValues: new Map<string, unknown>([
[SYSTEM_VARIABLE_ID, initialSystem],
["2", ""],
]),
});
});

Expand All @@ -445,7 +456,6 @@ test("page root scope should use variable and resource values", () => {
createDefaultPages({
rootInstanceId: "homeRootId",
homePageId: "homePageId",
systemDataSourceId: "system",
})
);
$awareness.set({ pageId: "homePageId" });
Expand Down Expand Up @@ -473,21 +483,24 @@ test("page root scope should use variable and resource values", () => {
$resourceValues.set(new Map([["resourceId", "resource variable value"]]));
expect($pageRootScope.get()).toEqual({
aliases: new Map([
["$ws$system", "system"],
["$ws$dataSource$valueVariableId", "value variable"],
["$ws$dataSource$resourceVariableId", "resource variable"],
]),
scope: {
$ws$system: initialSystem,
$ws$dataSource$resourceVariableId: "resource variable value",
$ws$dataSource$valueVariableId: "value variable value",
},
variableValues: new Map([
variableValues: new Map<string, unknown>([
[SYSTEM_VARIABLE_ID, initialSystem],
["valueVariableId", "value variable value"],
["resourceVariableId", "resource variable value"],
]),
});
});

test("page root scope should prefill default system variable value", () => {
test("page root scope should provide page system variable value", () => {
$pages.set(
createDefaultPages({
rootInstanceId: "homeRootId",
Expand Down
7 changes: 6 additions & 1 deletion apps/builder/app/builder/features/pages/page-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
ROOT_FOLDER_ID,
isRootFolder,
ROOT_INSTANCE_ID,
systemParameter,
SYSTEM_VARIABLE_ID,
} from "@webstudio-is/sdk";
import { removeByMutable } from "~/shared/array-utils";
import {
Expand Down Expand Up @@ -259,7 +261,10 @@ export const $pageRootScope = computed(
getInstanceKey([page.rootInstanceId, ROOT_INSTANCE_ID])
) ?? new Map<string, unknown>();
for (const [dataSourceId, value] of values) {
const dataSource = dataSources.get(dataSourceId);
let dataSource = dataSources.get(dataSourceId);
if (dataSourceId === SYSTEM_VARIABLE_ID) {
dataSource = systemParameter;
}
if (dataSource === undefined) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {
generateObjectExpression,
isLiteralExpression,
parseObjectExpression,
SYSTEM_VARIABLE_ID,
systemParameter,
} from "@webstudio-is/sdk";
import { sitemapResourceUrl } from "@webstudio-is/sdk/runtime";
import {
Expand All @@ -34,7 +36,6 @@ import {
theme,
} from "@webstudio-is/design-system";
import { TrashIcon, InfoCircleIcon, PlusIcon } from "@webstudio-is/icons";
import { isFeatureEnabled } from "@webstudio-is/feature-flags";
import { humanizeString } from "~/shared/string-utils";
import {
$dataSources,
Expand Down Expand Up @@ -375,7 +376,7 @@ const $hiddenDataSourceIds = computed(
dataSourceIds.add(dataSource.id);
}
}
if (page?.systemDataSourceId && isFeatureEnabled("filters")) {
if (page?.systemDataSourceId) {
dataSourceIds.delete(page.systemDataSourceId);
}
return dataSourceIds;
Expand Down Expand Up @@ -406,7 +407,10 @@ const $selectedInstanceScope = computed(
if (hiddenDataSourceIds.has(dataSourceId)) {
continue;
}
const dataSource = dataSources.get(dataSourceId);
let dataSource = dataSources.get(dataSourceId);
if (dataSourceId === SYSTEM_VARIABLE_ID) {
dataSource = systemParameter;
}
if (dataSource === undefined) {
continue;
}
Expand Down
7 changes: 6 additions & 1 deletion apps/builder/app/builder/features/settings-panel/shared.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import equal from "fast-deep-equal";
import {
decodeDataSourceVariable,
encodeDataSourceVariable,
SYSTEM_VARIABLE_ID,
systemParameter,
} from "@webstudio-is/sdk";
import type { PropMeta, Prop, Asset } from "@webstudio-is/sdk";
import { InfoCircleIcon, MinusIcon } from "@webstudio-is/icons";
Expand Down Expand Up @@ -328,7 +330,10 @@ export const $selectedInstanceScope = computed(
const values = variableValuesByInstanceSelector.get(instanceKey);
if (values) {
for (const [dataSourceId, value] of values) {
const dataSource = dataSources.get(dataSourceId);
let dataSource = dataSources.get(dataSourceId);
if (dataSourceId === SYSTEM_VARIABLE_ID) {
dataSource = systemParameter;
}
if (dataSource === undefined) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { EllipsesIcon, PlusIcon } from "@webstudio-is/icons";
import type { DataSource } from "@webstudio-is/sdk";
import {
decodeDataSourceVariable,
findPageByIdOrPath,
getExpressionIdentifiers,
} from "@webstudio-is/sdk";
import {
Expand Down Expand Up @@ -196,6 +197,7 @@ const VariablesItem = ({
value: unknown;
usageCount: number;
}) => {
const selectedPage = useStore($selectedPage);
const [inspectDialogOpen, setInspectDialogOpen] = useState(false);
const [isMenuOpen, setIsMenuOpen] = useState(false);
const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false);
Expand Down Expand Up @@ -265,6 +267,23 @@ const VariablesItem = ({
Delete {usageCount > 0 && `(${usageCount} bindings)`}
</DropdownMenuItem>
)}
{source === "local" &&
variable.id === selectedPage?.systemDataSourceId && (
<DropdownMenuItem
onSelect={() => {
updateWebstudioData((data) => {
const page = findPageByIdOrPath(
selectedPage.id,
data.pages
);
delete page?.systemDataSourceId;
deleteVariableMutable(data, variable.id);
});
}}
>
Delete legacy system variable
</DropdownMenuItem>
)}
</DropdownMenuContent>
</DropdownMenu>

Expand Down
10 changes: 9 additions & 1 deletion apps/builder/app/shared/data-variables.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import {
Variable,
ws,
} from "@webstudio-is/template";
import { encodeDataVariableId, ROOT_INSTANCE_ID } from "@webstudio-is/sdk";
import {
encodeDataVariableId,
ROOT_INSTANCE_ID,
SYSTEM_VARIABLE_ID,
} from "@webstudio-is/sdk";
import {
computeExpression,
decodeDataVariableName,
Expand Down Expand Up @@ -56,6 +60,7 @@ test("find available variables", () => {
expect(
findAvailableVariables({ ...data, startingInstanceId: "boxId" })
).toEqual([
expect.objectContaining({ name: "system", id: SYSTEM_VARIABLE_ID }),
expect.objectContaining({ name: "bodyVariable" }),
expect.objectContaining({ name: "boxVariable" }),
]);
Expand All @@ -72,6 +77,7 @@ test("find masked variables", () => {
expect(
findAvailableVariables({ ...data, startingInstanceId: "boxId" })
).toEqual([
expect.objectContaining({ name: "system", id: SYSTEM_VARIABLE_ID }),
expect.objectContaining({ scopeInstanceId: "boxId", name: "myVariable" }),
]);
});
Expand All @@ -90,6 +96,7 @@ test("find global variables", () => {
expect(
findAvailableVariables({ ...data, startingInstanceId: "boxId" })
).toEqual([
expect.objectContaining({ name: "system", id: SYSTEM_VARIABLE_ID }),
expect.objectContaining({ name: "globalVariable" }),
expect.objectContaining({ name: "boxVariable" }),
]);
Expand All @@ -114,6 +121,7 @@ test("find global variables in slots", () => {
expect(
findAvailableVariables({ ...data, startingInstanceId: "boxId" })
).toEqual([
expect.objectContaining({ name: "system", id: SYSTEM_VARIABLE_ID }),
expect.objectContaining({ name: "globalVariable" }),
expect.objectContaining({ name: "boxVariable" }),
]);
Expand Down
7 changes: 7 additions & 0 deletions apps/builder/app/shared/data-variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import {
type Resources,
type WebstudioData,
ROOT_INSTANCE_ID,
SYSTEM_VARIABLE_ID,
decodeDataVariableId,
encodeDataVariableId,
findTreeInstanceIdsExcludingSlotDescendants,
systemParameter,
transpileExpression,
} from "@webstudio-is/sdk";
import {
Expand Down Expand Up @@ -213,6 +215,8 @@ const findMaskedVariablesByInstanceId = ({
// allow accessing global variables everywhere
instanceIdsPath.push(ROOT_INSTANCE_ID);
const maskedVariables = new Map<DataSource["name"], DataSource["id"]>();
// global system variable always present
maskedVariables.set("system", SYSTEM_VARIABLE_ID);
// start from the root to descendant
// so child variables override parent variables
for (const instanceId of instanceIdsPath.reverse()) {
Expand Down Expand Up @@ -245,6 +249,9 @@ export const findAvailableVariables = ({
if (dataSource) {
availableVariables.push(dataSource);
}
if (dataSourceId === SYSTEM_VARIABLE_ID) {
availableVariables.push(systemParameter);
}
}
return availableVariables;
};
Expand Down
11 changes: 8 additions & 3 deletions apps/builder/app/shared/instance-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ import { current, isDraft } from "immer";
* structuredClone can be invoked on draft and throw error
* extract current snapshot before cloning
*/
const unwrap = <Value>(value: Value) =>
export const unwrap = <Value>(value: Value) =>
isDraft(value) ? current(value) : value;

export const updateWebstudioData = (mutate: (data: WebstudioData) => void) => {
Expand Down Expand Up @@ -522,7 +522,8 @@ const traverseStyleValue = (

export const extractWebstudioFragment = (
data: Omit<WebstudioData, "pages">,
rootInstanceId: string
rootInstanceId: string,
options: { unsetVariables?: Set<DataSource["id"]> } = {}
): WebstudioFragment => {
const {
assets,
Expand Down Expand Up @@ -604,8 +605,12 @@ export const extractWebstudioFragment = (
const fragmentDataSources: DataSources = new Map();
const fragmentResourceIds = new Set<Resource["id"]>();
const unsetNameById = new Map<DataSource["id"], DataSource["name"]>();
const unsetVariables = options.unsetVariables ?? new Set();
for (const dataSource of dataSources.values()) {
if (fragmentInstanceIds.has(dataSource.scopeInstanceId ?? "")) {
if (
fragmentInstanceIds.has(dataSource.scopeInstanceId ?? "") &&
unsetVariables.has(dataSource.id) === false
) {
fragmentDataSources.set(dataSource.id, dataSource);
if (dataSource.type === "resource") {
fragmentResourceIds.add(dataSource.resourceId);
Expand Down
Loading
Loading