Don't render when code ahead (#6069)

**Problem:** When making code changes, we'll cause the canvas to re-render twice: once when the code is changed; and then a second time once the parsed model has caught up. **Fix:** Specifically check if any code is ahead before triggering a canvas render. To do this I used the existing `shouldRerender` in `editor.tsx`, but realised that was actually a bit of a misnomer, and was gating logic that it shouldn't be. So I have explicitly used the relevant boolean checks around the individual parts of the code called after a dispatch. Note that I have completely removed the check around `editorDispatchClosingOut`. I'm very much certain that we should never have been gating that with a check that something had changed, as the internal logic of that function is already checking that something actually changed before e.g. triggering a save. My only concern with this was that if you are typing fast enough it would prevent the canvas from rendering until you paused briefly, because we don't change the revisions state following a parser-printer update if the update is stale here: <https://github.com/concrete-utopia/utopia/blob/67abcdd1d9fa11c0bf42be255cf655bba45a5a38/editor/src/core/shared/parser-projectcontents-utils.ts#L100-L109> However, in practice, even on the hydrogen project this never happened, because the IndexedDB polling part which forms the code-editor-to-utopia communication channel is slower than parsing the code. So I then added a 600ms pause in the parsing to force the code updates to come through faster, which meant updating the canvas only happened when you stopped typing, and even in that forced case it actually didn't feel bad. **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 --------- Co-authored-by: Sean Parsons <[email protected]>
concrete-utopia · Dec 13, 2024 · 39f050c · 39f050c
1 parent df2b44e
commit 39f050c
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 81 deletions.
diff --git a/editor/src/components/assets.ts b/editor/src/components/assets.ts
@@ -7,7 +7,12 @@ import type {
   AssetFile,
   ParseSuccess,
 } from '../core/shared/project-file-types'
-import { directory, isDirectory, isImageFile } from '../core/shared/project-file-types'
+import {
+  directory,
+  isDirectory,
+  isImageFile,
+  RevisionsState,
+} from '../core/shared/project-file-types'
 import { isTextFile, isParseSuccess, isAssetFile } from '../core/shared/project-file-types'
 import Utils from '../utils/utils'
 import { dropLeadingSlash } from './filebrowser/filepath-utils'
@@ -28,6 +33,8 @@ import type {
   ProjectContentsTree,
   PathAndFileEntry,
 } from 'utopia-shared/src/types/assets'
+import { filtered, fromField, fromTypeGuard } from '../core/shared/optics/optic-creators'
+import { anyBy, toArrayOf } from '../core/shared/optics/optic-utilities'
 export type {
   AssetFileWithFileName,
   ProjectContentTreeRoot,
@@ -391,6 +398,32 @@ export const contentsTreeOptic: Optic<ProjectContentTreeRoot, PathAndFileEntry>
   },
 )
 
