From 47ac149b0bd9a0c79f39cf04eea1bba3b1fd8974 Mon Sep 17 00:00:00 2001 From: Balazs Bajorics <2226774+balazsbajorics@users.noreply.github.com> Date: Tue, 4 Jun 2024 15:33:43 +0200 Subject: [PATCH] Data Picker: be able to pick global variables (#5790) **Problem:** Globally declared variables were missing from the data picker. To investigate, I created a sample project with nested scopes and I've identified two missing cases, the first one is: globally declared variables in the code file. I had to re-acquaint myself with the render-core-element codebase, and with @seanparsons 's help I realized what we are missing was `runBlockUpdatingScope` returning spied variables. **Fix:** The solution was to extend the file-scope-level `mutableContext` and add a new field `spiedVariablesDeclaredInRootScope: VariableData` in there. **Commit Details:** - Added a new FileRootPath type so it's now `insertionCeiling: FileRootPath | ElementPath`, this lets us not use `null` or `EP.emptyPath` as the "file root" reference - New easier-to-read test case in `scoped-variables.spec.tsx` - runBlockUpdatingScope returns `spiedVariablesDeclaredWithinBlock: VariableData` - feed spiedVariablesDeclaredWithinBlock into `mutableContext.spiedVariablesDeclaredInRootScope` - extend `spiedVariablesInScope` in `createComponentRendererComponent` to include the root scope variables **Manual Tests:** I hereby swear that: - [x] I opened a hydrogen project and it loaded - [x] I could navigate to various routes in Preview mode --- .../strategies/reparent-utils.ts | 7 +- .../canvas/scoped-variables.spec.tsx | 177 +++++++++++++++++- .../ui-jsx-canvas-component-renderer.tsx | 28 +-- .../ui-jsx-canvas-contexts.tsx | 2 + .../ui-jsx-canvas-execution-scope.tsx | 18 +- .../ui-jsx-canvas-scope-utils.ts | 12 +- .../src/components/canvas/ui-jsx-canvas.tsx | 27 ++- .../editor/actions/actions.spec.tsx | 2 +- .../component-section/data-picker-popup.tsx | 2 +- .../component-section/data-picker-utils.tsx | 13 +- .../component-section/data-selector-modal.tsx | 40 ++-- .../variables-in-scope-utils.spec.ts | 3 + .../variables-in-scope-utils.ts | 42 ++++- .../component-picker-context-menu.tsx | 2 +- .../navigator-item/component-picker.tsx | 3 +- .../project-components.spec.ts.snap | 56 ++++-- .../components/shared/project-components.ts | 15 +- .../src/components/shared/scoped-variables.ts | 4 +- editor/src/core/data-tracing/data-tracing.ts | 6 +- .../model/element-metadata.spec.browser2.tsx | 16 ++ editor/src/core/shared/element-template.ts | 12 +- 21 files changed, 401 insertions(+), 86 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/reparent-utils.ts b/editor/src/components/canvas/canvas-strategies/strategies/reparent-utils.ts index 93f98bcc26ab..c71c406e3d3b 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/reparent-utils.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/reparent-utils.ts @@ -61,6 +61,7 @@ import { renameDuplicateImports } from '../../../../core/shared/import-shared-ut import { set } from '../../../../core/shared/optics/optic-utilities' import { fromField, fromTypeGuard } from '../../../../core/shared/optics/optic-creators' import type { PropertyControlsInfo } from '../../../custom-code/code-file' +import type { FileRootPath } from '../../ui-jsx-canvas' interface GetReparentOutcomeResult { commands: Array @@ -614,10 +615,10 @@ export function applyElementCeilingToReparentTarget( elementsToInsert: JSXElementChild[], elementPathTree: ElementPathTrees, reparentTarget: Either, - elementCeiling: ElementPath | null, + elementCeiling: ElementPath | FileRootPath, propertyControlsInfo: PropertyControlsInfo, ): Either { - if (elementCeiling == null) { + if (elementCeiling.type === 'file-root') { return reparentTarget } else { return flatMapEither((targetForPaste) => { @@ -669,7 +670,7 @@ export function getTargetParentForOneShotInsertion( metadata: ElementInstanceMetadataMap, elementsToInsert: JSXElementChild[], elementPathTree: ElementPathTrees, - insertionCeiling: ElementPath | null, + insertionCeiling: ElementPath | FileRootPath, propertyControlsInfo: PropertyControlsInfo, ): Either { if (!isNonEmptyArray(selectedViews)) { diff --git a/editor/src/components/canvas/scoped-variables.spec.tsx b/editor/src/components/canvas/scoped-variables.spec.tsx index b2e40cdfaa09..32ad1b337d5f 100644 --- a/editor/src/components/canvas/scoped-variables.spec.tsx +++ b/editor/src/components/canvas/scoped-variables.spec.tsx @@ -1,5 +1,9 @@ +import json5 from 'json5' import { createBuiltInDependenciesList } from '../../core/es-modules/package-manager/built-in-dependencies-list' import { getSamplePreviewFile, getSamplePreviewHTMLFile } from '../../core/model/new-project-files' +import * as EP from '../../core/shared/element-path' +import { objectMap } from '../../core/shared/object-utils' +import { optionalMap } from '../../core/shared/optional-utils' import type { ProjectContents, TextFile } from '../../core/shared/project-file-types' import { textFile, @@ -9,6 +13,7 @@ import { directory, } from '../../core/shared/project-file-types' import { complexDefaultProjectPreParsed } from '../../sample-projects/sample-project-utils.test-utils' +import type { ProjectContentTreeRoot } from '../assets' import { contentsToTree } from '../assets' import type { PersistentModel } from '../editor/store/editor-state' import { @@ -16,7 +21,12 @@ import { StoryboardFilePath, persistentModelForProjectContents, } from '../editor/store/editor-state' -import { DefaultStartingFeatureSwitches, renderTestEditorWithModel } from './ui-jsx.test-utils' +import { insertionCeilingToString, type VariableData, type VariableMetadata } from './ui-jsx-canvas' +import { + DefaultStartingFeatureSwitches, + renderTestEditorWithModel, + renderTestEditorWithProjectContent, +} from './ui-jsx.test-utils' function projectWithInlineComponentContents(): ProjectContents { function createCodeFile(path: string, contents: string): TextFile { @@ -210,7 +220,172 @@ function projectWithInlineComponentDestructured(): PersistentModel { return persistentModel } +function projectWithDeepNestedScopes(): ProjectContentTreeRoot { + function createCodeFile(contents: string): TextFile { + return textFile(textFileContents(contents, unparsed, RevisionsState.CodeAhead), null, null, 0) + } + + return contentsToTree({ + '/package.json': textFile( + textFileContents( + JSON.stringify(DefaultPackageJson, null, 2), + unparsed, + RevisionsState.CodeAhead, + ), + null, + null, + 0, + ), + '/src': directory(), + '/assets': directory(), + '/public': directory(), + [StoryboardFilePath]: createCodeFile( + `import * as React from 'react' + import Utopia, { + Scene, + Storyboard, + } from 'utopia-api' + import { App } from '/src/app.js' + + export var storyboard = ( + + + + + + )`, + ), + '/src/app.js': createCodeFile( + `import * as React from 'react' + + const globalVar = ['alap'] + + export var App = ({ style }) => { + const localVar = ['local'] + return ( +
+ { + // @utopia/uid=globalVarMap + globalVar.map((global) => ( +
+ { + // @utopia/uid=localVarMap + localVar.map((local) => ( +
+ { + // @utopia/uid=firstMap + ['a', 'b', 'c'].map((first) => ( +
+ { + // @utopia/uid=secondMap + [1, 2, 3].map((second) => ( +
+ {(() => { + const innerVar = ['cica', 'kutya'] + // @utopia/uid=innerVarMap + return innerVar.map((inner) => { + return ( +
+ {global} -{local} -{first} - + {second} -{inner} +
+ ) + }) + })()} +
+ ))} +
+ ))} +
+ ))} +
+ ))} +
+ ) + } + `, + ), + }) +} + +function prettyPrintVariableData(variableData: VariableData) { + function prettyPrintVariableMetadata(variableMetadata: VariableMetadata) { + return { + spiedValue: JSON.stringify(variableMetadata.spiedValue, null, 2), + insertionCeiling: insertionCeilingToString(variableMetadata.insertionCeiling), + } + } + + return objectMap(prettyPrintVariableMetadata, variableData) +} + describe('scoped variables', () => { + it('project with deep nested scopes works', async () => { + const renderResult = await renderTestEditorWithProjectContent( + projectWithDeepNestedScopes(), + 'dont-await-first-dom-report', + ) + + const spiedVarsInScopeForInnermostElement = prettyPrintVariableData( + renderResult.getEditorState().editor.variablesInScope[ + 'storyboard-entity/scene-1-entity/app-entity:app-root/globalvarmap/div1~~~1/localvarmap/div2~~~1/firstmap/div3~~~1/secondmap/div4~~~1/5d9/div5~~~1' + ], + ) + + expect(spiedVarsInScopeForInnermostElement).toMatchInlineSnapshot(` + Object { + "first": Object { + "insertionCeiling": "storyboard-entity/scene-1-entity/app-entity:app-root/globalvarmap/div1~~~1/localvarmap/div2~~~1/firstmap/div3~~~1", + "spiedValue": "\\"a\\"", + }, + "global": Object { + "insertionCeiling": "storyboard-entity/scene-1-entity/app-entity:app-root/globalvarmap/div1~~~1", + "spiedValue": "\\"alap\\"", + }, + "globalVar": Object { + "insertionCeiling": "file-root", + "spiedValue": "[ + \\"alap\\" + ]", + }, + "local": Object { + "insertionCeiling": "storyboard-entity/scene-1-entity/app-entity:app-root/globalvarmap/div1~~~1/localvarmap/div2~~~1", + "spiedValue": "\\"local\\"", + }, + "localVar": Object { + "insertionCeiling": "storyboard-entity/scene-1-entity/app-entity:app-root", + "spiedValue": "[ + \\"local\\" + ]", + }, + "second": Object { + "insertionCeiling": "storyboard-entity/scene-1-entity/app-entity:app-root/globalvarmap/div1~~~1/localvarmap/div2~~~1/firstmap/div3~~~1/secondmap/div4~~~1", + "spiedValue": "1", + }, + "style": Object { + "insertionCeiling": "storyboard-entity/scene-1-entity/app-entity:app-root", + "spiedValue": undefined, + }, + } + `) + + // TODO: these should not be undefined, to be fixed in a follow up PR: + expect(spiedVarsInScopeForInnermostElement['innerVar']).toMatchInlineSnapshot(`undefined`) + expect(spiedVarsInScopeForInnermostElement['inner']).toMatchInlineSnapshot(`undefined`) + }) + it('includes scoped variables for an inline component destructuring the parameter', async () => { const renderResult = await renderTestEditorWithModel( projectWithInlineComponentDestructured(), diff --git a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx index 77ac87ae5c79..2cb0891afbd9 100644 --- a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx @@ -131,6 +131,8 @@ export function createComponentRendererComponent(params: { instancePath, ) + // TODO we should throw an error if rootElementPath is null + let codeError: Error | null = null const appliedProps = optionalMap( @@ -158,7 +160,7 @@ export function createComponentRendererComponent(params: { code: code, highlightBounds: highlightBounds, editedText: rerenderUtopiaContext.editedText, - variablesInScope: {}, + variablesInScope: mutableContext.spiedVariablesDeclaredInRootScope, filePathMappings: rerenderUtopiaContext.filePathMappings, }, undefined, @@ -172,20 +174,16 @@ export function createComponentRendererComponent(params: { ...appliedProps, } - let spiedVariablesInScope: VariableData = {} + let spiedVariablesInScope: VariableData = { + ...mutableContext.spiedVariablesDeclaredInRootScope, + } if (rootElementPath != null && utopiaJsxComponent.param != null) { - spiedVariablesInScope = mapArrayToDictionary( - propertiesExposedByParam(utopiaJsxComponent.param), - (paramName) => { - return paramName - }, - (paramName) => { - return { - spiedValue: scope[paramName], - insertionCeiling: rootElementPath, - } - }, - ) + propertiesExposedByParam(utopiaJsxComponent.param).forEach((paramName) => { + spiedVariablesInScope[paramName] = { + spiedValue: scope[paramName], + insertionCeiling: rootElementPath, + } + }) } // Protect against infinite recursion by taking the view that anything @@ -259,6 +257,7 @@ export function createComponentRendererComponent(params: { applyBlockReturnFunctions(scope) const arbitraryBlockResult = runBlockUpdatingScope( + rootElementPath, params.filePath, mutableContext.requireResult, utopiaJsxComponent.arbitraryJSBlock, @@ -270,6 +269,7 @@ export function createComponentRendererComponent(params: { if (rootElementPath != null) { spiedVariablesInScope = { ...spiedVariablesInScope, + ...arbitraryBlockResult.spiedVariablesDeclaredWithinBlock, ...objectMap( (spiedValue) => ({ spiedValue: spiedValue, diff --git a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx index 6e33e1425683..f223928216b2 100644 --- a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx @@ -7,6 +7,7 @@ import { left } from '../../../core/shared/either' import type { ElementPath } from '../../../core/shared/project-file-types' import type { ProjectContentTreeRoot } from '../../assets' import type { TransientFilesState, UIFileBase64Blobs } from '../../editor/store/editor-state' +import type { VariableData } from '../ui-jsx-canvas' import type { FilePathMappings } from '../../../core/model/project-file-utils' export interface MutableUtopiaCtxRefData { @@ -15,6 +16,7 @@ export interface MutableUtopiaCtxRefData { requireResult: MapLike fileBlobs: UIFileBase64Blobs rootScope: MapLike + spiedVariablesDeclaredInRootScope: VariableData jsxFactoryFunctionName: string | null } } diff --git a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-execution-scope.tsx b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-execution-scope.tsx index 2c2ab7b637a7..f188450fd076 100644 --- a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-execution-scope.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-execution-scope.tsx @@ -22,7 +22,11 @@ import { import { createLookupRender, utopiaCanvasJSXLookup } from './ui-jsx-canvas-element-renderer-utils' import { runBlockUpdatingScope } from './ui-jsx-canvas-scope-utils' import * as EP from '../../../core/shared/element-path' -import type { DomWalkerInvalidatePathsCtxData, UiJsxCanvasContextData } from '../ui-jsx-canvas' +import type { + DomWalkerInvalidatePathsCtxData, + UiJsxCanvasContextData, + VariableData, +} from '../ui-jsx-canvas' import type { ElementPath, HighlightBoundsForUids } from '../../../core/shared/project-file-types' import { isParseSuccess, isTextFile } from '../../../core/shared/project-file-types' import { defaultIfNull, optionalFlatMap } from '../../../core/shared/optional-utils' @@ -125,6 +129,8 @@ export function createExecutionScope( ...topLevelComponentRendererComponentsForFile, } + let spiedVariablesInRoot: VariableData = {} + // First make sure everything is in scope if (combinedTopLevelArbitraryBlock != null && openStoryboardFileNameKILLME != null) { const { highlightBounds, code } = getCodeAndHighlightBoundsForFile(filePath, projectContents) @@ -164,7 +170,14 @@ export function createExecutionScope( ) applyBlockReturnFunctions(executionScope) - runBlockUpdatingScope(filePath, requireResult, combinedTopLevelArbitraryBlock, executionScope) + const { spiedVariablesDeclaredWithinBlock } = runBlockUpdatingScope( + { type: 'file-root' }, + filePath, + requireResult, + combinedTopLevelArbitraryBlock, + executionScope, + ) + spiedVariablesInRoot = spiedVariablesDeclaredWithinBlock } // WARNING: mutating the mutableContextRef updateMutableUtopiaCtxRefWithNewProps(mutableContextRef, { @@ -173,6 +186,7 @@ export function createExecutionScope( mutableContext: { requireResult: requireResult, rootScope: executionScope, + spiedVariablesDeclaredInRootScope: spiedVariablesInRoot, fileBlobs: fileBlobsForFile, jsxFactoryFunctionName: jsxFactoryFunction, }, diff --git a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-scope-utils.ts b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-scope-utils.ts index f4a5a96bf393..a4f17c8b880f 100644 --- a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-scope-utils.ts +++ b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-scope-utils.ts @@ -5,8 +5,11 @@ import { type ArbitraryJSBlock, } from '../../../core/shared/element-template' import { resolveParamsAndRunJsCode } from '../../../core/shared/javascript-cache' +import type { ElementPath } from '../../../core/shared/project-file-types' +import { type FileRootPath, type VariableData } from '../ui-jsx-canvas' export function runBlockUpdatingScope( + elementPath: ElementPath | FileRootPath | null, filePath: string, requireResult: MapLike, block: ArbitraryJSBlock, @@ -20,11 +23,18 @@ export function runBlockUpdatingScope( ) if (result.type === 'ARBITRARY_BLOCK_RAN_TO_END') { const definedWithinWithValues: MapLike = {} + const definedWithinVariableData: VariableData = {} for (const within of block.definedWithin) { currentScope[within] = result.scope[within] definedWithinWithValues[within] = result.scope[within] + if (elementPath != null) { + definedWithinVariableData[within] = { + spiedValue: result.scope[within], + insertionCeiling: elementPath, + } + } } - return arbitraryBlockRanToEnd(definedWithinWithValues) + return arbitraryBlockRanToEnd(definedWithinWithValues, definedWithinVariableData) } else { return result } diff --git a/editor/src/components/canvas/ui-jsx-canvas.tsx b/editor/src/components/canvas/ui-jsx-canvas.tsx index 74fbf3f6f913..92bf28f445e1 100644 --- a/editor/src/components/canvas/ui-jsx-canvas.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas.tsx @@ -101,9 +101,34 @@ export const ElementsToRerenderGLOBAL: { current: ElementsToRerender } = { current: 'rerender-all-elements', } +export type FileRootPath = { + type: 'file-root' +} + +export function insertionCeilingToString(insertionCeiling: ElementPath | FileRootPath): string { + if (insertionCeiling.type === 'file-root') { + return 'file-root' + } else { + return EP.toString(insertionCeiling) + } +} + +export function insertionCeilingsEqual( + a: ElementPath | FileRootPath, + b: ElementPath | FileRootPath, +): boolean { + if (a.type === 'file-root' && b.type === 'file-root') { + return true + } else if (a.type === 'file-root' || b.type === 'file-root') { + return false + } else { + return EP.pathsEqual(a, b) + } +} + export interface VariableMetadata { spiedValue: unknown - insertionCeiling: ElementPath + insertionCeiling: FileRootPath | ElementPath } export interface VariableData { diff --git a/editor/src/components/editor/actions/actions.spec.tsx b/editor/src/components/editor/actions/actions.spec.tsx index b654e46e26d7..5bb199009b99 100644 --- a/editor/src/components/editor/actions/actions.spec.tsx +++ b/editor/src/components/editor/actions/actions.spec.tsx @@ -683,7 +683,7 @@ describe('INSERT_INSERTABLE', () => { 'View', [], null, - null, + { type: 'file-root' }, null, ) diff --git a/editor/src/components/inspector/sections/component-section/data-picker-popup.tsx b/editor/src/components/inspector/sections/component-section/data-picker-popup.tsx index 45dc41120541..4092d61c4cc6 100644 --- a/editor/src/components/inspector/sections/component-section/data-picker-popup.tsx +++ b/editor/src/components/inspector/sections/component-section/data-picker-popup.tsx @@ -56,6 +56,6 @@ export function dataPickerFilterOptionToString(mode: DataPickerFilterOption): st } } -export const DataPickerPreferredAllAtom = atom('preferred') +export const DataPickerPreferredAllAtom = atom('all') export type DataPickerCallback = (e: JSExpressionOtherJavaScript) => void diff --git a/editor/src/components/inspector/sections/component-section/data-picker-utils.tsx b/editor/src/components/inspector/sections/component-section/data-picker-utils.tsx index 3b543b5331fe..544a3d578f61 100644 --- a/editor/src/components/inspector/sections/component-section/data-picker-utils.tsx +++ b/editor/src/components/inspector/sections/component-section/data-picker-utils.tsx @@ -14,13 +14,14 @@ import type { ElementPath } from '../../../../core/shared/project-file-types' import { assertNever } from '../../../../core/shared/utils' import type { AllElementProps } from '../../../editor/store/editor-state' import type { ArrayInfo, JSXInfo, ObjectInfo, PrimitiveInfo } from './variables-in-scope-utils' +import { insertionCeilingToString, type FileRootPath } from '../../../canvas/ui-jsx-canvas' interface VariableOptionBase { depth: number definedElsewhere: string valuePath: Array disabled: boolean - insertionCeiling: ElementPath | null + insertionCeiling: ElementPath | FileRootPath } export interface PrimitiveOption extends VariableOptionBase { @@ -81,7 +82,7 @@ export function getEnclosingScopes( metadata: ElementInstanceMetadataMap, allElementProps: AllElementProps, elementPathTree: ElementPathTrees, - buckets: Array, + buckets: Array, lowestInsertionCeiling: ElementPath, ): Array<{ insertionCeiling: ElementPath @@ -93,7 +94,7 @@ export function getEnclosingScopes( label: string hasContent: boolean }> = [] - const pathsToCheck = [...EP.allPathsInsideComponent(lowestInsertionCeiling)] + const pathsToCheck = EP.allPathsInsideComponent(lowestInsertionCeiling) for (const current of pathsToCheck) { const parentOfCurrent = EP.parentPath(current) @@ -110,13 +111,13 @@ export function getEnclosingScopes( elementPathTree, metadata, ), - hasContent: buckets.includes(current), + hasContent: buckets.includes(insertionCeilingToString(current)), }) continue } // we also add anything that has content in scope even if it's not a component or map - if (buckets.includes(current)) { + if (buckets.includes(insertionCeilingToString(current))) { result.unshift({ insertionCeiling: current, label: MetadataUtils.getElementLabel( @@ -135,7 +136,7 @@ export function getEnclosingScopes( result.unshift({ insertionCeiling: EP.emptyElementPath, label: 'File', - hasContent: buckets.includes(EP.emptyElementPath), + hasContent: buckets.includes(insertionCeilingToString({ type: 'file-root' })), }) return result diff --git a/editor/src/components/inspector/sections/component-section/data-selector-modal.tsx b/editor/src/components/inspector/sections/component-section/data-selector-modal.tsx index f4934041fe40..32c7e552f6e3 100644 --- a/editor/src/components/inspector/sections/component-section/data-selector-modal.tsx +++ b/editor/src/components/inspector/sections/component-section/data-selector-modal.tsx @@ -39,6 +39,8 @@ import type { ElementPath } from '../../../../core/shared/project-file-types' import * as EP from '../../../../core/shared/element-path' import { Substores, useEditorState } from '../../../editor/store/store-hook' import { optionalMap } from '../../../../core/shared/optional-utils' +import type { FileRootPath } from '../../../canvas/ui-jsx-canvas' +import { insertionCeilingToString, insertionCeilingsEqual } from '../../../canvas/ui-jsx-canvas' export const DataSelectorPopupBreadCrumbsTestId = 'data-selector-modal-top-bar' @@ -141,17 +143,15 @@ export const DataSelectorModal = React.memo( [allVariablesInScope], ) - const scopeBucketPaths = Object.keys(scopeBuckets).map((k) => EP.fromString(k)) - - const lowestMatchingScope = React.useMemo(() => { + const lowestMatchingScope: ElementPath | FileRootPath = React.useMemo(() => { if (lowestInsertionCeiling == null) { - return null + return { type: 'file-root' } } const matchingScope = findClosestMatchingScope(lowestInsertionCeiling, scopeBuckets) - return matchingScope ?? lowestInsertionCeiling + return matchingScope }, [scopeBuckets, lowestInsertionCeiling]) - const [selectedScope, setSelectedScope] = React.useState( + const [selectedScope, setSelectedScope] = React.useState( lowestMatchingScope, ) const setSelectedScopeCurried = React.useCallback( @@ -172,7 +172,7 @@ export const DataSelectorModal = React.memo( store.editor.jsxMetadata, store.editor.allElementProps, store.editor.elementPathTree, - scopeBucketPaths, + Object.keys(scopeBuckets), lowestInsertionCeiling ?? EP.emptyElementPath, ) return scopes.map(({ insertionCeiling, label, hasContent }) => ({ @@ -474,7 +474,7 @@ export const DataSelectorModal = React.memo( borderRadius: 4, cursor: 'pointer', fontSize: 12, - fontWeight: EP.pathsEqual(selectedScope, scope) ? 800 : undefined, + fontWeight: insertionCeilingsEqual(selectedScope, scope) ? 800 : undefined, }} > {label} @@ -624,22 +624,24 @@ type ScopeBuckets = { } function findClosestMatchingScope( - targetScope: ElementPath, + targetScope: ElementPath | FileRootPath, scopeBuckets: ScopeBuckets, -): ElementPath | null { - const allPaths = EP.allPathsInsideComponent(targetScope) - for (const path of allPaths) { - if (scopeBuckets[EP.toString(path)] != null) { - return path +): ElementPath | FileRootPath { + if (targetScope.type === 'elementpath') { + const allPaths = EP.allPathsInsideComponent(targetScope) + for (const path of allPaths) { + if (scopeBuckets[EP.toString(path)] != null) { + return path + } } } - return null + return { type: 'file-root' } } function putVariablesIntoScopeBuckets(options: DataPickerOption[]): ScopeBuckets { const buckets: { [insertionCeiling: string]: Array } = groupBy( - (o) => optionalMap(EP.toString, o.insertionCeiling) ?? '', // '' represents "file root scope", TODO make it clearer + (o) => insertionCeilingToString(o.insertionCeiling), options, ) @@ -649,17 +651,17 @@ function putVariablesIntoScopeBuckets(options: DataPickerOption[]): ScopeBuckets function useFilterVariablesInScope( options: DataPickerOption[], scopeBuckets: ScopeBuckets, - scopeToShow: ElementPath | null | 'do-not-filter', + scopeToShow: ElementPath | FileRootPath | 'do-not-filter', ): { filteredVariablesInScope: Array } { const filteredOptions = React.useMemo(() => { - if (scopeToShow === 'do-not-filter' || scopeToShow == null) { + if (scopeToShow === 'do-not-filter') { return options } const matchingScope = findClosestMatchingScope(scopeToShow, scopeBuckets) - return matchingScope == null ? [] : scopeBuckets[EP.toString(matchingScope)] + return scopeBuckets[insertionCeilingToString(matchingScope)] }, [scopeBuckets, options, scopeToShow]) return { diff --git a/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.spec.ts b/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.spec.ts index a037958e31b1..fca9a29899d0 100644 --- a/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.spec.ts +++ b/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.spec.ts @@ -1,5 +1,6 @@ import * as EP from '../../../../core/shared/element-path' import { maybeToArray } from '../../../../core/shared/optional-utils' +import { emptySet } from '../../../../core/shared/set-utils' import type { ControlDescription } from '../../../custom-code/internal-property-controls' import type { PropertyValue, VariableInfo } from './variables-in-scope-utils' import { orderVariablesForRelevance, variableInfoFromValue } from './variables-in-scope-utils' @@ -12,6 +13,7 @@ describe('orderVariablesForRelevance', () => { 'style', { left: 300, position: 'relative' }, EP.fromString('aaa'), + emptySet(), ), ) const controlDescription: ControlDescription = { @@ -98,6 +100,7 @@ describe('orderVariablesForRelevance', () => { 'style', { left: 300, position: 'relative' }, EP.fromString('aaa'), + emptySet(), ), ) const controlDescription: ControlDescription = { diff --git a/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.ts b/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.ts index 491cecf515f6..241238a52a00 100644 --- a/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.ts +++ b/editor/src/components/inspector/sections/component-section/variables-in-scope-utils.ts @@ -4,7 +4,7 @@ import type { ObjectControlDescription, } from '../../../custom-code/internal-property-controls' import type { ElementPath, PropertyPath } from '../../../../core/shared/project-file-types' -import type { VariableData } from '../../../canvas/ui-jsx-canvas' +import type { FileRootPath, VariableData } from '../../../canvas/ui-jsx-canvas' import { useEditorState, Substores } from '../../../editor/store/store-hook' import type { DataPickerFilterOption, DataPickerOption } from './data-picker-utils' import * as EP from '../../../../core/shared/element-path' @@ -15,10 +15,11 @@ import { mapDropNulls } from '../../../../core/shared/array-utils' import { assertNever, identity } from '../../../../core/shared/utils' import { isValidReactNode } from '../../../../utils/react-utils' import { MetadataUtils } from '../../../../core/model/element-metadata-utils' +import { emptySet } from '../../../../core/shared/set-utils' function valuesFromObject( variable: ArrayInfo | ObjectInfo, - insertionCeiling: ElementPath | null, + insertionCeiling: ElementPath | FileRootPath, depth: number, originalObjectName: string, valuePath: Array, @@ -75,7 +76,7 @@ function valuesFromObject( function valuesFromVariable( variable: VariableInfo, - insertionCeiling: ElementPath | null, + insertionCeiling: ElementPath | FileRootPath, depth: number, originalObjectName: string, valuePath: Array, @@ -136,7 +137,7 @@ interface VariableInfoBase { expression: string expressionPathPart: string | number value: unknown - insertionCeiling: ElementPath + insertionCeiling: ElementPath | FileRootPath matches: boolean } @@ -164,8 +165,16 @@ export function variableInfoFromValue( expression: string, expressionPathPart: string | number, value: unknown, - insertionCeiling: ElementPath, + insertionCeiling: ElementPath | FileRootPath, + valueStackSoFar: Set, ): VariableInfo | null { + if (valueStackSoFar.has(value)) { + // prevent circular dependencies + return null + } + // mutation + valueStackSoFar.add(value) + switch (typeof value) { case 'function': case 'symbol': @@ -203,7 +212,14 @@ export function variableInfoFromValue( insertionCeiling: insertionCeiling, matches: false, elements: mapDropNulls( - (e, idx) => variableInfoFromValue(`${expression}[${idx}]`, idx, e, insertionCeiling), + (e, idx) => + variableInfoFromValue( + `${expression}[${idx}]`, + idx, + e, + insertionCeiling, + valueStackSoFar, + ), value, ), } @@ -226,7 +242,13 @@ export function variableInfoFromValue( insertionCeiling: insertionCeiling, matches: false, props: mapDropNulls(([key, propValue]) => { - return variableInfoFromValue(`${expression}['${key}']`, key, propValue, insertionCeiling) + return variableInfoFromValue( + `${expression}['${key}']`, + key, + propValue, + insertionCeiling, + valueStackSoFar, + ) }, Object.entries(value)), } } @@ -235,7 +257,7 @@ export function variableInfoFromValue( function variableInfoFromVariableData(variableNamesInScope: VariableData): Array { const info = mapDropNulls( ([key, { spiedValue, insertionCeiling }]) => - variableInfoFromValue(key, key, spiedValue, insertionCeiling), + variableInfoFromValue(key, key, spiedValue, insertionCeiling, emptySet()), Object.entries(variableNamesInScope), ) @@ -368,7 +390,7 @@ const keepLocalestScope = (variablesInScope: VariableData): VariableData => { let deepestInsertionCeiling = -Infinity Object.values(variablesInScope).forEach((variable) => { - if (variable.insertionCeiling == null) { + if (variable.insertionCeiling.type === 'file-root') { return } @@ -380,7 +402,7 @@ const keepLocalestScope = const result: VariableData = {} Object.entries(variablesInScope).forEach(([key, variable]) => { - if (variable.insertionCeiling == null) { + if (variable.insertionCeiling.type === 'file-root') { result[key] = variable return } diff --git a/editor/src/components/navigator/navigator-item/component-picker-context-menu.tsx b/editor/src/components/navigator/navigator-item/component-picker-context-menu.tsx index 27964c394194..8d2ab5336569 100644 --- a/editor/src/components/navigator/navigator-item/component-picker-context-menu.tsx +++ b/editor/src/components/navigator/navigator-item/component-picker-context-menu.tsx @@ -683,7 +683,7 @@ function insertPreferredChild( uid, ['do-not-add'], null, - null, + { type: 'file-root' }, null, ) diff --git a/editor/src/components/navigator/navigator-item/component-picker.tsx b/editor/src/components/navigator/navigator-item/component-picker.tsx index b6924d36ba36..7f4933357eb0 100644 --- a/editor/src/components/navigator/navigator-item/component-picker.tsx +++ b/editor/src/components/navigator/navigator-item/component-picker.tsx @@ -21,6 +21,7 @@ import type { Size } from '../../../core/shared/math-utils' import { dataPasteHandler } from '../../../utils/paste-handler' import { sortBy } from '../../../core/shared/array-utils' import { iconPropsForIcon } from './component-picker-context-menu' +import type { FileRootPath } from '../../canvas/ui-jsx-canvas' const FILTER_CATEGORIES: Array = ['Everything'] @@ -48,7 +49,7 @@ export function elementToInsertToInsertableComponent( uid: string, stylePropOptions: Array, defaultSize: Size | null, - insertionCeiling: ElementPath | null, + insertionCeiling: ElementPath | FileRootPath, icon: Icon | null, ): InsertableComponent { const element = elementToInsert.elementToInsert(uid) diff --git a/editor/src/components/shared/__snapshots__/project-components.spec.ts.snap b/editor/src/components/shared/__snapshots__/project-components.spec.ts.snap index 379e0870f730..8f4fcbee4592 100644 --- a/editor/src/components/shared/__snapshots__/project-components.spec.ts.snap +++ b/editor/src/components/shared/__snapshots__/project-components.spec.ts.snap @@ -9,7 +9,9 @@ Array [ "element": [Function], "icon": null, "importsToAdd": Object {}, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "App", "stylePropOptions": Array [ "do-not-add", @@ -51,7 +53,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "div", "stylePropOptions": Array [ "do-not-add", @@ -76,7 +80,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "span", "stylePropOptions": Array [ "do-not-add", @@ -94,7 +100,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "h1", "stylePropOptions": Array [ "do-not-add", @@ -112,7 +120,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "h2", "stylePropOptions": Array [ "do-not-add", @@ -130,7 +140,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "p", "stylePropOptions": Array [ "do-not-add", @@ -148,7 +160,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "button", "stylePropOptions": Array [ "do-not-add", @@ -166,7 +180,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "input", "stylePropOptions": Array [ "do-not-add", @@ -184,7 +200,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "video", "stylePropOptions": Array [ "do-not-add", @@ -202,7 +220,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "img", "stylePropOptions": Array [ "do-not-add", @@ -221,7 +241,9 @@ Array [ "element": [Function], "icon": "component", "importsToAdd": Object {}, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "Conditional", "stylePropOptions": Array [ "do-not-add", @@ -245,7 +267,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "Fragment", "stylePropOptions": Array [ "do-not-add", @@ -279,7 +303,9 @@ Array [ "importedWithName": null, }, }, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "List", "stylePropOptions": Array [ "do-not-add", @@ -297,7 +323,9 @@ Array [ "element": [Function], "icon": "component", "importsToAdd": Object {}, - "insertionCeiling": null, + "insertionCeiling": Object { + "type": "file-root", + }, "name": "Sample text", "stylePropOptions": Array [ "do-not-add", diff --git a/editor/src/components/shared/project-components.ts b/editor/src/components/shared/project-components.ts index aec4bb5dab51..a6e25286b1b7 100644 --- a/editor/src/components/shared/project-components.ts +++ b/editor/src/components/shared/project-components.ts @@ -62,6 +62,7 @@ import { elementUsesProperty } from '../../core/model/element-template-utils' import { intrinsicHTMLElementNamesThatSupportChildren } from '../../core/shared/dom-utils' import { getTopLevelElementByExportsDetail } from '../../core/model/project-file-utils' import { type Icon } from 'utopia-api' +import type { FileRootPath } from '../canvas/ui-jsx-canvas' export type StylePropOption = 'do-not-add' | 'add-size' @@ -71,7 +72,7 @@ export interface InsertableComponent { name: string stylePropOptions: Array defaultSize: Size | null - insertionCeiling: ElementPath | null + insertionCeiling: ElementPath | FileRootPath icon: Icon | null } @@ -81,7 +82,7 @@ export function insertableComponent( name: string, stylePropOptions: Array, defaultSize: Size | null, - insertionCeiling: ElementPath | null, + insertionCeiling: ElementPath | FileRootPath, icon: Icon | null, ): InsertableComponent { const component = { @@ -305,7 +306,7 @@ export function insertableVariable( variableType: InsertableVariableType, depth: number, originalName: string, - insertionCeiling: ElementPath | null, + insertionCeiling: ElementPath | FileRootPath, ): InsertableVariable { return { ...insertableComponent( @@ -653,7 +654,7 @@ export function moveSceneToTheBeginningAndSetDefaultSize( scene.name, scene.stylePropOptions, size(SceneDefaultWidth, SceneDefaultHeight), - null, + { type: 'file-root' }, null, ), ], @@ -744,7 +745,7 @@ export function getComponentGroups( insertOption.insertMenuLabel, stylePropOptions, null, - null, + { type: 'file-root' }, descriptor.icon, ), ) @@ -760,7 +761,7 @@ export function getComponentGroups( exportedComponent.listingName, stylePropOptions, null, - null, + { type: 'file-root' }, null, ), ) @@ -798,7 +799,7 @@ export function getComponentGroups( insertOption.insertMenuLabel, stylePropOptions, defaultSize ?? null, - null, + { type: 'file-root' }, component.icon, ), ) diff --git a/editor/src/components/shared/scoped-variables.ts b/editor/src/components/shared/scoped-variables.ts index e15c33c15bfb..7d2eb21e77b2 100644 --- a/editor/src/components/shared/scoped-variables.ts +++ b/editor/src/components/shared/scoped-variables.ts @@ -23,7 +23,7 @@ import { isJSXAttributeValue, simplifyAttributeIfPossible, } from '../../core/shared/element-template' -import type { VariableData } from '../canvas/ui-jsx-canvas' +import type { FileRootPath, VariableData } from '../canvas/ui-jsx-canvas' import { type VariablesInScope } from '../canvas/ui-jsx-canvas' import { getContainingComponent, toComponentId } from '../../core/shared/element-path' import { @@ -327,5 +327,5 @@ interface Variable { value?: unknown parent?: Variable depth: number - insertionCeiling: ElementPath | null + insertionCeiling: FileRootPath | ElementPath } diff --git a/editor/src/core/data-tracing/data-tracing.ts b/editor/src/core/data-tracing/data-tracing.ts index 43707f56b346..4c4b2fccd52c 100644 --- a/editor/src/core/data-tracing/data-tracing.ts +++ b/editor/src/core/data-tracing/data-tracing.ts @@ -1,4 +1,5 @@ import type { ProjectContentTreeRoot } from '../../components/assets' +import type { FileRootPath } from '../../components/canvas/ui-jsx-canvas' import { findUnderlyingTargetComponentImplementationFromImportInfo } from '../../components/custom-code/code-file' import { withUnderlyingTarget } from '../../components/editor/store/editor-state' import * as TPP from '../../components/template-property-path' @@ -428,12 +429,15 @@ export function traceDataFromElement( } export function traceDataFromVariableName( - enclosingScope: ElementPath, + enclosingScope: ElementPath | FileRootPath, variableName: string, metadata: ElementInstanceMetadataMap, projectContents: ProjectContentTreeRoot, pathDrillSoFar: DataPathPositiveResult, ): DataTracingResult { + if (enclosingScope.type === 'file-root') { + return dataTracingFailed('Cannot trace data from variable name in file root') + } const componentHoldingElement = findContainingComponentForElementPath( enclosingScope, projectContents, diff --git a/editor/src/core/model/element-metadata.spec.browser2.tsx b/editor/src/core/model/element-metadata.spec.browser2.tsx index 8143dcb3356d..48dc9c92dfd9 100644 --- a/editor/src/core/model/element-metadata.spec.browser2.tsx +++ b/editor/src/core/model/element-metadata.spec.browser2.tsx @@ -1400,6 +1400,10 @@ describe('record variable values', () => { const editor = await renderTestEditorWithCode(ProjectWithVariables, 'await-first-dom-report') const { variablesInScope } = editor.getEditorState().editor expect(variablesInScope['sb/scene/pg:root']).toEqual({ + add: { + insertionCeiling: { type: 'file-root' }, + spiedValue: expect.any(Function), + }, definedInsideNumber: { spiedValue: 12, insertionCeiling: EP.fromString('sb/scene/pg:root') }, definedInsideObject: { spiedValue: { @@ -1415,6 +1419,10 @@ describe('record variable values', () => { style: { spiedValue: {}, insertionCeiling: EP.fromString('sb/scene/pg:root') }, }) expect(variablesInScope['sb/scene/pg:root/111']).toEqual({ + add: { + insertionCeiling: { type: 'file-root' }, + spiedValue: expect.any(Function), + }, definedInsideNumber: { spiedValue: 12, insertionCeiling: EP.fromString('sb/scene/pg:root') }, definedInsideObject: { spiedValue: { @@ -1430,6 +1438,10 @@ describe('record variable values', () => { style: { spiedValue: {}, insertionCeiling: EP.fromString('sb/scene/pg:root') }, }) expect(variablesInScope['sb/scene/pg:root/222']).toEqual({ + add: { + insertionCeiling: { type: 'file-root' }, + spiedValue: expect.any(Function), + }, definedInsideNumber: { spiedValue: 12, insertionCeiling: EP.fromString('sb/scene/pg:root') }, definedInsideObject: { spiedValue: { @@ -1445,6 +1457,10 @@ describe('record variable values', () => { style: { spiedValue: {}, insertionCeiling: EP.fromString('sb/scene/pg:root') }, }) expect(variablesInScope['sb/scene/pg:root/333']).toEqual({ + add: { + insertionCeiling: { type: 'file-root' }, + spiedValue: expect.any(Function), + }, definedInsideNumber: { spiedValue: 12, insertionCeiling: EP.fromString('sb/scene/pg:root') }, definedInsideObject: { spiedValue: { diff --git a/editor/src/core/shared/element-template.ts b/editor/src/core/shared/element-template.ts index 806ec416dd0d..072e1180260c 100644 --- a/editor/src/core/shared/element-template.ts +++ b/editor/src/core/shared/element-template.ts @@ -33,6 +33,7 @@ import { allComments } from './comment-flags' import type { Optic } from './optics/optics' import { fromField } from './optics/optic-creators' import { jsxSimpleAttributeToValue } from './jsx-attribute-utils' +import type { VariableData } from '../../components/canvas/ui-jsx-canvas' export interface ParsedComments { leadingComments: Array @@ -2627,35 +2628,44 @@ export type ConditionValue = ActiveAndDefaultConditionValues | 'not-a-conditiona export interface EarlyReturnVoid { type: 'EARLY_RETURN_VOID' + spiedVariablesDeclaredWithinBlock: VariableData } export function earlyReturnVoid(): EarlyReturnVoid { return { type: 'EARLY_RETURN_VOID', + spiedVariablesDeclaredWithinBlock: {}, } } export interface EarlyReturnResult { type: 'EARLY_RETURN_RESULT' result: unknown + spiedVariablesDeclaredWithinBlock: VariableData } export function earlyReturnResult(result: unknown): EarlyReturnResult { return { type: 'EARLY_RETURN_RESULT', result: result, + spiedVariablesDeclaredWithinBlock: {}, } } export interface ArbitraryBlockRanToEnd { type: 'ARBITRARY_BLOCK_RAN_TO_END' scope: MapLike + spiedVariablesDeclaredWithinBlock: VariableData } -export function arbitraryBlockRanToEnd(scope: MapLike): ArbitraryBlockRanToEnd { +export function arbitraryBlockRanToEnd( + scope: MapLike, + spiedVariablesDeclaredWithinBlock: VariableData, +): ArbitraryBlockRanToEnd { return { type: 'ARBITRARY_BLOCK_RAN_TO_END', scope: scope, + spiedVariablesDeclaredWithinBlock: spiedVariablesDeclaredWithinBlock, } }