From 0049b907a05bf43e3a727839a58dbac6281408a0 Mon Sep 17 00:00:00 2001 From: Sean Parsons <217400+seanparsons@users.noreply.github.com> Date: Wed, 11 Sep 2024 12:49:50 +0100 Subject: [PATCH] performance(editor) Improved `buildTree`. (#6325) - Reworked `buildTree_MUTATE` to build the tree from the root downwards, combining it with some of the logic taken from `getChildrenPaths`. - Further improvement to `isDescendantOf` replacing the previous implementation. - `getReorderedPaths` now avoids a lot of repeated logic for checking the element type. - Broke apart `ElementPathTree.children` into two separate fields to make for easier ordering. - Added `getElementPathTreeChildren` and `forEachElementPathTreeChild` utility functions. - `buildTree_MUTATE` now adds to the two different children fields based on the element path representing inner children over property children. - `getReorderedIndexInPaths` further reworked to handle shifting by the siblings. - Fixed some tests that had expressions incorrectly ordered. --- editor/src/benchmarks.benchmark.ts | 4 +- editor/src/components/canvas/canvas.ts | 4 +- .../controls/select-mode/scene-label.tsx | 4 +- .../canvas/dom-sampler.spec.browser2.tsx | 6 +- editor/src/components/canvas/gap-utils.ts | 8 +- .../store/store-deep-equality-instances.ts | 6 +- .../components/navigator/navigator-utils.ts | 18 +- .../navigator/navigator.spec.browser2.tsx | 6 +- .../model/element-metadata-utils.spec.tsx | 14 + .../src/core/model/element-metadata-utils.ts | 19 +- .../shared/element-path-tree.benchmark.ts | 56 ++++ .../element-path-tree.spec.browser2.tsx | 6 +- editor/src/core/shared/element-path-tree.ts | 283 ++++++++++-------- editor/src/core/shared/element-path.ts | 50 +++- 14 files changed, 324 insertions(+), 160 deletions(-) diff --git a/editor/src/benchmarks.benchmark.ts b/editor/src/benchmarks.benchmark.ts index 4cb01c8292cf..57192a78c629 100644 --- a/editor/src/benchmarks.benchmark.ts +++ b/editor/src/benchmarks.benchmark.ts @@ -5,9 +5,9 @@ import { benchmarkOptics } from './core/shared/optics.benchmark' import { benchmarkPropertyPathFunction } from './core/shared/property-path.benchmark' import { benchmarkGetUniqueUids } from './core/model/get-unique-ids.benchmark' -// await benchmarkBuildTree() +await benchmarkBuildTree() // await benchmarkElementPathFunction() // await benchmarkPropertyPathFunction() // await benchmarkAttributes() // await benchmarkOptics() -await benchmarkGetUniqueUids() +// await benchmarkGetUniqueUids() diff --git a/editor/src/components/canvas/canvas.ts b/editor/src/components/canvas/canvas.ts index ca9b4009d4db..d59c126e1400 100644 --- a/editor/src/components/canvas/canvas.ts +++ b/editor/src/components/canvas/canvas.ts @@ -18,7 +18,7 @@ import type { } from '../editor/store/editor-state' import * as EP from '../../core/shared/element-path' import type { ElementPathTree, ElementPathTrees } from '../../core/shared/element-path-tree' -import { getSubTree } from '../../core/shared/element-path-tree' +import { forEachElementPathTreeChild, getSubTree } from '../../core/shared/element-path-tree' import { assertNever, fastForEach } from '../../core/shared/utils' import { memoize } from '../../core/shared/memoize' import { maybeToArray } from '../../core/shared/optional-utils' @@ -73,7 +73,7 @@ function getFramesInCanvasContextUncached( let children: Array = [] let unfurledComponents: Array = [] - fastForEach(Object.values(componentTree.children), (childTree) => { + forEachElementPathTreeChild(componentTree, (childTree) => { if (EP.isRootElementOfInstance(childTree.path)) { unfurledComponents.push(childTree) } else { diff --git a/editor/src/components/canvas/controls/select-mode/scene-label.tsx b/editor/src/components/canvas/controls/select-mode/scene-label.tsx index fc5db3cd09f0..36e9227f4157 100644 --- a/editor/src/components/canvas/controls/select-mode/scene-label.tsx +++ b/editor/src/components/canvas/controls/select-mode/scene-label.tsx @@ -18,7 +18,7 @@ import { boundingArea, createInteractionViaMouse } from '../../canvas-strategies import { windowToCanvasCoordinates } from '../../dom-lookup' import { CanvasOffsetWrapper } from '../canvas-offset-wrapper' import { isCommentMode, isSelectModeWithArea } from '../../../editor/editor-modes' -import { getSubTree } from '../../../../core/shared/element-path-tree' +import { getElementPathTreeChildren, getSubTree } from '../../../../core/shared/element-path-tree' interface SceneLabelControlProps { maybeHighlightOnHover: (target: ElementPath) => void @@ -65,7 +65,7 @@ const SceneLabel = React.memo((props) => { if (subTree == null) { return false } else { - return subTree.children.length === 1 + return getElementPathTreeChildren(subTree).length === 1 } }, 'SceneLabel sceneHasSingleChild', diff --git a/editor/src/components/canvas/dom-sampler.spec.browser2.tsx b/editor/src/components/canvas/dom-sampler.spec.browser2.tsx index f1ca07c54a40..3b93654cda41 100644 --- a/editor/src/components/canvas/dom-sampler.spec.browser2.tsx +++ b/editor/src/components/canvas/dom-sampler.spec.browser2.tsx @@ -296,16 +296,14 @@ export var storyboard = ( navigatorEntryToKey, ), ).toEqual([ - 'regular-sb/1e7', 'regular-sb/sc', 'regular-sb/sc/app', 'regular-sb/sc/app:app-root', 'regular-sb/sc/app:app-root/card', 'regular-sb/sc/app:app-root/card:card-root', - 'regular-sb/sc/app:app-root/card:card-root/30d', 'regular-sb/sc/app:app-root/card:card-root/card-span', + 'regular-sb/sc/app:app-root/card:card-root/30d', 'regular-sb/sc/app:app-root/card/card-child', - 'regular-sb/sc/app:app-root/children-code-block', 'regular-sb/sc/app:app-root/frag', 'regular-sb/sc/app:app-root/frag/frag-child', 'regular-sb/sc/app:app-root/frag/cond-1', @@ -319,7 +317,9 @@ export var storyboard = ( 'synthetic-sb/sc/app:app-root/frag/cond-1/cond-1-true/cond-2/d84-attribute', 'conditional-clause-sb/sc/app:app-root/frag/cond-1-false-case', 'synthetic-sb/sc/app:app-root/frag/cond-1/019-attribute', + 'regular-sb/sc/app:app-root/children-code-block', 'regular-sb/sc/app/app-child', + 'regular-sb/1e7', ]) }) diff --git a/editor/src/components/canvas/gap-utils.ts b/editor/src/components/canvas/gap-utils.ts index 99a70eeae5c4..c30454a2afeb 100644 --- a/editor/src/components/canvas/gap-utils.ts +++ b/editor/src/components/canvas/gap-utils.ts @@ -22,7 +22,11 @@ import type { CSSNumber, FlexDirection } from '../inspector/common/css-utils' import type { Sides } from 'utopia-api/core' import { sides } from 'utopia-api/core' import { styleStringInArray } from '../../utils/common-constants' -import { getSubTree, type ElementPathTrees } from '../../core/shared/element-path-tree' +import { + getElementPathTreeChildren, + getSubTree, + type ElementPathTrees, +} from '../../core/shared/element-path-tree' import { isReversedFlexDirection } from '../../core/model/flex-utils' import * as EP from '../../core/shared/element-path' import { treatElementAsFragmentLike } from './canvas-strategies/strategies/fragment-like-helpers' @@ -380,7 +384,7 @@ export function recurseIntoChildrenOfMapOrFragment( return [] } - return subTree.children.flatMap((element) => { + return getElementPathTreeChildren(subTree).flatMap((element) => { const elementPath = element.path if (EP.isRootElementOfInstance(elementPath)) { return [] diff --git a/editor/src/components/editor/store/store-deep-equality-instances.ts b/editor/src/components/editor/store/store-deep-equality-instances.ts index 6f3b13d23626..e19d4f36fab4 100644 --- a/editor/src/components/editor/store/store-deep-equality-instances.ts +++ b/editor/src/components/editor/store/store-deep-equality-instances.ts @@ -2996,12 +2996,14 @@ export function ElementPathTreeKeepDeepEquality( oldValue: ElementPathTree, newValue: ElementPathTree, ): KeepDeepEqualityResult { - return combine3EqualityCalls( + return combine4EqualityCalls( (pathTree) => pathTree.path, ElementPathKeepDeepEquality, (pathTree) => pathTree.pathString, createCallWithTripleEquals(), - (pathTree) => pathTree.children, + (pathTree) => pathTree.innerChildren, + arrayDeepEquality(ElementPathTreeKeepDeepEquality), + (pathTree) => pathTree.propsChildren, arrayDeepEquality(ElementPathTreeKeepDeepEquality), elementPathTree, )(oldValue, newValue) diff --git a/editor/src/components/navigator/navigator-utils.ts b/editor/src/components/navigator/navigator-utils.ts index c3a8c358566e..d84324561244 100644 --- a/editor/src/components/navigator/navigator-utils.ts +++ b/editor/src/components/navigator/navigator-utils.ts @@ -37,7 +37,11 @@ import { syntheticNavigatorEntry, } from '../editor/store/editor-state' import type { ElementPathTree, ElementPathTrees } from '../../core/shared/element-path-tree' -import { getCanvasRoots, getSubTree } from '../../core/shared/element-path-tree' +import { + getCanvasRoots, + getElementPathTreeChildren, + getSubTree, +} from '../../core/shared/element-path-tree' import { assertNever } from '../../core/shared/utils' import type { ConditionalCase } from '../../core/model/conditionals' import { getConditionalClausePath } from '../../core/model/conditionals' @@ -320,7 +324,9 @@ function walkRegularNavigatorEntry( const childPath = EP.appendToPath(elementPath, getUtopiaID(propValue)) - const subTreeChild = subTree?.children.find((child) => EP.pathsEqual(child.path, childPath)) + const subTreeChild = getElementPathTreeChildren(subTree).find((child) => + EP.pathsEqual(child.path, childPath), + ) if (subTreeChild != null) { const childTreeEntry = createNavigatorSubtree( metadata, @@ -359,7 +365,7 @@ function walkRegularNavigatorEntry( }) } - const childrenPaths = subTree.children.filter( + const childrenPaths = getElementPathTreeChildren(subTree).filter( (child) => !processedAccumulator.has(EP.toString(child.path)), ) const children: Array = mapDropNulls((child) => { @@ -457,7 +463,7 @@ function walkConditionalClause( const branch = conditionalCase === 'true-case' ? conditional.whenTrue : conditional.whenFalse // Walk the clause of the conditional. - const clausePathTrees = Object.values(conditionalSubTree.children).filter((childPath) => { + const clausePathTrees = getElementPathTreeChildren(conditionalSubTree).filter((childPath) => { if (isDynamic(childPath.path) && hasElementsWithin(branch)) { for (const element of Object.values(branch.elementsWithin)) { const firstChildPath = EP.appendToPath(EP.parentPath(clausePath), element.uid) @@ -513,7 +519,7 @@ function walkMapExpression( const commentFlag = findUtopiaCommentFlag(element.comments, 'map-count') const mapCountOverride = isUtopiaPropOrCommentFlagMapCount(commentFlag) ? commentFlag.value : null - const mappedChildren = Object.values(subTree.children).map((child) => + const mappedChildren = getElementPathTreeChildren(subTree).map((child) => createNavigatorSubtree( metadata, elementPathTrees, @@ -531,7 +537,7 @@ function walkMapExpression( } let invalidEntries: Array = [] - for (let i = Object.values(subTree.children).length; i < mapCountOverride; i++) { + for (let i = getElementPathTreeChildren(subTree).length; i < mapCountOverride; i++) { const entry = invalidOverrideNavigatorEntry( EP.appendToPath(subTree.path, `invalid-override-${i + 1}`), 'data source not found', diff --git a/editor/src/components/navigator/navigator.spec.browser2.tsx b/editor/src/components/navigator/navigator.spec.browser2.tsx index 2da3ac334c1d..e8911f87814c 100644 --- a/editor/src/components/navigator/navigator.spec.browser2.tsx +++ b/editor/src/components/navigator/navigator.spec.browser2.tsx @@ -5495,16 +5495,14 @@ describe('Navigator row order', () => { navigatorEntryToKey, ), ).toEqual([ - 'regular-sb/1e7', 'regular-sb/sc', 'regular-sb/sc/app', 'regular-sb/sc/app:app-root', 'regular-sb/sc/app:app-root/card', 'regular-sb/sc/app:app-root/card:card-root', - 'regular-sb/sc/app:app-root/card:card-root/30d', 'regular-sb/sc/app:app-root/card:card-root/card-span', + 'regular-sb/sc/app:app-root/card:card-root/30d', 'regular-sb/sc/app:app-root/card/card-child', - 'regular-sb/sc/app:app-root/children-code-block', 'regular-sb/sc/app:app-root/frag', 'regular-sb/sc/app:app-root/frag/frag-child', 'regular-sb/sc/app:app-root/frag/cond-1', @@ -5518,7 +5516,9 @@ describe('Navigator row order', () => { 'synthetic-sb/sc/app:app-root/frag/cond-1/cond-1-true/cond-2/d84-attribute', 'conditional-clause-sb/sc/app:app-root/frag/cond-1-false-case', 'synthetic-sb/sc/app:app-root/frag/cond-1/019-attribute', + 'regular-sb/sc/app:app-root/children-code-block', 'regular-sb/sc/app/app-child', + 'regular-sb/1e7', ]) }) diff --git a/editor/src/core/model/element-metadata-utils.spec.tsx b/editor/src/core/model/element-metadata-utils.spec.tsx index 49ab92bb84da..695b9a2de175 100644 --- a/editor/src/core/model/element-metadata-utils.spec.tsx +++ b/editor/src/core/model/element-metadata-utils.spec.tsx @@ -1101,6 +1101,7 @@ describe('getElementLabel', () => { spanElementMetadata.elementPath, EP.toString(spanElementMetadata.elementPath), [], + [], ), [EP.toString(divElementMetadata.elementPath)]: elementPathTree( divElementMetadata.elementPath, @@ -1110,8 +1111,10 @@ describe('getElementLabel', () => { spanElementMetadata.elementPath, EP.toString(spanElementMetadata.elementPath), [], + [], ), ], + [], ), } it('the label of a spin containing text is that text', () => { @@ -1138,16 +1141,19 @@ describe('getting the root paths', () => { testComponentSceneElement.elementPath, EP.toString(testComponentSceneElement.elementPath), [], + [], ) const testStoryboardChildTree = elementPathTree( testStoryboardChildElement.elementPath, EP.toString(testStoryboardChildElement.elementPath), [], + [], ) const storyboardTree = elementPathTree( EP.elementPath([[BakedInStoryboardUID]]), EP.toString(EP.elementPath([[BakedInStoryboardUID]])), [testComponentSceneTree, testStoryboardChildTree], + [], ) const pathTrees: ElementPathTrees = { [EP.toString(EP.elementPath([[BakedInStoryboardUID]]))]: storyboardTree, @@ -1170,41 +1176,49 @@ describe('getting the root paths', () => { testComponentSceneChildElement.elementPath, EP.toString(testComponentSceneChildElement.elementPath), [], + [], ) const testComponentChild1Tree = elementPathTree( testComponentMetadataChild1.elementPath, EP.toString(testComponentMetadataChild1.elementPath), [], + [], ) const testComponentChild2Tree = elementPathTree( testComponentMetadataChild2.elementPath, EP.toString(testComponentMetadataChild2.elementPath), [], + [], ) const testComponentChild3Tree = elementPathTree( testComponentMetadataChild3.elementPath, EP.toString(testComponentMetadataChild3.elementPath), [], + [], ) const testComponentRoot1Tree = elementPathTree( testComponentRoot1.elementPath, EP.toString(testComponentRoot1.elementPath), [testComponentChild1Tree, testComponentChild2Tree, testComponentChild3Tree], + [], ) const testComponentSceneTree = elementPathTree( testComponentSceneElement.elementPath, EP.toString(testComponentSceneElement.elementPath), [testComponentSceneChildTree, testComponentRoot1Tree], + [], ) const testStoryboardChildTree = elementPathTree( testStoryboardChildElement.elementPath, EP.toString(testStoryboardChildElement.elementPath), [], + [], ) const storyboardTree = elementPathTree( EP.elementPath([[BakedInStoryboardUID]]), EP.toString(EP.elementPath([[BakedInStoryboardUID]])), [testComponentSceneChildTree, testStoryboardChildTree], + [], ) const pathTrees: ElementPathTrees = { [EP.toString(EP.elementPath([[BakedInStoryboardUID]]))]: storyboardTree, diff --git a/editor/src/core/model/element-metadata-utils.ts b/editor/src/core/model/element-metadata-utils.ts index de4faaa17ad5..6c089afe8b5f 100644 --- a/editor/src/core/model/element-metadata-utils.ts +++ b/editor/src/core/model/element-metadata-utils.ts @@ -138,7 +138,15 @@ import { import type { ProjectContentTreeRoot } from '../../components/assets' import { memoize } from '../shared/memoize' import type { ElementPathTree, ElementPathTrees } from '../shared/element-path-tree' -import { buildTree, getSubTree, getCanvasRoots, elementPathTree } from '../shared/element-path-tree' +import { + buildTree, + getSubTree, + getCanvasRoots, + elementPathTree, + getElementPathTreeChildren, + forEachElementPathTreeChild, + printTree, +} from '../shared/element-path-tree' import type { PropertyControlsInfo } from '../../components/custom-code/code-file' import { findUnderlyingTargetComponentImplementationFromImportInfo } from '../../components/custom-code/code-file' import type { @@ -323,7 +331,6 @@ export const MetadataUtils = { if (target == null) { return [] } - const parentPath = EP.parentPath(target) const siblingPathsOrNull = EP.isRootElementOfInstance(target) ? MetadataUtils.getRootViewPathsOrdered(metadata, pathTree, parentPath) @@ -724,7 +731,7 @@ export const MetadataUtils = { if (subTree == null) { return [] } else { - return subTree.children + return getElementPathTreeChildren(subTree) .map((child) => child.path) .filter((path) => !EP.isRootElementOfInstance(path)) } @@ -845,7 +852,7 @@ export const MetadataUtils = { let result: Array = [] function recurseElement(tree: ElementPathTree): void { result.push(tree.path) - fastForEach(Object.values(tree.children), (childTree) => { + forEachElementPathTreeChild(tree, (childTree) => { recurseElement(childTree) }) } @@ -878,7 +885,7 @@ export const MetadataUtils = { if (tree != null) { result.push(tree.path) - fastForEach(Object.values(tree.children), (childTree) => { + forEachElementPathTreeChild(tree, (childTree) => { recurseElement(childTree) }) } @@ -1375,7 +1382,7 @@ export const MetadataUtils = { let unfurledComponents: Array = [] - fastForEach(Object.values(subTree.children), (child) => { + forEachElementPathTreeChild(subTree, (child) => { if (EP.isRootElementOfInstance(child.path)) { unfurledComponents.push(child) } else { diff --git a/editor/src/core/shared/element-path-tree.benchmark.ts b/editor/src/core/shared/element-path-tree.benchmark.ts index cc55e77ac41e..ffd7351e1ff3 100644 --- a/editor/src/core/shared/element-path-tree.benchmark.ts +++ b/editor/src/core/shared/element-path-tree.benchmark.ts @@ -17,6 +17,62 @@ function dummyMetadataFromPaths(elementPaths: ElementPath[]): ElementInstanceMet } export async function benchmarkBuildTree(): Promise { + await Benny.suite( + 'isDescendantOf - for immediate descendant', + Benny.add('for immediate descendant', () => { + const path1 = EP.fromString(`aaa/bbb/ccc`) + const path2 = EP.fromString(`aaa/bbb/ccc:ddd`) + + return () => { + EP.isDescendantOf(path2, path1) + } + }), + Benny.cycle(), + Benny.complete(), + Benny.save({ file: 'buildTree immediate descendant', details: true }), + ) + await Benny.suite( + 'isDescendantOf - non-descendant', + Benny.add('non-descendant', () => { + const path1 = EP.fromString(`aaa/bbb/ccc`) + const path2 = EP.fromString(`aaa/bbb/ddd`) + + return () => { + EP.isDescendantOf(path2, path1) + } + }), + Benny.cycle(), + Benny.complete(), + Benny.save({ file: 'buildTree non-descendant', details: true }), + ) + await Benny.suite( + 'isChildOf - child', + Benny.add('child', () => { + const path1 = EP.fromString(`aaa/bbb/ccc`) + const path2 = EP.fromString(`aaa/bbb/ccc:ddd`) + + return () => { + EP.isChildOf(path2, path1) + } + }), + Benny.cycle(), + Benny.complete(), + Benny.save({ file: 'isChildOf child', details: true }), + ) + await Benny.suite( + 'isChildOf - not child', + Benny.add('not child', () => { + const path1 = EP.fromString(`aaa/bbb/ccc`) + const path2 = EP.fromString(`aaa/bbb/ddd`) + + return () => { + EP.isChildOf(path2, path1) + } + }), + Benny.cycle(), + Benny.complete(), + Benny.save({ file: 'isChildOf not child', details: true }), + ) await Benny.suite( 'buildTree - deeply nested elements', Benny.add('deeply nested elements', () => { diff --git a/editor/src/core/shared/element-path-tree.spec.browser2.tsx b/editor/src/core/shared/element-path-tree.spec.browser2.tsx index 16348c6b1b1d..321414400399 100644 --- a/editor/src/core/shared/element-path-tree.spec.browser2.tsx +++ b/editor/src/core/shared/element-path-tree.spec.browser2.tsx @@ -126,16 +126,14 @@ describe('Building and ordering the element path tree for a real project', () => expect(printTree(renderResult.getEditorState().editor.elementPathTree)).toEqual( `sb - sb/1e7 sb/sc sb/sc/app sb/sc/app:app-root sb/sc/app:app-root/card sb/sc/app:app-root/card:card-root - sb/sc/app:app-root/card:card-root/30d sb/sc/app:app-root/card:card-root/card-span + sb/sc/app:app-root/card:card-root/30d sb/sc/app:app-root/card/card-child - sb/sc/app:app-root/52e sb/sc/app:app-root/frag sb/sc/app:app-root/frag/frag-child sb/sc/app:app-root/frag/cond-1 @@ -144,7 +142,9 @@ describe('Building and ordering the element path tree for a real project', () => sb/sc/app:app-root/frag/cond-1/cond-1-true/cond-2 sb/sc/app:app-root/frag/cond-1/cond-1-true/cond-2/e26 sb/sc/app:app-root/frag/cond-1/cond-1-true/cond-2/e26/cond-2-child~~~1 + sb/sc/app:app-root/52e sb/sc/app/app-child + sb/1e7 `, ) }) diff --git a/editor/src/core/shared/element-path-tree.ts b/editor/src/core/shared/element-path-tree.ts index 1c223657b4f9..55d36be8332f 100644 --- a/editor/src/core/shared/element-path-tree.ts +++ b/editor/src/core/shared/element-path-tree.ts @@ -1,140 +1,155 @@ import { isFeatureEnabled } from '../../utils/feature-switches' import { MetadataUtils } from '../model/element-metadata-utils' -import { isLeft, isRight } from './either' +import { forEachRight, isLeft } from './either' import * as EP from './element-path' -import type { ElementInstanceMetadataMap } from './element-template' +import type { ElementInstanceMetadataMap, JSXElementChild } from './element-template' import { - isJSElementAccess, - isJSExpressionMapOrOtherJavaScript, - isJSIdentifier, - isJSPropertyAccess, isJSXConditionalExpression, isJSXElement, + isJSXElementLike, isJSXFragment, - isJSXTextBlock, } from './element-template' import type { ElementPath } from './project-file-types' -import { fastForEach } from './utils' +import { assertNever } from './utils' export interface ElementPathTree { path: ElementPath pathString: string - children: Array + innerChildren: Array + propsChildren: Array } export function elementPathTree( path: ElementPath, pathString: string, - children: Array, + innerChildren: Array, + propsChildren: Array, ): ElementPathTree { return { path: path, pathString: pathString, - children: children, + innerChildren: innerChildren, + propsChildren: propsChildren, } } +export function getElementPathTreeChildren(tree: ElementPathTree): Array { + return [...tree.innerChildren, ...tree.propsChildren] +} + +export function forEachElementPathTreeChild( + tree: ElementPathTree, + handler: (elementPathTree: ElementPathTree) => void, +): void { + tree.innerChildren.forEach(handler) + tree.propsChildren.forEach(handler) +} + export type ElementPathTrees = { [key: string]: ElementPathTree } export function buildTree(...metadatas: Array): ElementPathTrees { let tree: ElementPathTrees = {} for (const metadata of metadatas) { + // The tree should maintain the ordering of the statically defined elements, + // But the metadata itself may not match that ordering. const elementPaths = Object.values(metadata).map((m) => m.elementPath) - if (elementPaths.length === 0) { - return {} + const firstElementPath = elementPaths.at(0) + if (firstElementPath == null) { + continue } - const root = EP.getStoryboardPathFromPath(elementPaths[0]) + const root = EP.getStoryboardPathFromPath(firstElementPath) if (root == null) { - return {} + continue } const missingParents = getMissingParentPaths(elementPaths, metadata) const paths = getReorderedPaths(elementPaths, metadata, missingParents) - buildTreeRecursive_MUTATE(root, tree, paths, metadata) + buildTree_MUTATE(root, tree, paths, metadata) } return tree } -function buildTreeRecursive_MUTATE( +function addRootToTree(rootPath: ElementPath, trees: ElementPathTrees): void { + const rootPathString = EP.toString(rootPath) + if (!(rootPathString in trees)) { + trees[rootPathString] = elementPathTree(rootPath, rootPathString, [], []) + } +} + +function buildTree_MUTATE( rootPath: ElementPath, trees: ElementPathTrees, originalPaths: ElementPath[], metadata: ElementInstanceMetadataMap, -): ElementPathTree[] { - const rootPathString = EP.toString(rootPath) - trees[rootPathString] = elementPathTree(rootPath, rootPathString, []) - - const childrenPaths = getChildrenPaths(metadata, rootPath, originalPaths) +): void { + addRootToTree(rootPath, trees) - let children: ElementPathTree[] = [] - for (const path of childrenPaths) { + function addPathToTree(path: ElementPath): void { const pathString = EP.toString(path) - const subTree = elementPathTree( - path, - pathString, - buildTreeRecursive_MUTATE(path, trees, originalPaths, metadata), - ) - trees[rootPathString].children.push(subTree) - children.push(subTree) - } + const alreadyAdded = pathString in trees + if (!alreadyAdded) { + const parentPath = EP.parentPath(path) + const subTree = elementPathTree(path, pathString, [], []) + trees[pathString] = subTree + const pathLastPart = EP.lastElementPathForPath(path) - return children -} + if (pathLastPart?.length === 1) { + // If the last part of the path is a single element, it should be added to the inner children. + // This would be a path like `a/b/c:d`. + trees[EP.toString(parentPath)].innerChildren.push(subTree) + } else { + // Otherwise this should be added to the props children. + trees[EP.toString(parentPath)].propsChildren.push(subTree) + } + } + } -function getChildrenPaths( - metadata: ElementInstanceMetadataMap, - rootPath: ElementPath, - paths: ElementPath[], -): ElementPath[] { - const element = metadata[EP.toString(rootPath)] + let workingPaths = [...originalPaths] + workingPaths.sort((a, b) => { + return EP.fullDepth(a) - EP.fullDepth(b) + }) + for (const workingPath of workingPaths) { + addPathToTree(workingPath) + } - // Grab the child elements from the element metadata, if available. - let childrenFromElement: ElementPath[] = [] - if ( - element != null && - isRight(element.element) && - (isJSXElement(element.element.value) || isJSXFragment(element.element.value)) && - element.element.value.children.length > 0 - ) { - childrenFromElement = element.element.value.children - .filter((child) => { + for (const elementMetadata of Object.values(metadata)) { + forEachRight(elementMetadata.element, (element) => { + if (isJSXElementLike(element)) { + let elementChildren: Array if (isFeatureEnabled('Condensed Navigator Entries')) { - // if Data Entries are enabled, we should always show them in the Navigator - return true + elementChildren = element.children } else { - return ( - !isJSXTextBlock(child) && - !isJSExpressionMapOrOtherJavaScript(child) && - !isJSIdentifier(child) && - !isJSPropertyAccess(child) && - !isJSElementAccess(child) - ) + elementChildren = element.children.filter((child) => { + switch (child.type) { + case 'JSX_TEXT_BLOCK': + case 'ATTRIBUTE_OTHER_JAVASCRIPT': + case 'JSX_MAP_EXPRESSION': + case 'JS_IDENTIFIER': + case 'JS_PROPERTY_ACCESS': + case 'JS_ELEMENT_ACCESS': + return false + case 'JSX_ELEMENT': + case 'JSX_CONDITIONAL_EXPRESSION': + case 'JSX_FRAGMENT': + case 'ATTRIBUTE_FUNCTION_CALL': + case 'ATTRIBUTE_NESTED_ARRAY': + case 'ATTRIBUTE_NESTED_OBJECT': + case 'ATTRIBUTE_VALUE': + return true + default: + assertNever(child) + } + }) } - }) - .map((child) => EP.appendToPath(rootPath, child.uid)) + for (const child of elementChildren) { + const childPath = EP.appendToPath(elementMetadata.elementPath, child.uid) + addPathToTree(childPath) + } + } + }) } - - // Then, grab any other child from the paths array, which is not included in the - // elements from the metadata. - const otherChildrenFromPaths = paths.filter( - (path) => - EP.isChildOf(path, rootPath) && - !childrenFromElement.some((other) => EP.pathsEqual(other, path)), - ) - - // If there are children outside the element metadata, need to merge the two and sort them. - // Otherwise, return the children from the meta. - return otherChildrenFromPaths.length > 0 - ? childrenFromElement - .concat(otherChildrenFromPaths) - .sort( - (a, b) => - paths.findIndex((p) => EP.pathsEqual(p, a)) - - paths.findIndex((p) => EP.pathsEqual(p, b)), - ) - : childrenFromElement } function getMissingParentPaths( @@ -159,32 +174,19 @@ function getReorderedPaths( metadata: ElementInstanceMetadataMap, missingParents: ElementPath[], ): ElementPath[] { - const elementsToReorder = original.filter((path) => { - const element = MetadataUtils.findElementByElementPath(metadata, path) - if (element == null) { - return false - } - return ( - MetadataUtils.isConditionalFromMetadata(element) || - MetadataUtils.isExpressionOtherJavascriptFromMetadata(element) || - MetadataUtils.isJSXMapExpressionFromMetadata(element) || - MetadataUtils.isIdentifierFromMetadata(element) || - MetadataUtils.isPropertyAccessFromMetadata(element) || - MetadataUtils.isElementAccessFromMetadata(element) - ) + const pathsToBeReordered = original.filter((path) => { + // Omit elements that have a missing ancestor. + return !missingParents.some((parentPath) => EP.isDescendantOf(path, parentPath)) }) - const pathsToBeReordered = original.filter( - (path) => - !elementsToReorder.some((other) => EP.pathsEqual(other, path)) && // omit conditionals, that will be reordered - !missingParents.some((parentPath) => EP.isDescendantOf(path, parentPath)), // omit elements that have a missing parent - ) - return elementsToReorder.reduce((paths, path) => { + + return original.reduce((paths, path) => { const index = getReorderedIndexInPaths(paths, metadata, path) if (index === 'do-not-reorder') { return paths } - const before = paths.slice(0, index) - const after = paths.slice(index) + const pathsWithout = paths.filter((pathEntry) => !EP.pathsEqual(pathEntry, path)) + const before = pathsWithout.slice(0, index) + const after = pathsWithout.slice(index) return [...before, path, ...after] }, pathsToBeReordered) } @@ -192,34 +194,73 @@ function getReorderedPaths( function getReorderedIndexInPaths( paths: ElementPath[], metadata: ElementInstanceMetadataMap, - conditionalPath: ElementPath, + pathToReorder: ElementPath, ): number | 'do-not-reorder' { - const index = paths.findIndex((path) => EP.isDescendantOf(path, conditionalPath)) - if (index >= 0) { - return index - } - - const parent = MetadataUtils.getParent(metadata, conditionalPath) + const parent = MetadataUtils.getParent(metadata, pathToReorder) if (parent == null || isLeft(parent.element)) { return 'do-not-reorder' } + const parentElement = parent.element.value - if (isJSXElement(parent.element.value) || isJSXFragment(parent.element.value)) { - const innerIndex = parent.element.value.children.findIndex((child) => { + if (isJSXElement(parentElement) || isJSXFragment(parentElement)) { + let innerIndex: number | null = null + let priorSiblings: Array = [] + for (let childIndex = 0; childIndex < parentElement.children.length; childIndex++) { + const child = parentElement.children[childIndex] const childPath = EP.appendToPath(parent.elementPath, child.uid) - return EP.pathsEqual(childPath, conditionalPath) - }) - const parentIndex = paths.findIndex((path) => EP.pathsEqual(parent.elementPath, path)) - if (parentIndex < 0) { + if (EP.pathsEqual(childPath, pathToReorder)) { + // We've found the item that we're trying to reorder, so record the index + // and stop searching. + innerIndex = childIndex + break + } else { + // As we haven't reached the element of interest, record + // this prior sibling for later use. + priorSiblings.push(child) + } + } + if (innerIndex == null) { return 'do-not-reorder' } - return parentIndex + innerIndex - } else if (isJSXConditionalExpression(parent.element.value)) { + + const parentIndex = paths.findIndex((path) => EP.pathsEqual(parent.elementPath, path)) + + if (innerIndex === 0) { + if (parentIndex < 0) { + return 'do-not-reorder' + } else { + // As this is the first item, shift the index past the parent. + return parentIndex + 1 + } + } else { + const priorSiblingPaths = priorSiblings.map((sibling) => { + return EP.appendToPath(parent.elementPath, sibling.uid) + }) + // As there are prior siblings, we need to count those and their descendants, + // so as to shift this element past them. + const priorSiblingDescendants = paths.reduce((workingCount, path) => { + if ( + priorSiblingPaths.some((priorSiblingPath) => { + return EP.isDescendantOfOrEqualTo(path, priorSiblingPath) + }) + ) { + return workingCount + 1 + } else { + return workingCount + } + }, 0) + + // Shift by the parent position, add 1 to put it past the parent and then shift it by the + // count of the prior siblings and their descendants. + return parentIndex + 1 + priorSiblingDescendants + } + } else if (isJSXConditionalExpression(parentElement)) { const parentIndex = paths.findIndex((path) => EP.pathsEqual(parent.elementPath, path)) if (parentIndex < 0) { return 'do-not-reorder' + } else { + return parentIndex } - return parentIndex } else { return 'do-not-reorder' } @@ -236,7 +277,7 @@ export function getCanvasRoots(trees: ElementPathTrees): Array return [] } - return storyboardTree.children + return getElementPathTreeChildren(storyboardTree) } export function printTree(treeRoot: ElementPathTrees): string { @@ -247,7 +288,7 @@ export function printTree(treeRoot: ElementPathTrees): string { } outputText += EP.toString(element.path) outputText += '\n' - for (const child of element.children) { + for (const child of getElementPathTreeChildren(element)) { printElement(child, depth + 1) } } @@ -265,7 +306,7 @@ export function forEachChildOfTarget( ): void { const foundTree = getSubTree(treeRoot, target) if (foundTree != null) { - fastForEach(Object.values(foundTree.children), (subTree) => { + forEachElementPathTreeChild(foundTree, (subTree) => { handler(subTree.path) }) } diff --git a/editor/src/core/shared/element-path.ts b/editor/src/core/shared/element-path.ts index 8bfb72740ed1..a63ee8c85a88 100644 --- a/editor/src/core/shared/element-path.ts +++ b/editor/src/core/shared/element-path.ts @@ -642,14 +642,48 @@ export function findAmongAncestorsOfPath( } export function isDescendantOf(target: ElementPath, maybeAncestor: ElementPath): boolean { - const targetString = toString(target) - const maybeAncestorString = toString(maybeAncestor) - return ( - targetString !== maybeAncestorString && - targetString.startsWith(maybeAncestorString) && - (targetString[maybeAncestorString.length] === SceneSeparator || - targetString[maybeAncestorString.length] === ElementSeparator) - ) + // If the target has less parts than the potential ancestor, it's by default + // higher up the hierarchy. + if (target.parts.length < maybeAncestor.parts.length) { + return false + } + // Check if any of the shared (by index) parts are different, if any are that + // means these two are on different branches of the hierarchy. + for (let index = 0; index < maybeAncestor.parts.length - 1; index++) { + const maybeAncestorPart = maybeAncestor.parts[index] + const targetPart = target.parts[index] + if (!elementPathPartsEqual(maybeAncestorPart, targetPart)) { + return false + } + } + // As the rest of the parts are the same, we can check the last part to see + // if the target has less parts as that puts it higher up the hierarchy than the + // potential ancestor. + const lastTargetPart = target.parts[maybeAncestor.parts.length - 1] + const lastAncestorPart = maybeAncestor.parts[maybeAncestor.parts.length - 1] + if (lastTargetPart.length < lastAncestorPart.length) { + return false + } + // Check the elements of the last part to see if there are any differences + // as that means they're on a different branch. Limiting it to the length of + // the potential ancestor. + for (let partIndex = 0; partIndex < lastAncestorPart.length; partIndex++) { + const part = lastAncestorPart[partIndex] + if (lastTargetPart[partIndex] !== part) { + return false + } + } + // If the lengths of the parts and the lengths of the last parts are the same, + // since so far we've determined those to all match, then these are identical paths. + if ( + target.parts.length === maybeAncestor.parts.length && + lastTargetPart.length === lastAncestorPart.length + ) { + return false + } + + // Fallback meaning this is a descendant. + return true } export function getAncestorsForLastPart(path: ElementPath): ElementPath[] {