+export function anyCodeAhead(tree: ProjectContentTreeRoot): boolean {
+  const revisionsStateOptic = contentsTreeOptic
+    .compose(fromField('file'))
+    .compose(fromTypeGuard(isTextFile))
+    .compose(fromField('fileContents'))
+    .compose(filtered((f) => f.parsed.type === 'PARSE_SUCCESS'))
+    .compose(fromField('revisionsState'))
+
+  return anyBy(
+    revisionsStateOptic,
+    (revisionsState) => {
+      switch (revisionsState) {
+        case 'BOTH_MATCH':
+        case 'PARSED_AHEAD':
+          return false
+        case 'CODE_AHEAD':
+        case 'CODE_AHEAD_BUT_PLEASE_TELL_VSCODE_ABOUT_IT':
+          return true
+        default:
+          assertNever(revisionsState)
+      }
+    },
+    tree,
+  )
+}
+
 export function walkContentsTreeForParseSuccess(
   tree: ProjectContentTreeRoot,
   onElement: (fullPath: string, parseSuccess: ParseSuccess) => void,

diff --git a/editor/src/templates/editor.tsx b/editor/src/templates/editor.tsx
@@ -127,6 +127,7 @@ import { hasReactRouterErrorBeenLogged } from '../core/shared/runtime-report-log
 import { InitialOnlineState, startOnlineStatusPolling } from '../components/editor/online-status'
 import { useAnimate } from 'framer-motion'
 import { AnimationContext } from '../components/canvas/ui-jsx-canvas-renderer/animation-context'
+import { anyCodeAhead } from '../components/assets'
 
 if (PROBABLY_ELECTRON) {
   let { webFrame } = requireElectron()
@@ -450,7 +451,12 @@ export class Editor {
       const reactRouterErrorPreviouslyLogged = hasReactRouterErrorBeenLogged()
 
       const runDomWalker = shouldRunDOMWalker(dispatchedActions, dispatchResult)
-      const shouldRerender = !dispatchResult.nothingChanged || runDomWalker
+
+      const somethingChanged = !dispatchResult.nothingChanged
+
+      const shouldRerender =
+        (somethingChanged || runDomWalker) &&
+        !anyCodeAhead(dispatchResult.unpatchedEditor.projectContents)
 
       const updateId = canvasUpdateId++
       if (shouldRerender) {
@@ -467,102 +473,101 @@ export class Editor {
             })
           })
         })
