feat(import): split checks (#6618)

This PR splits the validation checks to allow an early break before parsing the files. In the UI they are split to these phases: - **Validating code** (which are actually the pre-parse validations) - Parse files (as today) - **Checking Utopia requirements** (which are the post-parse validations) **Details:** 1. The requirements are still aggregated in the editor state, but they are separated to phases for running as well as for displaying in the wizard ([see the separation here](https://github.com/concrete-utopia/utopia/pull/6618/files#diff-15c53165ea0f64ccf6f092783f2dce88639666dd1f4fd29e46035dc96ca4f2b3R20-R30)). 2. Main changes are in [editor/src/core/shared/import/project-health-check/utopia-requirements-service.ts](https://github.com/concrete-utopia/utopia/pull/6618/files#diff-9691feb390abe09b4d2b2aca374b464cf8bcb91a10942f8f6f07a2b4e9355fa6), where the split actually happens (`startPreParseValidation` and `startPostParseValidation`). 3. Renaming the requirement phases added changes in several other files, but they are simply refactors. 4. Minor - we also remove the shimmer when pausing the wizard (to make it clearer that the import has stopped). See here - first a successful project and then one with errors: <video src="https://github.com/user-attachments/assets/a907fe6b-68be-44a9-8a01-7e951ed90001"></video> **Manual Tests:** I hereby swear that: - [X] I opened a hydrogen project and it loaded - [X] I could navigate to various routes in Play mode
concrete-utopia · Nov 7, 2024 · b08b24f · b08b24f
1 parent c49b3b9
commit b08b24f
Show file tree

Hide file tree

Showing 16 changed files with 351 additions and 177 deletions.
diff --git a/editor/src/components/canvas/canvas-loading-screen.tsx b/editor/src/components/canvas/canvas-loading-screen.tsx
@@ -1,9 +1,35 @@
 import React from 'react'
 import { Global, css } from '@emotion/react'
 import { useColorTheme } from '../../uuiui'
+import { useEditorState } from '../editor/store/store-hook'
+import { Substores } from '../editor/store/store-hook'
 
 export const CanvasLoadingScreen = React.memo(() => {
   const colorTheme = useColorTheme()
+  const importState = useEditorState(
+    Substores.github,
+    (store) => store.editor.importState,
+    'CanvasLoadingScreen importState',
+  )
+  const importWizardOpen = useEditorState(
+    Substores.restOfEditor,
+    (store) => store.editor.importWizardOpen,
+    'CanvasLoadingScreen importWizardOpen',
+  )
+
+  const importingStoppedStyleOverride = React.useMemo(
+    () =>
+      // if the importing was stopped, we want to pause the shimmer animation
+      (importWizardOpen && importState.importStatus.status === 'done') ||
+      importState.importStatus.status === 'paused'
+        ? {
+            background: colorTheme.codeEditorShimmerPrimary.value,
+            animation: 'none',
+          }
+        : {},
+    [importWizardOpen, importState.importStatus.status, colorTheme.codeEditorShimmerPrimary.value],
+  )
+
   return (
     <React.Fragment>
       <Global
@@ -34,6 +60,11 @@ export const CanvasLoadingScreen = React.memo(() => {
             background-size: 1468px 104px;
             position: relative;
           }
+
+          .no-shimmer {
+            animation: none;
+            background: ${colorTheme.codeEditorShimmerPrimary.value};
+          }
         `}
       />
       <div
@@ -48,6 +79,7 @@ export const CanvasLoadingScreen = React.memo(() => {
             top: 0,
             width: '100vw',
             height: '100vh',
+            ...importingStoppedStyleOverride,
           }}
         ></div>
       </div>

diff --git a/editor/src/components/editor/import-wizard/components.tsx b/editor/src/components/editor/import-wizard/components.tsx
@@ -3,15 +3,13 @@
 import { jsx } from '@emotion/react'
 import React from 'react'
 import type {
-  ImportCheckRequirementAndFix,
   ImportFetchDependency,
   ImportOperation,
 } from '../../../core/shared/import/import-operation-types'
 import { ImportOperationResult } from '../../../core/shared/import/import-operation-types'
 import { assertNever } from '../../../core/shared/utils'
 import { Icn, Icons, useColorTheme } from '../../../uuiui'
 import { GithubSpinner } from '../../../components/navigator/left-pane/github-pane/github-spinner'
-import { RequirementResolutionResult } from '../../../core/shared/import/project-health-check/utopia-requirements-types'
 
 export function OperationLine({ operation }: { operation: ImportOperation }) {
   const operationRunningStatus = React.useMemo(() => {
@@ -58,7 +56,18 @@ export function OperationLine({ operation }: { operation: ImportOperation }) {
           </div>
         ) : null}
       </OperationLineContent>
-      {shouldShowChildren && hasChildren ? <OperationChildrenList operation={operation} /> : null}
+      {shouldShowChildren && hasChildren ? (
+        <div
+          className='import-wizard-operation-children'
+          style={{
+            display: 'flex',
+            flexDirection: 'column',
+            gap: 15,
+          }}
+        >
+          <OperationChildrenList operation={operation} />
+        </div>
+      ) : null}
     </OperationLineWrapper>
   )
 }
@@ -67,39 +76,31 @@ function OperationChildrenList({ operation }: { operation: ImportOperation }) {
   if (operation.children == null || operation.children.length === 0) {
     return null
   }
+  // this is a special case where we don't list all of the children
+  // but we collapse the successful ones to a single line
+  if (operation.type === 'refreshDependencies') {
+    return (
+      <AggregatedChildrenStatus
+        childOperations={operation.children as ImportFetchDependency[]}
+        successFn={dependenciesSuccessFn}
+        successTextFn={dependenciesSuccessTextFn}
+      />
+    )
+  }
+  // otherwise, we list all of the children
   return (
-    <div
-      className='import-wizard-operation-children'
-      style={{
-        display: 'flex',
-        flexDirection: 'column',
-        gap: 15,
-      }}
-    >
-      {operation.type === 'refreshDependencies' ? (
-        <AggregatedChildrenStatus
-          childOperations={operation.children as ImportFetchDependency[]}
-          successFn={dependenciesSuccessFn}
-          successTextFn={dependenciesSuccessTextFn}
-        />
-      ) : operation.type === 'checkRequirements' ? (
-        operation.children.map((childOperation) => (
-          <OperationLine
-            key={childOperation.id ?? childOperation.type}
-            operation={childOperation}
-          />
-        ))
-      ) : null}
-    </div>
+    <React.Fragment>
+      {operation.children?.map((childOperation) => (
+        <OperationLine key={childOperation.id ?? childOperation.type} operation={childOperation} />
+      ))}
+    </React.Fragment>
   )
 }
+
 const dependenciesSuccessFn = (op: ImportFetchDependency) =>
   op.result === ImportOperationResult.Success
 const dependenciesSuccessTextFn = (successCount: number) =>
   `${successCount} dependencies fetched successfully`
-const requirementsSuccessFn = (op: ImportCheckRequirementAndFix) =>
-  op.resolution === RequirementResolutionResult.Passed
-const requirementsSuccessTextFn = (successCount: number) => `${successCount} requirements met`
 
 function AggregatedChildrenStatus<T extends ImportOperation>({
   childOperations,
@@ -255,7 +256,7 @@ function getImportOperationText(operation: ImportOperation): React.ReactNode {
       if (operation.branchName != null) {
         return (
           <span>
-            Loading branch{' '}
+            Fetching branch{' '}
             <strong>
               {operation.githubRepo?.owner}/{operation.githubRepo?.repository}@
               {operation.branchName}
@@ -265,7 +266,7 @@ function getImportOperationText(operation: ImportOperation): React.ReactNode {
       } else {
         return (
           <span>
-            Loading repository{' '}
+            Fetching repository{' '}
             <strong>
               {operation.githubRepo?.owner}/{operation.githubRepo?.repository}
             </strong>
@@ -278,9 +279,13 @@ function getImportOperationText(operation: ImportOperation): React.ReactNode {
       return 'Parsing files'
     case 'refreshDependencies':
       return 'Fetching dependencies'
-    case 'checkRequirements':
+    case 'checkRequirementsPreParse':
+      return 'Validating code'
+    case 'checkRequirementsPostParse':
       return 'Checking Utopia requirements'
-    case 'checkRequirementAndFix':
+    case 'checkRequirementAndFixPreParse':
+      return operation.text
+    case 'checkRequirementAndFixPostParse':
       return operation.text
     default:
       assertNever(operation)

diff --git a/editor/src/components/editor/import-wizard/import-wizard.tsx b/editor/src/components/editor/import-wizard/import-wizard.tsx
@@ -41,12 +41,6 @@ export const ImportWizard = React.memo(() => {
 
   const operations = importState.importOperations
 
-  const dispatch = useDispatch()
-
-  const handleDismiss = React.useCallback(() => {
-    hideImportWizard(dispatch)
-  }, [dispatch])
-
   const stopPropagation = React.useCallback((e: React.MouseEvent) => {
     e.stopPropagation()
   }, [])
@@ -84,7 +78,7 @@ export const ImportWizard = React.memo(() => {
             boxShadow: UtopiaStyles.popup.boxShadow,
             borderRadius: 10,
             width: 600,
-            height: 450,
+            height: 480,
             position: 'relative',
             display: 'flex',
             flexDirection: 'column',
@@ -106,20 +100,7 @@ export const ImportWizard = React.memo(() => {
               flex: 'none',
             }}
           >
-            <div css={{ fontSize: 16, fontWeight: 400 }}>Loading Project</div>
-            {when(
-              totalImportResult.importStatus.status === 'in-progress',
-              <Button
-                highlight
-                style={{
-                  padding: 15,
-                  color: colorTheme.fg6.value,
-                }}
-                onClick={handleDismiss}
-              >
-                Cancel
-              </Button>,
-            )}
+            <div css={{ fontSize: 16, fontWeight: 400 }}>Cloning Project</div>
           </FlexRow>
           <div
             className='import-wizard-body'
@@ -218,56 +199,66 @@ function ActionButtons({ importResult }: { importResult: TotalImportResult }) {
   ) {
     return null
   }
-  if (importResult.result == ImportOperationResult.Success) {
-    return (
-      <React.Fragment>
-        <div style={textStyle}>Project Imported Successfully</div>
-        <Button onClick={hideWizard} style={buttonStyle}>
-          Continue To Editor
-        </Button>
-      </React.Fragment>
-    )
-  }
-  if (importResult.result == ImportOperationResult.Warn) {
-    return (
-      <React.Fragment>
-        <div style={textStyle}>Project Imported With Warnings</div>
-        <Button onClick={hideWizard} style={buttonStyle}>
-          Continue To Editor
-        </Button>
-      </React.Fragment>
-    )
-  }
-  if (
-    importResult.result == ImportOperationResult.Error ||
-    importResult.result == ImportOperationResult.CriticalError
-  ) {
-    return (
-      <React.Fragment>
-        <div style={textStyle}>
-          {importResult.importStatus.status !== 'done' ||
-          importResult.result === ImportOperationResult.CriticalError
-            ? 'Error Importing Project'
-            : 'Project Imported With Errors'}
-        </div>
-        <Button style={{ ...buttonStyle, marginLeft: 'auto' }} onClick={importADifferentProject}>
-          Import A Different Project
-        </Button>
-        {unless(
-          importResult.result == ImportOperationResult.CriticalError,
-          <Button
-            style={{
-              cursor: 'pointer',
-            }}
-            onClick={continueAnyway}
-          >
-            {importResult.importStatus.status === 'done'
-              ? 'Continue To Editor'
-              : 'Continue Importing'}
-          </Button>,
-        )}
-      </React.Fragment>
-    )
+  switch (importResult.result) {
+    case ImportOperationResult.Success:
+      return (
+        <React.Fragment>
+          <div style={textStyle}>Project Imported Successfully</div>
+          <Button onClick={hideWizard} style={buttonStyle}>
+            Continue To Editor
+          </Button>
+        </React.Fragment>
+      )
+    case ImportOperationResult.Warn:
+      return (
+        <React.Fragment>
+          <div style={textStyle}>Project Imported With Warnings</div>
+          <Button onClick={hideWizard} style={buttonStyle}>
+            Continue To Editor
+          </Button>
+        </React.Fragment>
+      )
+    case ImportOperationResult.CriticalError:
+      return (
+        <React.Fragment>
+          <div style={textStyle}>Error Importing Project</div>
+          <Button style={{ ...buttonStyle, marginLeft: 'auto' }} onClick={importADifferentProject}>
+            Cancel
+          </Button>
+        </React.Fragment>
+      )
+    case ImportOperationResult.Error:
+      return (
+        <React.Fragment>
+          <div style={textStyle}>
+            {importResult.importStatus.status !== 'done'
+              ? 'Error While Importing Project'
+              : 'Project Imported With Errors'}
+          </div>
+          {when(
+            importResult.importStatus.status !== 'done',
+            <Button
+              style={{
+                cursor: 'pointer',
+                marginLeft: 'auto',
+              }}
+              onClick={continueAnyway}
+            >
+              Continue Anyway
+            </Button>,
+          )}
+          {importResult.importStatus.status === 'done' ? (
+            <Button style={{ ...buttonStyle }} onClick={hideWizard}>
+              Continue To Editor
+            </Button>
+          ) : (
+            <Button style={{ ...buttonStyle }} onClick={importADifferentProject}>
+              Cancel
+            </Button>
+          )}
+        </React.Fragment>
+      )
+    default:
+      assertNever(importResult.result)
   }
-  return null
 }
diff --git a/editor/src/components/editor/store/editor-state.ts b/editor/src/components/editor/store/editor-state.ts
@@ -191,13 +191,13 @@ import type { FancyError } from '../../../core/shared/code-exec-utils'
 import type { GridCellCoordinates } from '../../canvas/canvas-strategies/strategies/grid-cell-bounds'
 import {
   emptyImportState,
-  type ImportOperation,
   type ImportState,
 } from '../../../core/shared/import/import-operation-types'
 import {
   emptyProjectRequirements,
   type ProjectRequirements,
 } from '../../../core/shared/import/project-health-check/utopia-requirements-types'
+import { isFeatureEnabled } from '../../../utils/feature-switches'
 
 const ObjectPathImmutable: any = OPI
 
@@ -362,6 +362,8 @@ export function githubOperationLocksEditor(op: GithubOperation): boolean {
     case 'loadRepositories':
     case 'listPullRequestsForBranch':
       return false
+    case 'loadBranch':
+      return !isFeatureEnabled('Import Wizard')
     default:
       return true
   }