Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Changing Imports In Canvas #6567

Merged
merged 4 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ const gridDrawToInsertStrategyInner =
),
updateHighlightedViews('mid-interaction', [targetParent]),
],
[targetParent],
[],
seanparsons marked this conversation as resolved.
Show resolved Hide resolved
)
}

Expand Down
79 changes: 77 additions & 2 deletions editor/src/components/canvas/canvas-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ import type {
HighlightBoundsForUids,
ExportsDetail,
} from '../../core/shared/project-file-types'
import { isExportDefault, isParseSuccess, isTextFile } from '../../core/shared/project-file-types'
import {
importsEquals,
isExportDefault,
isParseSuccess,
isTextFile,
} from '../../core/shared/project-file-types'
import {
applyUtopiaJSXComponentsChanges,
getDefaultExportedTopLevelElement,
Expand Down Expand Up @@ -140,7 +145,11 @@ import { getStoryboardUID } from '../../core/model/scene-utils'
import { optionalMap } from '../../core/shared/optional-utils'
import { assertNever, fastForEach } from '../../core/shared/utils'
import type { ProjectContentTreeRoot } from '../assets'
import { getProjectFileByFilePath } from '../assets'
import {
getProjectFileByFilePath,
isProjectContentDirectory,
isProjectContentFile,
} from '../assets'
import type { CSSNumber } from '../inspector/common/css-utils'
import { parseCSSLengthPercent, printCSSNumber } from '../inspector/common/css-utils'
import { getTopLevelName, importedFromWhere } from '../editor/import-utils'
Expand Down Expand Up @@ -2180,3 +2189,69 @@ export function canvasPanelOffsets(): {
right: inspector?.clientWidth ?? 0,
}
}

