From bc5d2951eca0a1ce3c4fc3bcf43490911dbf07db Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Mon, 27 Jan 2025 08:18:37 +0400 Subject: [PATCH] feat: support expressions copy paste between instances (#4790) Ref https://github.com/webstudio-is/webstudio/issues/4768 Here improved copy paste experience between expressions. All expressions while editing have are no longer encoded with ids. For example `system.search.name` is the same. Though invalid js characters are encoded with code point like this `Collection Item.title` becomes `Collection$32$Item.title` when copy into textual editor. And this less obscure name can be copied between different lists with the same `Collection Item` name. --- .../settings-panel/variable-popover.tsx | 5 +- .../settings-panel/variables-section.tsx | 29 ++- .../app/builder/shared/expression-editor.tsx | 224 ++++++++---------- .../builder/app/shared/data-variables.test.ts | 58 +++++ apps/builder/app/shared/data-variables.ts | 94 ++++++++ packages/sdk/src/expression.ts | 2 + 6 files changed, 283 insertions(+), 129 deletions(-) create mode 100644 apps/builder/app/shared/data-variables.test.ts create mode 100644 apps/builder/app/shared/data-variables.ts diff --git a/apps/builder/app/builder/features/settings-panel/variable-popover.tsx b/apps/builder/app/builder/features/settings-panel/variable-popover.tsx index 80d3a36a4d90..62d0398ee458 100644 --- a/apps/builder/app/builder/features/settings-panel/variable-popover.tsx +++ b/apps/builder/app/builder/features/settings-panel/variable-popover.tsx @@ -97,7 +97,10 @@ const NameField = ({ const variablesByName = useStore($variablesByName); const validateName = useCallback( (value: string) => { - if (variablesByName.get(value) !== variableId) { + if ( + variablesByName.has(value) && + variablesByName.get(value) !== variableId + ) { return "Name is already used by another variable on this instance"; } if (value.trim().length === 0) { diff --git a/apps/builder/app/builder/features/settings-panel/variables-section.tsx b/apps/builder/app/builder/features/settings-panel/variables-section.tsx index 00e284e518e5..16066841be8b 100644 --- a/apps/builder/app/builder/features/settings-panel/variables-section.tsx +++ b/apps/builder/app/builder/features/settings-panel/variables-section.tsx @@ -3,6 +3,7 @@ import { computed } from "nanostores"; import { useStore } from "@nanostores/react"; import { Button, + css, CssValueListArrowFocus, CssValueListItem, DropdownMenu, @@ -61,6 +62,8 @@ const $availableVariables = computed( if (instancePath === undefined) { return []; } + const [{ instanceSelector }] = instancePath; + const [selectedInstanceId] = instanceSelector; const availableVariables = new Map(); // order from ancestor to descendant // so descendants can override ancestor variables @@ -71,7 +74,12 @@ const $availableVariables = computed( } } } - return Array.from(availableVariables.values()); + // order local variables first + return Array.from(availableVariables.values()).sort((left, right) => { + const leftRank = left.scopeInstanceId === selectedInstanceId ? 0 : 1; + const rightRank = right.scopeInstanceId === selectedInstanceId ? 0 : 1; + return leftRank - rightRank; + }); } ); @@ -184,6 +192,13 @@ const EmptyVariables = () => { ); }; +const variableLabelStyle = css({ + whiteSpace: "nowrap", + overflow: "hidden", + textOverflow: "ellipsis", + maxWidth: "100%", +}); + const VariablesItem = ({ variable, source, @@ -197,8 +212,6 @@ const VariablesItem = ({ value: unknown; usageCount: number; }) => { - const labelValue = - value === undefined ? "" : `: ${formatValuePreview(value)}`; const [inspectDialogOpen, setInspectDialogOpen] = useState(false); const [isMenuOpen, setIsMenuOpen] = useState(false); return ( @@ -207,9 +220,14 @@ const VariablesItem = ({ id={variable.id} index={index} label={ - + - {labelValue} + {value !== undefined && ( + +   + {formatValuePreview(value)} + + )} } disabled={source === "remote"} @@ -276,6 +294,7 @@ const VariablesList = () => { return ( + {/* local variables should be ordered first to not block tab to first item */} {availableVariables.map((variable, index) => ( { - const [{ scope, aliases }] = context.state.facet(VariablesData); + const [{ scope }] = context.state.facet(VariablesData); const path = completionPath(context); if (path === undefined) { return null; @@ -195,7 +195,7 @@ const scopeCompletionSource: CompletionSource = (context) => { if (typeof target === "object" && target !== null) { options = Object.entries(target).map(([name, value]) => ({ label: name, - displayLabel: aliases.get(name), + displayLabel: decodeDataVariableName(name), detail: formatValuePreview(value), apply: (view, completion, from, to) => { // complete valid js identifier or top level variable without quotes @@ -266,50 +266,48 @@ class VariableWidget extends WidgetType { } } -const variableMatcher = new MatchDecorator({ - regexp: /(\$ws\$dataSource\$\w+)/g, - - decorate: (add, from, _to, match, view) => { - const name = match[1]; - const [{ aliases }] = view.state.facet(VariablesData); - - // The regexp may match variables not in scope, but the key problem we're solving is this: - // We have an alias $ws$dataSource$321 -> SomeVar, which we display as '[SomeVar]' ([] means decoration in the editor). - // If the user types a symbol (e.g., 'a') immediately after '[SomeVar]', - // the raw text becomes $ws$dataSource$321a, but we want to display '[SomeVar]a'. - const dataSourceId = [...aliases.keys()].find((key) => name.includes(key)); - - if (dataSourceId === undefined) { - return; - } - - const endPos = from + dataSourceId.length; - - add( - from, - endPos, - Decoration.replace({ - widget: new VariableWidget(aliases.get(dataSourceId)!), - }) - ); - }, -}); +const getVariableDecorations = (view: EditorView) => { + const builder = new RangeSetBuilder(); + syntaxTree(view.state).iterate({ + from: 0, + to: view.state.doc.length, + enter: (node) => { + if (node.name === "VariableName") { + const [{ scope }] = view.state.facet(VariablesData); + const identifier = view.state.doc.sliceString(node.from, node.to); + const variableName = decodeDataVariableName(identifier); + if (identifier in scope) { + builder.add( + node.from, + node.to, + Decoration.replace({ + widget: new VariableWidget(variableName!), + }) + ); + } + } + }, + }); + return builder.finish(); +}; -const variables = ViewPlugin.fromClass( +const variablesPlugin = ViewPlugin.fromClass( class { - variables: DecorationSet; + decorations: DecorationSet; constructor(view: EditorView) { - this.variables = variableMatcher.createDeco(view); + this.decorations = getVariableDecorations(view); } update(update: ViewUpdate) { - this.variables = variableMatcher.updateDeco(update, this.variables); + if (update.docChanged) { + this.decorations = getVariableDecorations(update.view); + } } }, { - decorations: (instance) => instance.variables, + decorations: (instance) => instance.decorations, provide: (plugin) => EditorView.atomicRanges.of((view) => { - return view.plugin(plugin)?.variables || Decoration.none; + return view.plugin(plugin)?.decorations || Decoration.none; }), } ); @@ -323,76 +321,6 @@ const wrapperStyle = css({ "--ws-code-editor-max-height": "320px", }); -/** - * Replaces variables with their IDs, e.g., someVar -> $ws$dataSource$321 - */ -const replaceWithWsVariables = EditorState.transactionFilter.of((tr) => { - if (!tr.docChanged) { - return tr; - } - - const state = tr.startState; - const [{ aliases }] = state.facet(VariablesData); - - const aliasesByName = mapGroupBy(Array.from(aliases), ([_id, name]) => name); - - // The idea of cursor preservation is simple: - // There are 2 cases we are handling: - // 1. A variable is replaced while typing its name. In this case, we preserve the cursor position from the end of the text. - // 2. A variable is replaced when an operation makes the expression valid. For example, ('' b) -> ('' + b). - // In this case, we preserve the cursor position from the start of the text. - // This does not cover cases like (a b) -> (a + b). We are not handling it because I haven't found a way to enter such a case into real input. - // We can improve it if issues arise. - - const cursorPos = tr.selection?.main.head ?? 0; - const cursorPosFromEnd = tr.newDoc.length - cursorPos; - - const content = tr.newDoc.toString(); - const originalContent = tr.startState.doc.toString(); - - let updatedContent = content; - - try { - updatedContent = transpileExpression({ - expression: content, - replaceVariable: (identifier) => { - if (decodeDataSourceVariable(identifier) && aliases.has(identifier)) { - return; - } - // prevent matching variables by unambiguous name - const matchedAliases = aliasesByName.get(identifier); - if (matchedAliases && matchedAliases.length === 1) { - const [id, _name] = matchedAliases[0]; - - return id; - } - }, - }); - } catch { - // empty block - } - - if (updatedContent !== content) { - return [ - { - changes: { - from: 0, - to: originalContent.length, - insert: updatedContent, - }, - selection: { - anchor: - updatedContent.slice(0, cursorPos) === content.slice(0, cursorPos) - ? cursorPos - : updatedContent.length - cursorPosFromEnd, - }, - }, - ]; - } - - return tr; -}); - const linterTooltipTheme = EditorView.theme({ ".cm-tooltip:has(.cm-tooltip-lint)": { backgroundColor: "transparent", @@ -416,10 +344,10 @@ const linterTooltipTheme = EditorView.theme({ }); const expressionLinter = linter((view) => { - const [{ aliases }] = view.state.facet(VariablesData); + const [{ scope }] = view.state.facet(VariablesData); return lintExpression({ expression: view.state.doc.toString(), - availableVariables: new Set(aliases.keys()), + availableVariables: new Set(Object.keys(scope)), }); }); @@ -450,13 +378,51 @@ export const ExpressionEditor = ({ onChange: (value: string) => void; onChangeComplete: (value: string) => void; }) => { + const { nameById, idByName } = useMemo(() => { + const nameById = new Map(); + const idByName = new Map(); + for (const [identifier, name] of aliases) { + const id = decodeDataVariableId(identifier); + if (id) { + nameById.set(id, name); + idByName.set(name, id); + } + } + return { nameById, idByName }; + }, [aliases]); + const expressionWithUnsetVariables = useMemo(() => { + return unsetExpressionVariables({ + expression: value, + unsetNameById: nameById, + }); + }, [value, nameById]); + const scopeWithUnsetVariables = useMemo(() => { + const newScope: typeof scope = {}; + for (const [identifier, value] of Object.entries(scope)) { + const name = aliases.get(identifier); + if (name) { + newScope[encodeDataVariableName(name)] = value; + } + } + return newScope; + }, [scope, aliases]); + const aliasesWithUnsetVariables = useMemo(() => { + const newAliases: typeof aliases = new Map(); + for (const [_identifier, name] of aliases) { + newAliases.set(encodeDataVariableName(name), name); + } + return newAliases; + }, [aliases]); + const extensions = useMemo( () => [ bracketMatching(), closeBrackets(), javascript({}), - VariablesData.of({ scope, aliases }), - replaceWithWsVariables, + VariablesData.of({ + scope: scopeWithUnsetVariables, + aliases: aliasesWithUnsetVariables, + }), // render autocomplete in body // to prevent popover scroll overflow tooltips({ parent: document.body }), @@ -464,12 +430,12 @@ export const ExpressionEditor = ({ override: [scopeCompletionSource], icons: false, }), - variables, + variablesPlugin, keymap.of([...closeBracketsKeymap, ...completionKeymap]), expressionLinter, linterTooltipTheme, ], - [scope, aliases] + [scopeWithUnsetVariables, aliasesWithUnsetVariables] ); // prevent clicking on autocomplete options propagating to body @@ -497,9 +463,21 @@ export const ExpressionEditor = ({ invalid={color === "error"} readOnly={readOnly} autoFocus={autoFocus} - value={value} - onChange={onChange} - onChangeComplete={onChangeComplete} + value={expressionWithUnsetVariables} + onChange={(newValue) => { + const expressionWithRestoredVariables = restoreExpressionVariables({ + expression: newValue, + maskedIdByName: idByName, + }); + onChange(expressionWithRestoredVariables); + }} + onChangeComplete={(newValue) => { + const expressionWithRestoredVariables = restoreExpressionVariables({ + expression: newValue, + maskedIdByName: idByName, + }); + onChangeComplete(expressionWithRestoredVariables); + }} /> ); diff --git a/apps/builder/app/shared/data-variables.test.ts b/apps/builder/app/shared/data-variables.test.ts new file mode 100644 index 000000000000..68269106ff66 --- /dev/null +++ b/apps/builder/app/shared/data-variables.test.ts @@ -0,0 +1,58 @@ +import { expect, test } from "vitest"; +import { + decodeDataVariableName, + encodeDataVariableName, + restoreExpressionVariables, + unsetExpressionVariables, +} from "./data-variables"; + +test("encode data variable name when necessary", () => { + expect(encodeDataVariableName("formState")).toEqual("formState"); + expect(encodeDataVariableName("Collection Item")).toEqual( + "Collection$32$Item" + ); + expect(encodeDataVariableName("$my$Variable")).toEqual("$36$my$36$Variable"); +}); + +test("dencode data variable name", () => { + expect(decodeDataVariableName(encodeDataVariableName("formState"))).toEqual( + "formState" + ); + expect( + decodeDataVariableName(encodeDataVariableName("Collection Item")) + ).toEqual("Collection Item"); +}); + +test("dencode data variable name with dollar sign", () => { + expect( + decodeDataVariableName(encodeDataVariableName("$my$Variable")) + ).toEqual("$my$Variable"); + expect(decodeDataVariableName("$my$Variable")).toEqual("$my$Variable"); +}); + +test("unset expression variables", () => { + expect( + unsetExpressionVariables({ + expression: `$ws$dataSource$myId + arbitaryVariable`, + unsetNameById: new Map([["myId", "My Variable"]]), + }) + ).toEqual("My$32$Variable + arbitaryVariable"); +}); + +test("ignore not existing variables in expressions", () => { + expect( + unsetExpressionVariables({ + expression: `$ws$dataSource$myId + arbitaryVariable`, + unsetNameById: new Map(), + }) + ).toEqual("$ws$dataSource$myId + arbitaryVariable"); +}); + +test("restore expression variables", () => { + expect( + restoreExpressionVariables({ + expression: `My$32$Variable + missingVariable`, + maskedIdByName: new Map([["My Variable", "myId"]]), + }) + ).toEqual("$ws$dataSource$myId + missingVariable"); +}); diff --git a/apps/builder/app/shared/data-variables.ts b/apps/builder/app/shared/data-variables.ts new file mode 100644 index 000000000000..1c17c8d6b155 --- /dev/null +++ b/apps/builder/app/shared/data-variables.ts @@ -0,0 +1,94 @@ +import { + type DataSource, + decodeDataVariableId, + encodeDataVariableId, + transpileExpression, +} from "@webstudio-is/sdk"; + +const allowedJsChars = /[A-Za-z_]/; + +/** + * variable names can contain any characters and + * this utility encodes data variable name into valid js identifier + * for example + * "Collection Item" -> "Collection$20$Item" + */ +export const encodeDataVariableName = (name: string) => { + let encodedName = ""; + for (let index = 0; index < name.length; index += 1) { + const char = name[index]; + encodedName += allowedJsChars.test(char) + ? char + : `$${char.codePointAt(0)}$`; + } + return encodedName; +}; + +/** + * Variable name should be restorable from encoded js identifier + */ +export const decodeDataVariableName = (identifier: string) => { + const name = identifier.replaceAll(/\$(\d+)\$/g, (_match, code) => + String.fromCodePoint(code) + ); + return name; +}; + +/** + * replace all encoded ids with encoded names + * to make expression transferrable + */ +export const unsetExpressionVariables = ({ + expression, + unsetNameById, +}: { + expression: string; + unsetNameById: Map; +}) => { + try { + return transpileExpression({ + expression, + replaceVariable: (identifier) => { + const id = decodeDataVariableId(identifier); + if (id) { + const name = unsetNameById.get(id); + if (name) { + return encodeDataVariableName(name); + } + } + return identifier; + }, + }); + } catch { + return expression; + } +}; + +/** + * restore variable ids by js identifiers + */ +export const restoreExpressionVariables = ({ + expression, + maskedIdByName, +}: { + expression: string; + maskedIdByName: Map; +}) => { + try { + return transpileExpression({ + expression, + replaceVariable: (identifier) => { + const name = decodeDataVariableName(identifier); + if (name) { + const id = maskedIdByName.get(name); + if (id) { + return encodeDataVariableId(id); + } + } + return identifier; + }, + }); + } catch { + return expression; + } +}; diff --git a/packages/sdk/src/expression.ts b/packages/sdk/src/expression.ts index 601450b1c74e..db8b256b3407 100644 --- a/packages/sdk/src/expression.ts +++ b/packages/sdk/src/expression.ts @@ -308,6 +308,7 @@ export const encodeDataSourceVariable = (id: string) => { const encoded = id.replaceAll("-", "__DASH__"); return `${dataSourceVariablePrefix}${encoded}`; }; +export { encodeDataSourceVariable as encodeDataVariableId }; export const decodeDataSourceVariable = (name: string) => { if (name.startsWith(dataSourceVariablePrefix)) { @@ -316,6 +317,7 @@ export const decodeDataSourceVariable = (name: string) => { } return; }; +export { decodeDataSourceVariable as decodeDataVariableId }; export const generateExpression = ({ expression,