+      }
+
+      // run the dom-walker
+      if (shouldRerender || runDomWalker) {
+        const domWalkerDispatchResult = runDomWalkerAndSaveResults(
+          this.boundDispatch,
+          this.domWalkerMutableState,
+          this.storedState,
+          this.spyCollector,
+          ElementsToRerenderGLOBAL.current,
+        )
 
-        // run the dom-walker
-        if (runDomWalker) {
-          const domWalkerDispatchResult = runDomWalkerAndSaveResults(
+        if (domWalkerDispatchResult != null) {
+          this.storedState = domWalkerDispatchResult
+          entireUpdateFinished = Promise.all([
+            entireUpdateFinished,
+            domWalkerDispatchResult.entireUpdateFinished,
+          ])
+        }
+      }
+
+      // true up groups if needed
+      if (this.storedState.unpatchedEditor.trueUpElementsAfterDomWalkerRuns.length > 0) {
+        // updated editor with trued up groups
+        Measure.taskTime(`Group true up ${updateId}`, () => {
+          const projectContentsBeforeGroupTrueUp = this.storedState.unpatchedEditor.projectContents
+          const dispatchResultWithTruedUpGroups = editorDispatchActionRunner(
             this.boundDispatch,
-            this.domWalkerMutableState,
+            [{ action: 'TRUE_UP_ELEMENTS' }],
             this.storedState,
             this.spyCollector,
-            ElementsToRerenderGLOBAL.current,
           )
+          this.storedState = dispatchResultWithTruedUpGroups
 
-          if (domWalkerDispatchResult != null) {
-            this.storedState = domWalkerDispatchResult
-            entireUpdateFinished = Promise.all([
-              entireUpdateFinished,
-              domWalkerDispatchResult.entireUpdateFinished,
-            ])
+          entireUpdateFinished = Promise.all([
+            entireUpdateFinished,
+            dispatchResultWithTruedUpGroups.entireUpdateFinished,
+          ])
+
+          if (
+            projectContentsBeforeGroupTrueUp === this.storedState.unpatchedEditor.projectContents
+          ) {
+            // no group-related re-render / re-measure is needed, bail out
+            return
           }
-        }
 
-        // true up groups if needed
-        if (this.storedState.unpatchedEditor.trueUpElementsAfterDomWalkerRuns.length > 0) {
-          // updated editor with trued up groups
-          Measure.taskTime(`Group true up ${updateId}`, () => {
-            const projectContentsBeforeGroupTrueUp =
-              this.storedState.unpatchedEditor.projectContents
-            const dispatchResultWithTruedUpGroups = editorDispatchActionRunner(
+          // re-render the canvas
+          Measure.taskTime(`Canvas re-render because of groups ${updateId}`, () => {
+            ElementsToRerenderGLOBAL.current = fixElementsToRerender(
+              this.storedState.patchedEditor.canvas.elementsToRerender,
+              dispatchedActions,
+            ) // Mutation!
+
+            ReactDOM.flushSync(() => {
+              ReactDOM.unstable_batchedUpdates(() => {
+                this.canvasStore.setState(
+                  patchedStoreFromFullStore(this.storedState, 'canvas-store'),
+                )
+              })
+            })
+          })
+
+          // re-run the dom-walker
+          Measure.taskTime(`Dom walker re-run because of groups ${updateId}`, () => {
+            const domWalkerDispatchResult = runDomWalkerAndSaveResults(
               this.boundDispatch,
-              [{ action: 'TRUE_UP_ELEMENTS' }],
+              this.domWalkerMutableState,
               this.storedState,
               this.spyCollector,
+              ElementsToRerenderGLOBAL.current,
             )
-            this.storedState = dispatchResultWithTruedUpGroups
-
-            entireUpdateFinished = Promise.all([
-              entireUpdateFinished,
-              dispatchResultWithTruedUpGroups.entireUpdateFinished,
-            ])
 
-            if (
-              projectContentsBeforeGroupTrueUp === this.storedState.unpatchedEditor.projectContents
-            ) {
-              // no group-related re-render / re-measure is needed, bail out
-              return
+            if (domWalkerDispatchResult != null) {
+              this.storedState = domWalkerDispatchResult
+              entireUpdateFinished = Promise.all([
+                entireUpdateFinished,
+                domWalkerDispatchResult.entireUpdateFinished,
+              ])
             }
-
-            // re-render the canvas
-            Measure.taskTime(`Canvas re-render because of groups ${updateId}`, () => {
-              ElementsToRerenderGLOBAL.current = fixElementsToRerender(
-                this.storedState.patchedEditor.canvas.elementsToRerender,
-                dispatchedActions,
-              ) // Mutation!
-
-              ReactDOM.flushSync(() => {
-                ReactDOM.unstable_batchedUpdates(() => {
-                  this.canvasStore.setState(
-                    patchedStoreFromFullStore(this.storedState, 'canvas-store'),
-                  )
-                })
-              })
-            })
-
-            // re-run the dom-walker
-            Measure.taskTime(`Dom walker re-run because of groups ${updateId}`, () => {
-              const domWalkerDispatchResult = runDomWalkerAndSaveResults(
-                this.boundDispatch,
-                this.domWalkerMutableState,
-                this.storedState,
-                this.spyCollector,
-                ElementsToRerenderGLOBAL.current,
-              )
-
-              if (domWalkerDispatchResult != null) {
-                this.storedState = domWalkerDispatchResult
-                entireUpdateFinished = Promise.all([
-                  entireUpdateFinished,
-                  domWalkerDispatchResult.entireUpdateFinished,
-                ])
-              }
-            })
           })
-        }
-
-        this.storedState = editorDispatchClosingOut(
-          this.boundDispatch,
-          dispatchedActions,
-          oldEditorState,
-          {
-            ...this.storedState,
-            entireUpdateFinished: entireUpdateFinished,
-            nothingChanged: dispatchResult.nothingChanged,
-          },
-          reactRouterErrorPreviouslyLogged,
-        )
+        })
       }
 
+      this.storedState = editorDispatchClosingOut(
+        this.boundDispatch,
+        dispatchedActions,
+        oldEditorState,
+        {
+          ...this.storedState,
+          entireUpdateFinished: entireUpdateFinished,
+          nothingChanged: dispatchResult.nothingChanged,
+        },
+        reactRouterErrorPreviouslyLogged,
+      )
+
       Measure.taskTime(`Update Editor ${updateId}`, () => {
         ReactDOM.flushSync(() => {
           ReactDOM.unstable_batchedUpdates(() => {