Skip to content

Commit

Permalink
fix(imports): pass import aliases to mergeImports (#5995)
Browse files Browse the repository at this point in the history
**Problem:**
Currently our duplicate import mechanism doesn't consider import
aliases, resulting in user actions generating duplicate imports in our
sample project.

**Fix:**
This is the prep PR for passing the aliases to `mergeImports` - since it
is running in a worker it cannot access it directly but needs to receive
it from the calling context.

**This PR only adds `filePathMappings: FilePathMappings` as an argument
to `mergeImports` and `renameDuplicateImports`**

**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
liady authored Jul 9, 2024
1 parent ec8f2c0 commit 879bf53
Show file tree
Hide file tree
Showing 41 changed files with 217 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ 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'
import { getFilePathMappings } from '../../../../core/model/project-file-utils'

interface GetReparentOutcomeResult {
commands: Array<CanvasCommand>
Expand Down Expand Up @@ -110,6 +111,7 @@ function adjustElementDuplicateName(
fileContents.imports,
element.imports,
targetFile,
getFilePathMappings(projectContents),
)
// handle element name
if (isJSXElement(element.element)) {
Expand Down
13 changes: 10 additions & 3 deletions editor/src/components/canvas/commands/add-element-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import {
} from '../../editor/store/insertion-path'
import type { EditorState, EditorStatePatch } from '../../../components/editor/store/editor-state'
import { forUnderlyingTargetFromEditorState } from '../../../components/editor/store/editor-state'
import { getUtopiaJSXComponentsFromSuccess } from '../../../core/model/project-file-utils'
import {
getFilePathMappings,
getUtopiaJSXComponentsFromSuccess,
} from '../../../core/model/project-file-utils'
import type { JSXElementChild } from '../../../core/shared/element-template'
import type { Imports } from '../../../core/shared/project-file-types'
import type { BaseCommand, CommandFunction, WhenToRun } from './commands'
Expand Down Expand Up @@ -69,8 +72,12 @@ export const runAddElement: CommandFunction<AddElement> = (
const editorStatePatchNewParentFile = getPatchForComponentChange(
parentSuccess.topLevelElements,
withElementInserted,
mergeImports(underlyingFilePathNewParent, parentSuccess.imports, command.importsToAdd ?? {})
.imports,
mergeImports(
underlyingFilePathNewParent,
getFilePathMappings(editorState.projectContents),
parentSuccess.imports,
command.importsToAdd ?? {},
).imports,
underlyingFilePathNewParent,
)

Expand Down
13 changes: 10 additions & 3 deletions editor/src/components/canvas/commands/add-elements-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { includeToastPatch } from '../../../components/editor/actions/toast-help
import type { EditorState, EditorStatePatch } from '../../../components/editor/store/editor-state'
import { forUnderlyingTargetFromEditorState } from '../../../components/editor/store/editor-state'
import { insertJSXElementChildren } from '../../../core/model/element-template-utils'
import { getUtopiaJSXComponentsFromSuccess } from '../../../core/model/project-file-utils'
import {
getFilePathMappings,
getUtopiaJSXComponentsFromSuccess,
} from '../../../core/model/project-file-utils'
import type { JSXElementChild } from '../../../core/shared/element-template'
import type { Imports } from '../../../core/shared/project-file-types'
import { mergeImports } from '../../../core/workers/common/project-file-utils'
Expand Down Expand Up @@ -76,8 +79,12 @@ export const runAddElements = (
const editorStatePatchNewParentFile = getPatchForComponentChange(
parentSuccess.topLevelElements,
withElementsInserted,
mergeImports(underlyingFilePathNewParent, parentSuccess.imports, command.importsToAdd ?? {})
.imports,
mergeImports(
underlyingFilePathNewParent,
getFilePathMappings(editorState.projectContents),
parentSuccess.imports,
command.importsToAdd ?? {},
).imports,
underlyingFilePathNewParent,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { EditorState } from '../../../components/editor/store/editor-state'
import type { Imports } from '../../../core/shared/project-file-types'
import type { BaseCommand, WhenToRun, CommandFunction, CommandFunctionResult } from './commands'
import { patchParseSuccessAtFilePath } from './patch-utils'
import { getFilePathMappings } from '../../../core/model/project-file-utils'

export interface AddImportsToFile extends BaseCommand {
type: 'ADD_IMPORTS_TO_FILE'
Expand Down Expand Up @@ -34,6 +35,7 @@ export const runAddImportsToFile: CommandFunction<AddImportsToFile> = (
// TODO handle duplicate name mapping
const { imports: updatedImports } = mergeImports(
command.targetFile,
getFilePathMappings(editorState.projectContents),
parseSuccess.imports,
command.imports,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import {
insertJSXElementChildren,
renameJsxElementChild,
} from '../../../core/model/element-template-utils'
import { getUtopiaJSXComponentsFromSuccess } from '../../../core/model/project-file-utils'
import {
getFilePathMappings,
getUtopiaJSXComponentsFromSuccess,
} from '../../../core/model/project-file-utils'
import * as EP from '../../../core/shared/element-path'
import { optionalMap } from '../../../core/shared/optional-utils'
import { type ElementPath } from '../../../core/shared/project-file-types'
Expand Down Expand Up @@ -50,8 +53,12 @@ export const runInsertElementInsertionSubject: CommandFunction<InsertElementInse
editor,
(success, _element, _underlyingTarget, underlyingFilePath) => {
const utopiaComponents = getUtopiaJSXComponentsFromSuccess(success)

const updatedImports = mergeImports(underlyingFilePath, success.imports, subject.importsToAdd)
const updatedImports = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
subject.importsToAdd,
)

subjectElement = renameJsxElementChild(subject.element, updatedImports.duplicateNameMapping)

Expand Down
12 changes: 10 additions & 2 deletions editor/src/components/canvas/commands/wrap-in-container-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ import type { BaseCommand, CommandFunction, WhenToRun } from './commands'
import { getPatchForComponentChange } from './commands'
import * as EP from '../../../core/shared/element-path'
import * as PP from '../../../core/shared/property-path'
import { getUtopiaJSXComponentsFromSuccess } from '../../../core/model/project-file-utils'
import {
getFilePathMappings,
getUtopiaJSXComponentsFromSuccess,
} from '../../../core/model/project-file-utils'
import type { InsertionSubjectWrapper } from '../../editor/editor-modes'
import { assertNever } from '../../../core/shared/utils'
import { mergeImports } from '../../../core/workers/common/project-file-utils'
Expand Down Expand Up @@ -116,7 +119,12 @@ export const runWrapInContainerCommand: CommandFunction<WrapInContainerCommand>
getPatchForComponentChange(
success.topLevelElements,
insertionResult.components,
mergeImports(underlyingFilePath, withElementRemoved.imports, imports).imports,
mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
withElementRemoved.imports,
imports,
).imports,
underlyingFilePath,
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ describe('jsxElementChildToText', () => {
it('complicated case', () => {
const parsedResult: ParsedTextFile = lintAndParse(
'test.js',
[],
`const TestComponent = (props) => <div>{something()?.[another().property?.deeperProperty]}</div>`,
null,
emptySet(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ describe('Updating a transitive dependency', () => {

const updatedIndirectFileParsedTextFile = lintAndParse(
indirectFilePath,
[],
updatedIndirectFileContent,
null,
emptySet(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ async function renderTestProject() {
const updatedProject = Object.keys(exampleFiles).reduce((workingProject, modifiedFilename) => {
const parsedFile = lintAndParse(
modifiedFilename,
[],
exampleFiles[modifiedFilename],
null,
emptySet(),
Expand Down
1 change: 1 addition & 0 deletions editor/src/components/custom-code/code-file.test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const Spring = (props) => {
export function createCodeFile(path: string, contents: string): TextFile {
const result = lintAndParse(
path,
[],
contents,
null,
emptySet(),
Expand Down
1 change: 1 addition & 0 deletions editor/src/components/editor/actions/actions.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ const originalModel = deepFreeze(
parseSuccess(
addImport(
'/code.js',
[],
'utopia-api',
null,
[importAlias('View'), importAlias('Scene'), importAlias('Storyboard')],
Expand Down
10 changes: 10 additions & 0 deletions editor/src/components/editor/actions/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
applyUtopiaJSXComponentsChanges,
fileExists,
fileTypeFromFileName,
getFilePathMappings,
getUtopiaJSXComponentsFromSuccess,
saveFile,
saveTextFileContents,
Expand Down Expand Up @@ -1851,6 +1852,7 @@ export const UPDATE_FNS = {
(success, _, underlyingFilePath) => {
const updatedImports = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
action.importsToAdd,
).imports
Expand Down Expand Up @@ -2320,6 +2322,7 @@ export const UPDATE_FNS = {

const updatedImports = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
action.importsToAdd,
)
Expand Down Expand Up @@ -2376,6 +2379,7 @@ export const UPDATE_FNS = {

const { imports, duplicateNameMapping } = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
action.importsToAdd,
)
Expand Down Expand Up @@ -2465,6 +2469,7 @@ export const UPDATE_FNS = {
const startingComponents = getUtopiaJSXComponentsFromSuccess(success)
const updatedImports = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
action.importsToAdd,
)
Expand Down Expand Up @@ -4127,6 +4132,7 @@ export const UPDATE_FNS = {
const existingUIDs = new Set(getAllUniqueUids(editor.projectContents).uniqueIDs)
const parsedResult = getParseFileResult(
newFileName,
getFilePathMappings(editor.projectContents),
templateFile.fileContents.code,
null,
1,
Expand Down Expand Up @@ -4589,6 +4595,7 @@ export const UPDATE_FNS = {
(success, _, underlyingFilePath) => {
const { imports, duplicateNameMapping } = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
action.importsToAdd,
)
Expand Down Expand Up @@ -4972,11 +4979,13 @@ export const UPDATE_FNS = {
changedTextFilenames,
)

const filePathMappings = getFilePathMappings(withFileChanges.unpatchedEditor.projectContents)
// For those files that changed, do a print-parse against each file.
const workerUpdates = filesToUpdateResult.filesToUpdate.flatMap((fileToUpdate) => {
if (fileToUpdate.type === 'printandreparsefile') {
const printParsedContent = getPrintAndReparseCodeResult(
fileToUpdate.filename,
filePathMappings,
fileToUpdate.parseSuccess,
fileToUpdate.stripUIDs,
fileToUpdate.versionNumber,
Expand Down Expand Up @@ -5384,6 +5393,7 @@ export const UPDATE_FNS = {

const updatedImports = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
action.toInsert.importsToAdd,
)
Expand Down
3 changes: 3 additions & 0 deletions editor/src/components/editor/actions/wrap-unwrap-helpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from '../../../core/model/element-template-utils'
import {
applyUtopiaJSXComponentsChanges,
getFilePathMappings,
getUtopiaJSXComponentsFromSuccess,
} from '../../../core/model/project-file-utils'
import * as EP from '../../../core/shared/element-path'
Expand Down Expand Up @@ -517,6 +518,7 @@ export function insertElementIntoJSXConditional(

const { imports, duplicateNameMapping } = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
importsToAdd,
)
Expand Down Expand Up @@ -574,6 +576,7 @@ function insertConditionalIntoConditionalClause(
(success, _, underlyingFilePath) => {
const { imports, duplicateNameMapping } = mergeImports(
underlyingFilePath,
getFilePathMappings(editor.projectContents),
success.imports,
importsToAdd,
)
Expand Down
3 changes: 3 additions & 0 deletions editor/src/components/editor/export-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export var Whatever = (props) => {
}`
const parseResult = parseCode(
'/src/index.js',
[],
codeForFile,
null,
emptySet(),
Expand Down Expand Up @@ -65,6 +66,7 @@ export var Whatever = (props) => {
export var Whatever = 'something'`
const parseResult = parseCode(
'/src/index.js',
[],
codeForFile,
null,
emptySet(),
Expand Down Expand Up @@ -115,6 +117,7 @@ export var Whatever = 'something'`
export var Whatever = 'something'`
const parseResult = parseCode(
'/src/index.js',
[],
codeForFile,
null,
emptySet(),
Expand Down
12 changes: 8 additions & 4 deletions editor/src/components/editor/import-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { resolveModulePathIncludingBuiltIns } from '../../core/es-modules/packag
import { foldEither } from '../../core/shared/either'

import {
applyFilePathMappingsToFilePath,
emptyImports,
emptyImportsMergeResolution,
mergeImports,
Expand Down Expand Up @@ -37,10 +38,7 @@ import * as EP from '../../core/shared/element-path'
import { renameDuplicateImportsInMergeResolution } from '../../core/shared/import-shared-utils'
import { getParseSuccessForFilePath } from '../canvas/canvas-utils'
import type { FilePathMappings } from '../../core/model/project-file-utils'
import {
applyFilePathMappingsToFilePath,
getFilePathMappings,
} from '../../core/model/project-file-utils'
import { getFilePathMappings } from '../../core/model/project-file-utils'

export function getRequiredImportsForElement(
target: ElementPath,
Expand Down Expand Up @@ -80,6 +78,7 @@ export function getRequiredImportsForElement(
case 'SAME_FILE_ORIGIN':
importsMergeResolution = mergeImportsResolutions(
targetFilePath,
filePathMappings,
importsMergeResolution,
importsResolution(
getImportsFor(
Expand All @@ -97,6 +96,7 @@ export function getRequiredImportsForElement(
if (importedFromResult.exportedName != null) {
importsMergeResolution = mergeImportsResolutions(
targetFilePath,
filePathMappings,
importsMergeResolution,
importsResolution(
getImportsFor(
Expand All @@ -123,6 +123,7 @@ export function getRequiredImportsForElement(
} else if (isJSXFragment(elem) && elem.longForm) {
importsMergeResolution = mergeImportsResolutions(
targetFilePath,
filePathMappings,
importsMergeResolution,
importsResolution({
react: {
Expand All @@ -141,6 +142,7 @@ export function getRequiredImportsForElement(
targetFileContents.imports,
importsMergeResolution,
targetFilePath,
filePathMappings,
)

return duplicateImportsResolution
Expand Down Expand Up @@ -271,11 +273,13 @@ export function getImportsFor(

export function mergeImportsResolutions(
fileUri: string,
filePathMappings: FilePathMappings,
existing: ImportsMergeResolution,
newImports: ImportsMergeResolution,
): ImportsMergeResolution {
const { imports, duplicateNameMapping } = mergeImports(
fileUri,
filePathMappings,
existing.imports,
newImports.imports,
)
Expand Down
2 changes: 2 additions & 0 deletions editor/src/components/editor/store/dispatch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ import {
} from '../../../core/property-controls/property-controls-local'
import { setReactRouterErrorHasBeenLogged } from '../../../core/shared/runtime-report-logs'
import type { PropertyControlsInfo } from '../../custom-code/code-file'
import { getFilePathMappings } from '../../../core/model/project-file-utils'

type DispatchResultFields = {
nothingChanged: boolean
Expand Down Expand Up @@ -345,6 +346,7 @@ function maybeRequestModelUpdate(
const parseFinished = getParseResult(
workers,
filesToUpdate,
getFilePathMappings(projectContents),
existingUIDs,
isSteganographyEnabled(),
)
Expand Down
Loading

0 comments on commit 879bf53

Please sign in to comment.