Skip to content

Commit

Permalink
Rename duplicate JSX element names (#4726)
Browse files Browse the repository at this point in the history
**Problem:**
Address more locations where we recognize duplicate imports (`mergeImports` returns a `duplicateNameMapping` that requires renaming the element)

**Fix:**
Make sure to also rename the inserted element in these cases (`ADD_IMPORTS`, `INSERT_INSERTABLE` and handling conditionals insert)
  • Loading branch information
liady authored Jan 21, 2024
1 parent d27b4d1 commit 714ab34
Show file tree
Hide file tree
Showing 9 changed files with 345 additions and 78 deletions.
29 changes: 28 additions & 1 deletion editor/src/components/custom-code/code-file.test-utils.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import type { NodeModules, ProjectContents, TextFile } from '../../core/shared/project-file-types'
import {
esCodeFile,
isParseSuccess,
isTextFile,
RevisionsState,
textFile,
textFileContents,
} from '../../core/shared/project-file-types'
import { lintAndParse } from '../../core/workers/parser-printer/parser-printer'
import {
lintAndParse,
printCode,
printCodeOptions,
} from '../../core/workers/parser-printer/parser-printer'
import type { ProjectContentTreeRoot } from '../assets'
import { contentsToTree, getProjectFileByFilePath, treeToContents } from '../assets'
import type { EditorState } from '../editor/store/editor-state'
import { StoryboardFilePath } from '../editor/store/editor-state'
import { createComplexDefaultProjectContents } from '../../sample-projects/sample-project-utils'
import { replaceAll } from '../../core/shared/string-utils'
Expand Down Expand Up @@ -80,3 +86,24 @@ export function getTextFileByPath(projectContents: ProjectContentTreeRoot, path:
throw new Error(`Unable to find a text file at path ${path}.`)
}
}

