fix(canvas) Handle Changing Imports In Canvas (#6567)

- Removed the previous handling of `shouldRerenderRef`.
- Implemented `projectContentsSameForRefreshRequire`.
- Changed out old `haveProjectImportsChanged` call to use the new
  `projectContentsSameForRefreshRequire`.

Author: seanparsons

editor/src/components/canvas/canvas-strategies/strategies/grid-draw-to-insert-strategy.tsx

@@ -171,7 +171,7 @@ const gridDrawToInsertStrategyInner =
               ),
               updateHighlightedViews('mid-interaction', [targetParent]),
             ],
-            [targetParent],
+            [],
           )
         }
 

editor/src/components/canvas/canvas-utils.ts

@@ -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,
@@ -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'
@@ -2180,3 +2189,73 @@ 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]
+      // No need to check these further if they have the same reference.
+      if (oldProjectTree === newProjectTree) {
+        continue
+      }
+      // If the file can't be found in the other tree, the imports are not the same.
+      if (newProjectTree == null) {
+        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
+        }
+      } else {
+        // One of the entries is a file and the other is a directory.
+        return false
+      }
+    }
+  }
+
+  // If nothing differs, return true.
+  return true
+}

editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-code-execution.spec.browser2.tsx

@@ -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(
@@ -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(
@@ -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 () => {

editor/src/components/canvas/ui-jsx-canvas.tsx

@@ -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,
@@ -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
   }