feat: forbid data variables with the same name on instance (#4771)

Ref https://github.com/webstudio-is/webstudio/issues/4768

We don't want to allow data variables on one instance with the same
name. Though we will allow child variables to mask parent ones in
another PR.


To test:
- create variable var1 on box
- try to create another variable with var1
- create variable var2
- try to rename it to var1
- create variable on body with var1
This commit is contained in:
Bogdan Chadkin
2025-01-22 16:24:05 +04:00
committed by GitHub
parent aa8a7209f2
commit 73767437b6
4 changed files with 112 additions and 58 deletions

View File

@ -1,5 +1,6 @@
import { z } from "zod";
import { nanoid } from "nanoid";
import { computed } from "nanostores";
import { useStore } from "@nanostores/react";
import {
type ReactNode,
@ -12,6 +13,7 @@ import {
useRef,
createContext,
useEffect,
useCallback,
} from "react";
import { CopyIcon, RefreshIcon, UpgradeIcon } from "@webstudio-is/icons";
import {
@ -55,7 +57,7 @@ import {
$userPlanFeatures,
} from "~/shared/nano-states";
import { serverSyncStore } from "~/shared/sync";
import { $selectedInstance } from "~/shared/awareness";
import { BindingPopoverProvider } from "~/builder/shared/binding-popover";
import {
EditorDialog,
@ -68,18 +70,46 @@ import {
SystemResourceForm,
} from "./resource-panel";
import { generateCurl } from "./curl";
import { $selectedInstance } from "~/shared/awareness";
const validateName = (value: string) =>
value.trim().length === 0 ? "Name is required" : "";
const $variablesByName = computed(
[$selectedInstance, $dataSources],
(instance, dataSources) => {
const variablesByName = new Map<DataSource["name"], DataSource["id"]>();
for (const dataSource of dataSources.values()) {
if (dataSource.scopeInstanceId === instance?.id) {
variablesByName.set(dataSource.name, dataSource.id);
}
}
return variablesByName;
}
);
const NameField = ({ defaultValue }: { defaultValue: string }) => {
const NameField = ({
variableId,
defaultValue,
}: {
variableId: undefined | DataSource["id"];
defaultValue: string;
}) => {
const ref = useRef<HTMLInputElement>(null);
const [error, setError] = useState("");
const nameId = useId();
const variablesByName = useStore($variablesByName);
const validateName = useCallback(
(value: string) => {
if (variablesByName.get(value) !== variableId) {
return "Name is already used by another variable on this instance";
}
if (value.trim().length === 0) {
return "Name is required";
}
return "";
},
[variablesByName, variableId]
);
useEffect(() => {
ref.current?.setCustomValidity(validateName(defaultValue));
}, [defaultValue]);
}, [defaultValue, validateName]);
return (
<Grid gap={1}>
<Label htmlFor={nameId}>Name</Label>
@ -510,7 +540,10 @@ const VariablePanel = forwardRef<
if (variableType === "parameter") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<ParameterForm ref={ref} variable={variable} />
</>
);
@ -518,7 +551,10 @@ const VariablePanel = forwardRef<
if (variableType === "string") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<TypeField value={variableType} onChange={setVariableType} />
<StringForm ref={ref} variable={variable} />
</>
@ -527,7 +563,10 @@ const VariablePanel = forwardRef<
if (variableType === "number") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<TypeField value={variableType} onChange={setVariableType} />
<NumberForm ref={ref} variable={variable} />
</>
@ -536,7 +575,10 @@ const VariablePanel = forwardRef<
if (variableType === "boolean") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<TypeField value={variableType} onChange={setVariableType} />
<BooleanForm ref={ref} variable={variable} />
</>
@ -545,7 +587,10 @@ const VariablePanel = forwardRef<
if (variableType === "json") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<TypeField value={variableType} onChange={setVariableType} />
<JsonForm ref={ref} variable={variable} />
</>
@ -555,7 +600,10 @@ const VariablePanel = forwardRef<
if (variableType === "resource") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<TypeField value={variableType} onChange={setVariableType} />
<ResourceForm ref={ref} variable={variable} />
</>
@ -565,7 +613,10 @@ const VariablePanel = forwardRef<
if (variableType === "graphql-resource") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<TypeField value={variableType} onChange={setVariableType} />
<GraphqlResourceForm ref={ref} variable={variable} />
</>
@ -575,7 +626,10 @@ const VariablePanel = forwardRef<
if (variableType === "system-resource") {
return (
<>
<NameField defaultValue={variable?.name ?? ""} />
<NameField
variableId={variable?.id}
defaultValue={variable?.name ?? ""}
/>
<TypeField value={variableType} onChange={setVariableType} />
<SystemResourceForm ref={ref} variable={variable} />
</>

View File

@ -1,6 +1,6 @@
import { expect, test } from "vitest";
import { evaluateExpressionWithinScope } from "./binding-popover";
import { encodeDataSourceVariable } from "@webstudio-is/sdk";
import { evaluateExpressionWithinScope } from "./binding-popover";
test("evaluateExpressionWithinScope works", () => {
const variableName = "jsonVariable";

View File

@ -19,15 +19,15 @@
"dependencies": {
"@atlaskit/pragmatic-drag-and-drop": "^1.4.0",
"@codemirror/autocomplete": "^6.18.4",
"@codemirror/commands": "^6.7.1",
"@codemirror/commands": "^6.8.0",
"@codemirror/lang-css": "^6.3.1",
"@codemirror/lang-html": "^6.4.9",
"@codemirror/lang-javascript": "^6.2.2",
"@codemirror/lang-markdown": "^6.3.1",
"@codemirror/lang-markdown": "^6.3.2",
"@codemirror/language": "^6.10.8",
"@codemirror/lint": "^6.8.4",
"@codemirror/state": "^6.5.0",
"@codemirror/view": "^6.36.1",
"@codemirror/state": "^6.5.1",
"@codemirror/view": "^6.36.2",
"@floating-ui/dom": "^1.6.12",
"@fontsource-variable/inter": "^5.0.20",
"@fontsource-variable/manrope": "^5.0.20",

78
pnpm-lock.yaml generated
View File

@ -125,8 +125,8 @@ importers:
specifier: ^6.18.4
version: 6.18.4
'@codemirror/commands':
specifier: ^6.7.1
version: 6.7.1
specifier: ^6.8.0
version: 6.8.0
'@codemirror/lang-css':
specifier: ^6.3.1
version: 6.3.1
@ -137,8 +137,8 @@ importers:
specifier: ^6.2.2
version: 6.2.2
'@codemirror/lang-markdown':
specifier: ^6.3.1
version: 6.3.1
specifier: ^6.3.2
version: 6.3.2
'@codemirror/language':
specifier: ^6.10.8
version: 6.10.8
@ -146,11 +146,11 @@ importers:
specifier: ^6.8.4
version: 6.8.4
'@codemirror/state':
specifier: ^6.5.0
version: 6.5.0
specifier: ^6.5.1
version: 6.5.1
'@codemirror/view':
specifier: ^6.36.1
version: 6.36.1
specifier: ^6.36.2
version: 6.36.2
'@floating-ui/dom':
specifier: ^1.6.12
version: 1.6.12
@ -2504,8 +2504,8 @@ packages:
'@codemirror/autocomplete@6.18.4':
resolution: {integrity: sha512-sFAphGQIqyQZfP2ZBsSHV7xQvo9Py0rV0dW7W3IMRdS+zDuNb2l3no78CvUaWKGfzFjI4FTrLdUSj86IGb2hRA==}
'@codemirror/commands@6.7.1':
resolution: {integrity: sha512-llTrboQYw5H4THfhN4U3qCnSZ1SOJ60ohhz+SzU0ADGtwlc533DtklQP0vSFaQuCPDn3BPpOd1GbbnUtwNjsrw==}
'@codemirror/commands@6.8.0':
resolution: {integrity: sha512-q8VPEFaEP4ikSlt6ZxjB3zW72+7osfAYW9i8Zu943uqbKuz6utc1+F170hyLUCUltXORjQXRyYQNfkckzA/bPQ==}
'@codemirror/lang-css@6.3.1':
resolution: {integrity: sha512-kr5fwBGiGtmz6l0LSJIbno9QrifNMUusivHbnA1H6Dmqy4HZFte3UAICix1VuKo0lMPKQr2rqB+0BkKi/S3Ejg==}
@ -2516,8 +2516,8 @@ packages:
'@codemirror/lang-javascript@6.2.2':
resolution: {integrity: sha512-VGQfY+FCc285AhWuwjYxQyUQcYurWlxdKYT4bqwr3Twnd5wP5WSeu52t4tvvuWmljT4EmgEgZCqSieokhtY8hg==}
'@codemirror/lang-markdown@6.3.1':
resolution: {integrity: sha512-y3sSPuQjBKZQbQwe3ZJKrSW6Silyl9PnrU/Mf0m2OQgIlPoSYTtOvEL7xs94SVMkb8f4x+SQFnzXPdX4Wk2lsg==}
'@codemirror/lang-markdown@6.3.2':
resolution: {integrity: sha512-c/5MYinGbFxYl4itE9q/rgN/sMTjOr8XL5OWnC+EaRMLfCbVUmmubTJfdgpfcSS2SCaT7b+Q+xi3l6CgoE+BsA==}
'@codemirror/language@6.10.8':
resolution: {integrity: sha512-wcP8XPPhDH2vTqf181U8MbZnW+tDyPYy0UzVOa+oHORjyT+mhhom9vBd7dApJwoDz9Nb/a8kHjJIsuA/t8vNFw==}
@ -2525,11 +2525,11 @@ packages:
'@codemirror/lint@6.8.4':
resolution: {integrity: sha512-u4q7PnZlJUojeRe8FJa/njJcMctISGgPQ4PnWsd9268R4ZTtU+tfFYmwkBvgcrK2+QQ8tYFVALVb5fVJykKc5A==}
'@codemirror/state@6.5.0':
resolution: {integrity: sha512-MwBHVK60IiIHDcoMet78lxt6iw5gJOGSbNbOIVBHWVXIH4/Nq1+GQgLLGgI1KlnN86WDXsPudVaqYHKBIx7Eyw==}
'@codemirror/state@6.5.1':
resolution: {integrity: sha512-3rA9lcwciEB47ZevqvD8qgbzhM9qMb8vCcQCNmDfVRPQG4JT9mSb0Jg8H7YjKGGQcFnLN323fj9jdnG59Kx6bg==}
'@codemirror/view@6.36.1':
resolution: {integrity: sha512-miD1nyT4m4uopZaDdO2uXU/LLHliKNYL9kB1C1wJHrunHLm/rpkb5QVSokqgw9hFqEZakrdlb/VGWX8aYZTslQ==}
'@codemirror/view@6.36.2':
resolution: {integrity: sha512-DZ6ONbs8qdJK0fdN7AB82CgI6tYXf4HWk1wSVa0+9bhVznCuuvhQtX8bFBoy3dv8rZSQqUd8GvhVAcielcidrA==}
'@cspotcode/source-map-support@0.8.1':
resolution: {integrity: sha512-IchNf6dN4tHoMFIn/7OE8LWZ19Y6q/67Bmf6vnGREv8RSbBVb9LPJxEcnwrcwX6ixSvaiGoomAUvu4YSxXrVgw==}
@ -9784,22 +9784,22 @@ snapshots:
'@codemirror/autocomplete@6.18.4':
dependencies:
'@codemirror/language': 6.10.8
'@codemirror/state': 6.5.0
'@codemirror/view': 6.36.1
'@codemirror/state': 6.5.1
'@codemirror/view': 6.36.2
'@lezer/common': 1.2.3
'@codemirror/commands@6.7.1':
'@codemirror/commands@6.8.0':
dependencies:
'@codemirror/language': 6.10.8
'@codemirror/state': 6.5.0
'@codemirror/view': 6.36.1
'@codemirror/state': 6.5.1
'@codemirror/view': 6.36.2
'@lezer/common': 1.2.3
'@codemirror/lang-css@6.3.1':
dependencies:
'@codemirror/autocomplete': 6.18.4
'@codemirror/language': 6.10.8
'@codemirror/state': 6.5.0
'@codemirror/state': 6.5.1
'@lezer/common': 1.2.3
'@lezer/css': 1.1.9
@ -9809,8 +9809,8 @@ snapshots:
'@codemirror/lang-css': 6.3.1
'@codemirror/lang-javascript': 6.2.2
'@codemirror/language': 6.10.8
'@codemirror/state': 6.5.0
'@codemirror/view': 6.36.1
'@codemirror/state': 6.5.1
'@codemirror/view': 6.36.2
'@lezer/common': 1.2.3
'@lezer/css': 1.1.9
'@lezer/html': 1.3.10
@ -9820,25 +9820,25 @@ snapshots:
'@codemirror/autocomplete': 6.18.4
'@codemirror/language': 6.10.8
'@codemirror/lint': 6.8.4
'@codemirror/state': 6.5.0
'@codemirror/view': 6.36.1
'@codemirror/state': 6.5.1
'@codemirror/view': 6.36.2
'@lezer/common': 1.2.3
'@lezer/javascript': 1.4.19
'@codemirror/lang-markdown@6.3.1':
'@codemirror/lang-markdown@6.3.2':
dependencies:
'@codemirror/autocomplete': 6.18.4
'@codemirror/lang-html': 6.4.9
'@codemirror/language': 6.10.8
'@codemirror/state': 6.5.0
'@codemirror/view': 6.36.1
'@codemirror/state': 6.5.1
'@codemirror/view': 6.36.2
'@lezer/common': 1.2.3
'@lezer/markdown': 1.3.1
'@codemirror/language@6.10.8':
dependencies:
'@codemirror/state': 6.5.0
'@codemirror/view': 6.36.1
'@codemirror/state': 6.5.1
'@codemirror/view': 6.36.2
'@lezer/common': 1.2.3
'@lezer/highlight': 1.2.1
'@lezer/lr': 1.4.2
@ -9846,17 +9846,17 @@ snapshots:
'@codemirror/lint@6.8.4':
dependencies:
'@codemirror/state': 6.5.0
'@codemirror/view': 6.36.1
'@codemirror/state': 6.5.1
'@codemirror/view': 6.36.2
crelt: 1.0.6
'@codemirror/state@6.5.0':
'@codemirror/state@6.5.1':
dependencies:
'@marijn/find-cluster-break': 1.0.2
'@codemirror/view@6.36.1':
'@codemirror/view@6.36.2':
dependencies:
'@codemirror/state': 6.5.0
'@codemirror/state': 6.5.1
style-mod: 4.1.2
w3c-keyname: 2.2.8
@ -12614,7 +12614,7 @@ snapshots:
'@vue/reactivity-transform': 3.3.4
'@vue/shared': 3.3.4
estree-walker: 2.0.2
magic-string: 0.30.15
magic-string: 0.30.17
postcss: 8.4.47
source-map-js: 1.2.1
@ -12629,7 +12629,7 @@ snapshots:
'@vue/compiler-core': 3.3.4
'@vue/shared': 3.3.4
estree-walker: 2.0.2
magic-string: 0.30.15
magic-string: 0.30.17
'@vue/reactivity@3.3.4':
dependencies:
@ -16649,7 +16649,7 @@ snapshots:
estree-walker: 3.0.3
is-reference: 3.0.1
locate-character: 3.0.0
magic-string: 0.30.15
magic-string: 0.30.17
periscopic: 3.1.0
svgo@3.0.2: