Revised fix for cross-tab issues

The previous fix still had issues when creating a new project in a second tab
and switching between them.

I initially fixed this by taking the same approach for new project (via
MakeCode) but it's slow and you see the old project render. The loading states
that we'd need don't seen proportionate.

So I've gone with:
- updating MakeCode only as needed
- ensuring import from .org has a header id by setting one explicitly
- carefully tracking when we expect to get new content from MakeCode in
  editorChange

Fixes #618

Author: microbit-matt-hillsdon

src/hooks/project-hooks.tsx

@@ -130,7 +130,8 @@ export const ProjectProvider = ({
   const openEditorTimedOutDialog = useStore(
     (s) => () => s.setIsEditorTimedOutDialogOpen(true)
   );
-  const expectChangedHeader = useStore((s) => s.setChangedHeaderExpected);
+  const setEditorLoadingFile = useStore((s) => s.setEditorLoadingFile);
+  const setEditorImportingState = useStore((s) => s.setEditorImportingState);
   const projectFlushedToEditor = useStore((s) => s.projectFlushedToEditor);
   const checkIfProjectNeedsFlush = useStore((s) => s.checkIfProjectNeedsFlush);
   const getCurrentProject = useStore((s) => s.getCurrentProject);
@@ -218,8 +219,8 @@ export const ProjectProvider = ({
             logging.log("[MakeCode] Importing project");
             await editorReadyPromise.promise;
             const project = getCurrentProject();
-            expectChangedHeader();
             try {
+              setEditorImportingState();
               await driverRef.current.importProject({ project });
               logging.log("[MakeCode] Project import succeeded");
               projectFlushedToEditor();
@@ -260,7 +261,7 @@ export const ProjectProvider = ({
       logging,
       editorReadyPromise.promise,
       getCurrentProject,
-      expectChangedHeader,
+      setEditorImportingState,
       projectFlushedToEditor,
       langChangeFlushedToEditor,
       checkIfEditorStartUpTimedOut,
@@ -332,6 +333,7 @@ export const ProjectProvider = ({
             return;
           }
           // This triggers the code in editorChanged to update actions etc.
+          setEditorLoadingFile();
           driverRef.current!.importFile({
             filename: file.name,
             parts: [hex],
@@ -346,12 +348,13 @@ export const ProjectProvider = ({
     [
       checkIfEditorStartUpTimedOut,
       driverRef,
-      editorReadyPromise,
+      editorReadyPromise.promise,
       loadDataset,
       logging,
       navigate,
       openEditorTimedOutDialog,
       setPostImportDialogState,
+      setEditorLoadingFile,
     ]
   );
 

src/store.ts

@@ -159,10 +159,26 @@ export interface State {
   projectLoadTimestamp: number;
   // false if we're sure the user hasn't changed the project, otherwise true
   projectEdited: boolean;
-  changedHeaderExpected: boolean;
+  /**
+   * Set to true when we need to update MakeCode before we open it.
+   */
   appEditNeedsFlushToEditor: boolean;
   isEditorOpen: boolean;
   isEditorReady: boolean;
+  /**
+   * Whether we're expecting an editorChange call with a new header id
+   * because we've
+   *
+   * In this case we need to update app state from the project.
+   */
+  isEditorLoadingFile: boolean;
+  /**
+   * Whether we're expecting a editorChange call with a new header id
+   * because we've imported an updated copy of the project.
+   *
+   * In this case app state (e.g. dataset) will already be in sync.
+   */
+  isEditorImportingState: boolean;
   editorStartUp: EditorStartUp;
   editorStartUpTimestamp: number;
   editorPromises: {
@@ -253,7 +269,8 @@ export interface Actions {
   editorTimedOut(): void;
   getEditorStartUp(): EditorStartUp;
   setIsEditorTimedOutDialogOpen(isOpen: boolean): void;
-  setChangedHeaderExpected(): void;
+  setEditorLoadingFile(): void;
+  setEditorImportingState(): void;
   projectFlushedToEditor(): void;
 
   setDownload(state: DownloadState): void;
@@ -316,6 +333,8 @@ const createMlStore = (logging: Logging) => {
           model: undefined,
           isEditorOpen: false,
           isEditorReady: false,
+          isEditorLoadingFile: false,
+          isEditorImportingState: false,
           editorStartUp: "in-progress",
           editorStartUpTimestamp: Date.now(),
           editorPromises: {
@@ -325,7 +344,7 @@ const createMlStore = (logging: Logging) => {
           isEditorTimedOutDialogOpen: false,
           langChanged: false,
           appEditNeedsFlushToEditor: true,
-          changedHeaderExpected: false,
+          waitingForLoad: false,
           // This dialog flow spans two pages
           trainModelDialogStage: TrainModelDialogStage.Closed,
           trainModelProgress: 0,
@@ -669,7 +688,16 @@ const createMlStore = (logging: Logging) => {
            */
           loadProject(project: MakeCodeProject, name: string) {
             const newActions = getActionsFromProject(project);
-            set(({ settings }) => {
+            set(({ settings, project: prevProject }) => {
+              project = renameProject(project, name);
+              project = {
+                ...project,
+                header: {
+                  ...project.header!,
+                  // .org projects have a partial header with no id which causes MakeCode sadness
+                  id: project.header?.id ?? prevProject.header!.id,
+                },
+              };
               const timestamp = Date.now();
               return {
                 settings: {
@@ -681,7 +709,7 @@ const createMlStore = (logging: Logging) => {
                 actions: newActions,
                 dataWindow: getDataWindowFromActions(newActions),
                 model: undefined,
-                project: renameProject(project, name),
+                project,
                 projectEdited: true,
                 appEditNeedsFlushToEditor: true,
                 timestamp,
@@ -842,35 +870,60 @@ const createMlStore = (logging: Logging) => {
           },
 
           editorChange(newProject: MakeCodeProject) {
+            // Notes on past issues with the MakeCode integration:
+            //
+            // We update MakeCode only as needed. However, it loads the background
+            // because we need it to be ready. This means it will have the initial
+            // project from the state open. So we must be sure to update MakeCode
+            // before "Edit in MakeCode" and "Save" actions.
+            //
+            // MakeCode has a visibility listener that will cause it to re-run
+            // its initialization when its tab becomes hidden/visible. We aim to
+            // ignore this when MakeCode is closed. When MakeCode is open we'll have
+            // up-to-date state to reinit MakeCode with. In the past this has
+            // caused us to update app state with old data from MakeCode.
+            //
+            // It's too slow/async from a UI perspective to rely on only
+            // understanding project contents via editorChange as they're
+            // delayed by MakeCode load and then the async nature of loading a
+            // project.
+            //
+            // We have no choice but to write to MakeCode and wait for the project
+            // data in editorChange when loading a hex file.
+
             const actionName = "editorChange";
             set(
               (state) => {
                 const {
                   project: prevProject,
                   isEditorOpen,
-                  isEditorReady,
-                  changedHeaderExpected,
+                  isEditorImportingState,
+                  isEditorLoadingFile,
                   settings,
                 } = state;
                 const newProjectHeader = newProject.header!.id;
                 const previousProjectHeader = prevProject.header!.id;
-                if (newProjectHeader !== previousProjectHeader) {
-                  if (changedHeaderExpected) {
+                if (
+                  (isEditorLoadingFile ||
+                    isEditorOpen ||
+                    isEditorImportingState) &&
+                  newProjectHeader !== previousProjectHeader
+                ) {
+                  if (isEditorImportingState) {
+                    // It's a change but we originated it so state is in sync.
                     logging.log(
-                      `[MakeCode] Detected new project, ignoring as expected due to import. ID change: ${prevProject.header?.id} -> ${newProject.header?.id}`
+                      `[MakeCode] Ignored header change due to us syncing state. ID change: ${prevProject.header?.id} -> ${newProject.header?.id}`
                     );
                     return {
-                      changedHeaderExpected: false,
+                      isEditorImportingState: false,
+                      // Still need to update this for the new header id.
                       project: newProject,
                     };
-                  }
-                  if (isEditorReady) {
+                  } else {
+                    // It's a change that originated in MakeCode, e.g. a hex load, so update our state.
                     logging.log(
-                      `[MakeCode] Detected new project, loading actions. ID change: ${prevProject.header?.id} -> ${newProject.header?.id}`
+                      `[MakeCode] Updating state from MakeCode header change. ID change: ${prevProject.header?.id} -> ${newProject.header?.id}`
                     );
-                    // It's a new project. Thanks user. We'll update our state.
-                    // This will cause another write to MakeCode but that's OK as it gives us
-                    // a chance to validate/update the project
                     const timestamp = Date.now();
                     const newActions = getActionsFromProject(newProject);
                     return {
@@ -892,12 +945,8 @@ const createMlStore = (logging: Logging) => {
                       dataWindow: getDataWindowFromActions(newActions),
                       model: undefined,
                       isEditorOpen: false,
+                      isEditorLoadingFile: false,
                     };
-                  } else {
-                    // In particular, this happens if the MakeCode init completes after we've updated our project state from an import from .org
-                    logging.log(
-                      `[MakeCode] Ignoring changed ID before editor ready. ID change: ${prevProject.header?.id} -> ${newProject.header?.id}`
-                    );
                   }
                 } else if (isEditorOpen) {
                   logging.log(
@@ -909,6 +958,7 @@ const createMlStore = (logging: Logging) => {
                     projectEdited: true,
                   };
                 } else {
+                  // This lets us skip more pointless init-time edits.
                   logging.log(
                     `[MakeCode] Edit ignored when closed. ID ${newProject.header?.id}`
                   );
@@ -932,11 +982,14 @@ const createMlStore = (logging: Logging) => {
           setSave(save: SaveState) {
             set({ save }, false, "setSave");
           },
-          setChangedHeaderExpected() {
+          setEditorLoadingFile() {
+            set({ isEditorLoadingFile: true }, false, "setEditorLoadingFile");
+          },
+          setEditorImportingState() {
             set(
-              { changedHeaderExpected: true },
+              { isEditorImportingState: true },
               false,
-              "setChangedHeaderExpected"
+              "setEditorImportingState"
             );
           },
           langChangeFlushedToEditor() {