export function printParsedCodeForFile(
actualResult: EditorState,
filename: string,
stripUIDs: boolean = true,
): string {
const codeFile = getTextFileByPath(actualResult.projectContents, filename)
const parsed = codeFile.fileContents.parsed
if (isParseSuccess(parsed)) {
return printCode(
filename,
printCodeOptions(false, true, false, stripUIDs),
parsed.imports,
parsed.topLevelElements,
parsed.jsxFactoryFunction,
parsed.exportsDetail,
)
} else {
throw new Error('No parsed version of the file.')
}
}
102 changes: 101 additions & 1 deletion editor/src/components/editor/actions/actions.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
RevisionsState,
exportFunction,
importAlias,
importDetails,
isParseSuccess,
isTextFile,
isUnparsed,
Expand All @@ -86,7 +87,7 @@ import {
import { getFrameChange } from '../../canvas/canvas-utils'
import { generateCodeResultCache } from '../../custom-code/code-file'
import { cssNumber } from '../../inspector/common/css-utils'
import { getComponentGroups } from '../../shared/project-components'
import { getComponentGroups, insertableComponent } from '../../shared/project-components'
import type { EditorState, PersistentModel } from '../store/editor-state'
import {
StoryboardFilePath,
Expand Down Expand Up @@ -660,6 +661,105 @@ describe('INSERT_INSERTABLE', () => {
}
})

it('inserts an element into the project with the given values, and duplicate name, also adding style props', () => {
const project = complexDefaultProjectPreParsed()
const editorState = editorModelFromPersistentModel(project, NO_OP)

const insertableGroups = getComponentGroups(
'insert',
{ antd: { status: 'loaded' } },
{ antd: DefaultThirdPartyControlDefinitions.antd },
editorState.projectContents,
[resolvedNpmDependency('antd', '4.0.0')],
StoryboardFilePath,
)
const antdGroup = forceNotNull(
'Group should exist.',
insertableGroups.find((group) => {
return (
group.source.type === 'PROJECT_DEPENDENCY_GROUP' && group.source.dependencyName === 'antd'
)
}),
)
const springInsertable = insertableComponent(
{
'./test.js': importDetails(null, [importAlias('Spring')], null),
},
() => jsxElement('Spring', 'spring', jsxAttributesFromMap({}), []),
'Spring',
[],
null,
)

const targetPath = EP.elementPath([
['storyboard-entity', 'scene-1-entity', 'app-entity'],
['app-outer-div', 'card-instance'],
['card-outer-div'],
])

const action = insertInsertable(
childInsertionPath(targetPath),
springInsertable,
'add-size',
null,
)

const actualResult = UPDATE_FNS.INSERT_INSERTABLE(action, editorState)
const cardFile = getProjectFileByFilePath(actualResult.projectContents, '/src/card.js')
if (cardFile != null && isTextFile(cardFile)) {
const parsed = cardFile.fileContents.parsed
if (isParseSuccess(parsed)) {
const printedCode = printCode(
'/src/card.js',
printCodeOptions(false, true, true, true),
parsed.imports,
parsed.topLevelElements,
parsed.jsxFactoryFunction,
parsed.exportsDetail,
)
expect(printedCode).toMatchInlineSnapshot(`
"import * as React from 'react'
import { Spring } from 'non-existant-dummy-library'
import { Spring as Spring_2 } from './test.js'
export var Card = (props) => {
return (
<div style={{ ...props.style }}>
<div
data-testid='card-inner-div'
style={{
position: 'absolute',
left: 0,
top: 0,
width: 50,
height: 50,
backgroundColor: 'red',
}}
/>
<Spring
data-testid='spring'
style={{
position: 'absolute',
left: 100,
top: 200,
width: 50,
height: 50,
backgroundColor: 'blue',
}}
/>
<Spring_2 style={{ width: 100, height: 100 }} />
</div>
)
}
"
`)
} else {
throw new Error('File does not contain parse success.')
}
} else {
throw new Error('File is not a text file.')
}
})

it('inserts an img element into the project, also adding style props', () => {
const project = complexDefaultProjectPreParsed()
const editorState = editorModelFromPersistentModel(project, NO_OP)
Expand Down
45 changes: 21 additions & 24 deletions editor/src/components/editor/actions/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
getIndexInParent,
insertJSXElementChildren,
renameJsxElementChild,
renameJsxElementChildWithoutId,
} from '../../../core/model/element-template-utils'
import {
applyToAllUIJSFiles,
Expand Down Expand Up @@ -49,8 +50,6 @@ import type {
JSExpressionValue,
JSXElement,
JSXElementChildren,
SettableLayoutSystem,
UtopiaJSXComponent,
JSXElementChild,
} from '../../../core/shared/element-template'
import {
Expand Down Expand Up @@ -128,7 +127,6 @@ import {
isImageFile,
isDirectory,
parseSuccess,
importsResolution,
} from '../../../core/shared/project-file-types'
import {
codeFile,
Expand Down Expand Up @@ -397,7 +395,7 @@ import {
import { loadStoredState } from '../stored-state'
import { applyMigrations } from './migrations/migrations'

import { defaultConfig, rename } from 'utopia-vscode-common'
import { defaultConfig } from 'utopia-vscode-common'
import { reorderElement } from '../../../components/canvas/commands/reorder-element-command'
import type { BuiltInDependencies } from '../../../core/es-modules/package-manager/built-in-dependencies-list'
import { fetchNodeModules } from '../../../core/es-modules/package-manager/fetch-packages'
Expand Down Expand Up @@ -485,7 +483,6 @@ import {
import { styleStringInArray } from '../../../utils/common-constants'
import { collapseTextElements } from '../../../components/text-editor/text-handling'
import { LayoutPropertyList, StyleProperties } from '../../inspector/common/css-utils'
import { isFeatureEnabled } from '../../../utils/feature-switches'
import { isUtopiaCommentFlag, makeUtopiaFlagComment } from '../../../core/shared/comment-flags'
import { toArrayOf } from '../../../core/shared/optics/optic-utilities'
import { fromField, traverseArray } from '../../../core/shared/optics/optic-creators'
Expand All @@ -501,10 +498,7 @@ import {
} from '../store/insertion-path'
import { getConditionalCaseCorrespondingToBranchPath } from '../../../core/model/conditionals'
import { deleteProperties } from '../../canvas/commands/delete-properties-command'
import {
replaceFragmentLikePathsWithTheirChildrenRecursive,
treatElementAsFragmentLike,
} from '../../canvas/canvas-strategies/strategies/fragment-like-helpers'
import { treatElementAsFragmentLike } from '../../canvas/canvas-strategies/strategies/fragment-like-helpers'
import {
fixParentContainingBlockSettings,
isTextContainingConditional,
Expand Down Expand Up @@ -552,8 +546,6 @@ import { convertToAbsoluteAndReparentToUnwrapStrategy } from '../one-shot-unwrap
import { addHookForProjectChanges } from '../store/collaborative-editing'
import { arrayDeepEquality, objectDeepEquality } from '../../../utils/deep-equality'
import type { ProjectServerState } from '../store/project-server-state'
import { fixParseSuccessUIDs } from '../../../core/workers/parser-printer/uid-fix'
import { mergeImportsResolutions } from '../import-utils'

export const MIN_CODE_PANE_REOPEN_WIDTH = 100

Expand Down Expand Up @@ -4168,13 +4160,18 @@ export const UPDATE_FNS = {
)
},
ADD_IMPORTS: (action: AddImports, editor: EditorModel): EditorModel => {
let duplicateNames = new Map<string, string>()
return modifyUnderlyingTargetElement(
action.target,
editor,
(element) => element,
(element) => renameJsxElementChild(element, duplicateNames),
(success, _, underlyingFilePath) => {
// TODO handle duplicate name mapping
const { imports } = mergeImports(underlyingFilePath, success.imports, action.importsToAdd)
const { imports, duplicateNameMapping } = mergeImports(
underlyingFilePath,
success.imports,
action.importsToAdd,
)
duplicateNames = duplicateNameMapping
return {
...success,
imports: imports,
Expand Down Expand Up @@ -4875,7 +4872,7 @@ export const UPDATE_FNS = {

const newPath = EP.appendToPath(action.insertionPath.intendedParentPath, newUID)

const element = action.toInsert.element()
let element = action.toInsert.element()

const withNewElement = modifyUnderlyingTargetElement(
insertionPath.intendedParentPath,
Expand All @@ -4884,6 +4881,15 @@ export const UPDATE_FNS = {
(success, _, underlyingFilePath) => {
const utopiaComponents = getUtopiaJSXComponentsFromSuccess(success)

const updatedImports = mergeImports(
underlyingFilePath,
success.imports,
action.toInsert.importsToAdd,
)

const { imports, duplicateNameMapping } = updatedImports
element = renameJsxElementChildWithoutId(element, duplicateNameMapping)

if (element.type === 'JSX_ELEMENT') {
const propsWithUid = forceRight(
setJSXValueAtPath(
Expand Down Expand Up @@ -4973,15 +4979,6 @@ export const UPDATE_FNS = {
withInsertedElement.components,
)

const updatedImports = mergeImports(
underlyingFilePath,
success.imports,
action.toInsert.importsToAdd,
)

// TODO handle duplicate name mapping
const { imports } = updatedImports

return {
...success,
topLevelElements: updatedTopLevelElements,
Expand Down
Loading

0 comments on commit 714ab34

Please sign in to comment.