Skip to content

Commit

Permalink
Data Picker: be able to pick global variables (#5790)
Browse files Browse the repository at this point in the history
**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
  • Loading branch information
balazsbajorics authored Jun 4, 2024
1 parent febab34 commit 47ac149
Show file tree
Hide file tree
Showing 21 changed files with 401 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<CanvasCommand>
Expand Down Expand Up @@ -614,10 +615,10 @@ export function applyElementCeilingToReparentTarget(
elementsToInsert: JSXElementChild[],
elementPathTree: ElementPathTrees,
reparentTarget: Either<PasteParentNotFoundError, ReparentTargetForPaste>,
elementCeiling: ElementPath | null,
elementCeiling: ElementPath | FileRootPath,
propertyControlsInfo: PropertyControlsInfo,
): Either<PasteParentNotFoundError, ReparentTargetForPaste> {
if (elementCeiling == null) {
if (elementCeiling.type === 'file-root') {
return reparentTarget
} else {
return flatMapEither((targetForPaste) => {
Expand Down Expand Up @@ -669,7 +670,7 @@ export function getTargetParentForOneShotInsertion(
metadata: ElementInstanceMetadataMap,
elementsToInsert: JSXElementChild[],
elementPathTree: ElementPathTrees,
insertionCeiling: ElementPath | null,
insertionCeiling: ElementPath | FileRootPath,
propertyControlsInfo: PropertyControlsInfo,
): Either<PasteParentNotFoundError, ReparentTargetForPaste> {
if (!isNonEmptyArray(selectedViews)) {
Expand Down
177 changes: 176 additions & 1 deletion editor/src/components/canvas/scoped-variables.spec.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -9,14 +13,20 @@ 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 {
DefaultPackageJson,
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 {
Expand Down Expand Up @@ -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 = (
<Storyboard data-uid='storyboard-entity'>
<Scene
data-label='Imported App'
data-uid='scene-1-entity'
style={{ position: 'absolute', left: 0, top: 0, width: 375, height: 812 }}
>
<App data-uid='app-entity' />
</Scene>
</Storyboard>
)`,
),
'/src/app.js': createCodeFile(
`import * as React from 'react'
const globalVar = ['alap']
export var App = ({ style }) => {
const localVar = ['local']
return (
<div
data-uid='app-root'
style={{
height: '100%',
width: '100%',
contain: 'layout',
...style,
}}
>
{
// @utopia/uid=globalVarMap
globalVar.map((global) => (
<div data-uid='div1'>
{
// @utopia/uid=localVarMap
localVar.map((local) => (
<div data-uid='div2'>
{
// @utopia/uid=firstMap
['a', 'b', 'c'].map((first) => (
<div data-uid='div3'>
{
// @utopia/uid=secondMap
[1, 2, 3].map((second) => (
<div data-uid='div4'>
{(() => {
const innerVar = ['cica', 'kutya']
// @utopia/uid=innerVarMap
return innerVar.map((inner) => {
return (
<div data-uid='div5'>
{global} -{local} -{first} -
{second} -{inner}
</div>
)
})
})()}
</div>
))}
</div>
))}
</div>
))}
</div>
))}
</div>
)
}
`,
),
})
}

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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -158,7 +160,7 @@ export function createComponentRendererComponent(params: {
code: code,
highlightBounds: highlightBounds,
editedText: rerenderUtopiaContext.editedText,
variablesInScope: {},
variablesInScope: mutableContext.spiedVariablesDeclaredInRootScope,
filePathMappings: rerenderUtopiaContext.filePathMappings,
},
undefined,
Expand All @@ -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
Expand Down Expand Up @@ -259,6 +257,7 @@ export function createComponentRendererComponent(params: {
applyBlockReturnFunctions(scope)

const arbitraryBlockResult = runBlockUpdatingScope(
rootElementPath,
params.filePath,
mutableContext.requireResult,
utopiaJsxComponent.arbitraryJSBlock,
Expand All @@ -270,6 +269,7 @@ export function createComponentRendererComponent(params: {
if (rootElementPath != null) {
spiedVariablesInScope = {
...spiedVariablesInScope,
...arbitraryBlockResult.spiedVariablesDeclaredWithinBlock,
...objectMap(
(spiedValue) => ({
spiedValue: spiedValue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -15,6 +16,7 @@ export interface MutableUtopiaCtxRefData {
requireResult: MapLike<any>
fileBlobs: UIFileBase64Blobs
rootScope: MapLike<any>
spiedVariablesDeclaredInRootScope: VariableData
jsxFactoryFunctionName: string | null
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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, {
Expand All @@ -173,6 +186,7 @@ export function createExecutionScope(
mutableContext: {
requireResult: requireResult,
rootScope: executionScope,
spiedVariablesDeclaredInRootScope: spiedVariablesInRoot,
fileBlobs: fileBlobsForFile,
jsxFactoryFunctionName: jsxFactoryFunction,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>,
block: ArbitraryJSBlock,
Expand All @@ -20,11 +23,18 @@ export function runBlockUpdatingScope(
)
if (result.type === 'ARBITRARY_BLOCK_RAN_TO_END') {
const definedWithinWithValues: MapLike<any> = {}
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
}
Expand Down
Loading

0 comments on commit 47ac149

Please sign in to comment.