export function projectContentsSameForRefreshRequire(
oldProjectContents: ProjectContentTreeRoot,
newProjectContents: ProjectContentTreeRoot,
): boolean {
if (oldProjectContents === newProjectContents) {
// Identical references, so the imports are the same.
return true
} else {
for (const [filename, oldProjectTree] of Object.entries(oldProjectContents)) {
const newProjectTree = newProjectContents[filename]
// If the file can't be found in the other tree, the imports are not the same.
if (newProjectTree == null) {
seanparsons marked this conversation as resolved.
Show resolved Hide resolved
return false
}
if (isProjectContentFile(oldProjectTree) && isProjectContentFile(newProjectTree)) {
// Both entries are files.
const oldContent = oldProjectTree.content
const newContent = newProjectTree.content
if (isTextFile(oldContent) || isTextFile(newContent)) {
if (isTextFile(oldContent) && isTextFile(newContent)) {
const oldParsed = oldContent.fileContents.parsed
const newParsed = newContent.fileContents.parsed
if (isParseSuccess(oldParsed) || isParseSuccess(newParsed)) {
if (isParseSuccess(oldParsed) && isParseSuccess(newParsed)) {
if (
!importsEquals(oldParsed.imports, newParsed.imports) ||
oldParsed.combinedTopLevelArbitraryBlock !==
newParsed.combinedTopLevelArbitraryBlock ||
oldParsed.exportsDetail !== newParsed.exportsDetail
) {
// For the same file the imports, combined top
// level arbitrary block or exports have changed.
return false
}
} else {
// One of the files is a parse success but the other is not.
return false
}
}
} else {
// One of the files is a text file but the other is not.
return false
}
}
} else if (
isProjectContentDirectory(oldProjectTree) &&
isProjectContentDirectory(newProjectTree)
) {
// Both entries are directories.
if (
!projectContentsSameForRefreshRequire(oldProjectTree.children, newProjectTree.children)
) {
// The imports of the subdirectories differ.
return false
}
Rheeseyb marked this conversation as resolved.
Show resolved Hide resolved
} else {
// One of the entries is a file and the other is a directory.
return false
}
}
}

// If nothing differs, return true.
return true
}
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,18 @@ describe('Re-mounting is avoided when', () => {

await switchToLiveMode(renderResult)

function checkClicky(expectedContentText: string): void {
const clicky = renderResult.renderedDOM.getByTestId('clicky')
expect(clicky.innerText).toEqual(expectedContentText)
}

// Ensure we can find the original text
expect(renderResult.renderedDOM.queryByText('Clicked 0 times')).not.toBeNull()
checkClicky('Clicked 0 times')

await clickButton(renderResult)

// Ensure it has been updated
expect(renderResult.renderedDOM.queryByText('Clicked 1 times')).not.toBeNull()
checkClicky('Clicked 1 times')

// Update the top level arbitrary JS block
await updateCode(
Expand All @@ -231,7 +236,7 @@ describe('Re-mounting is avoided when', () => {
)

// Check that it has updated without resetting the state
expect(renderResult.renderedDOM.queryByText('Clicked: 1 times')).not.toBeNull()
checkClicky('Clicked: 1 times')

// Update the component itself
await updateCode(
Expand All @@ -241,7 +246,7 @@ describe('Re-mounting is avoided when', () => {
)

// Check again that it has updated without resetting the state
expect(renderResult.renderedDOM.queryByText('Clicked: 1 times!')).not.toBeNull()
checkClicky('Clicked: 1 times!')
})

it('arbitrary JS or a component is edited in a remix project', async () => {
Expand Down
25 changes: 11 additions & 14 deletions editor/src/components/canvas/ui-jsx-canvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ import {
} from './ui-jsx-canvas-renderer/ui-jsx-canvas-execution-scope'
import { applyUIDMonkeyPatch } from '../../utils/canvas-react-utils'
import type { RemixValidPathsGenerationContext } from './canvas-utils'
import { getParseSuccessForFilePath, getValidElementPaths } from './canvas-utils'
import {
projectContentsSameForRefreshRequire,
getParseSuccessForFilePath,
getValidElementPaths,
} from './canvas-utils'
import { arrayEqualsByValue, fastForEach, NO_OP } from '../../core/shared/utils'
import {
AlwaysFalse,
Expand Down Expand Up @@ -360,20 +364,13 @@ export const UiJsxCanvas = React.memo<UiJsxCanvasPropsWithErrorCallback>((props)

useClearSpyMetadataOnRemount(props.invalidatedCanvasData, isRemounted, metadataContext)

const elementsToRerenderRef = React.useRef(ElementsToRerenderGLOBAL.current)
const shouldRerenderRef = React.useRef(false)
shouldRerenderRef.current =
ElementsToRerenderGLOBAL.current === 'rerender-all-elements' ||
elementsToRerenderRef.current === 'rerender-all-elements' || // TODO this means the first drag frame will still be slow, figure out a nicer way to immediately switch to true. probably this should live in a dedicated a function
!arrayEqualsByValue(
ElementsToRerenderGLOBAL.current,
elementsToRerenderRef.current,
EP.pathsEqual,
) // once we get here, we know that both `ElementsToRerenderGLOBAL.current` and `elementsToRerenderRef.current` are arrays
elementsToRerenderRef.current = ElementsToRerenderGLOBAL.current

const maybeOldProjectContents = React.useRef(projectContents)
if (shouldRerenderRef.current) {

const projectContentsSimilarEnough = projectContentsSameForRefreshRequire(
maybeOldProjectContents.current,
projectContents,
)
if (!projectContentsSimilarEnough) {
maybeOldProjectContents.current = projectContents
}

Expand Down
12 changes: 11 additions & 1 deletion editor/src/components/editor/import-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import type {
NodeModules,
} from '../../core/shared/project-file-types'
import { importAlias, importDetails, importsResolution } from '../../core/shared/project-file-types'
import type { ProjectContentTreeRoot } from '../assets'
import { walkContentsTreeForParseSuccess, type ProjectContentTreeRoot } from '../assets'
import type { BuiltInDependencies } from '../../core/es-modules/package-manager/built-in-dependencies-list'
import { withUnderlyingTarget } from './store/editor-state'
import * as EP from '../../core/shared/element-path'
Expand Down Expand Up @@ -366,3 +366,13 @@ function removeImportDetails(
importedFromWithin: importedFromWithin,
}
}

export function getProjectImports(projectContents: ProjectContentTreeRoot): {
Rheeseyb marked this conversation as resolved.
Show resolved Hide resolved
[filename: string]: Imports
} {
let result: { [filename: string]: Imports } = {}
walkContentsTreeForParseSuccess(projectContents, (filePath, parseSuccess) => {
result[filePath] = parseSuccess.imports
})
return result
}
Loading