Fix race condition in processing commands (navigate) (#1065)

mesop/examples/query_params.py

@@ -105,7 +105,21 @@ def increment_by_navigate(e: me.ClickEvent):
 def page_2():
   me.text(f"query_params(page_2)={me.query_params}")
   me.button("Navigate back", on_click=navigate_back)
+  me.button("Navigate page 3", on_click=navigate_page_3)
+
+
+def on_load_page_3(e: me.LoadEvent):
+  me.query_params["on_load_page_3"] = "loaded"
+
+
+@me.page(path="/examples/query_params/page_3", on_load=on_load_page_3)
+def page_3():
+  me.text(f"query_params(page_3)={me.query_params}")
 
 
 def navigate_back(e: me.ClickEvent):
   me.navigate("/examples/query_params", query_params=me.query_params)
+
+
+def navigate_page_3(e: me.ClickEvent):
+  me.navigate("/examples/query_params/page_3")

mesop/tests/e2e/query_params_test.ts

@@ -8,6 +8,20 @@ test('query_param: on_load hook', async ({page}) => {
   await expect(page).toHaveURL(/\?on_load=loaded$/);
 });
 
+test('query_param: navigate then on_load hook', async ({page}) => {
+  await page.goto('/examples/query_params/page_2');
+  await page
+    .getByRole('button', {
+      name: 'Navigate page 3',
+      exact: true,
+    })
+    .click();
+  expect(await page.getByText('query_params(page_3)=').textContent()).toEqual(
+    `query_params(page_3)={'on_load_page_3': ['loaded']}`,
+  );
+  await expect(page).toHaveURL(/page_3\?on_load_page_3=loaded$/);
+});
+
 test('query_param: load with query param in URL', async ({page}) => {
   await page.goto('/examples/query_params?a=1');
   expect(await page.getByText('query_params={').textContent()).toEqual(
@@ -87,23 +101,28 @@ test('query_param: increment query param ', async ({page}) => {
   await expect(
     page.getByText(`query_params={'on_load': ['loaded'], 'counter': ['1']}`),
   ).toBeVisible();
-  await expect(page).toHaveURL(/\?on_load=loaded&counter=1/);
-
+  let url = new URL(page.url());
+  expect(url.searchParams.get('on_load')).toBe('loaded');
+  expect(url.searchParams.get('counter')).toBe('1');
   await page
     .getByRole('button', {name: 'increment query param directly'})
     .click();
   await expect(
     page.getByText(`query_params={'on_load': ['loaded'], 'counter': ['2']}`),
   ).toBeVisible();
-  await expect(page).toHaveURL(/\?on_load=loaded&counter=2/);
+  url = new URL(page.url());
+  expect(url.searchParams.get('on_load')).toBe('loaded');
+  expect(url.searchParams.get('counter')).toBe('2');
 
   await page
     .getByRole('button', {name: 'increment query param by navigate'})
     .click();
   await expect(
     page.getByText(`query_params={'on_load': ['loaded'], 'counter': ['3']}`),
   ).toBeVisible();
-  await expect(page).toHaveURL(/\?on_load=loaded&counter=3/);
+  url = new URL(page.url());
+  expect(url.searchParams.get('on_load')).toBe('loaded');
+  expect(url.searchParams.get('counter')).toBe('3');
 });
 
 test('query_param: delete all query params ', async ({page}) => {

mesop/web/src/services/channel.ts

@@ -36,7 +36,7 @@ interface InitParams {
     jsModules: readonly string[],
   ) => void;
   onError: (error: ServerError) => void;
-  onCommand: (command: Command) => void;
+  onCommand: (command: Command) => Promise<void>;
 }
 
 export enum ChannelStatus {
@@ -64,6 +64,8 @@ export class Channel {
   private queuedEvents: (() => void)[] = [];
   private hotReloadBackoffCounter = 0;
   private hotReloadCounter = 0;
+  private commandQueue: Command[] = [];
+  private commandQueuePromise: Promise<void> | undefined;
 
   // Client-side state
   private overridedTitle = '';
@@ -237,7 +239,7 @@ export class Channel {
     };
   }
 
-  private handleUiResponse(
+  private async handleUiResponse(
     request: UiRequest,
     uiResponse: UiResponse,
     initParams: InitParams,
@@ -283,9 +285,8 @@ export class Channel {
         const rootComponent = uiResponse.getRender()!.getRootComponent()!;
         const componentDiff = uiResponse.getRender()!.getComponentDiff()!;
 
-        for (const command of uiResponse.getRender()!.getCommandsList()) {
-          onCommand(command);
-        }
+        this.commandQueue.push(...uiResponse.getRender()!.getCommandsList());
+        await this.processCommandQueue(onCommand);
 
         const title = this.overridedTitle || uiResponse.getRender()!.getTitle();
         if (title) {
@@ -354,6 +355,27 @@ export class Channel {
     }
   }
 
+  private async processCommandQueue(
+    onCommand: (command: Command) => Promise<void>,
+  ) {
+    if (this.commandQueuePromise) {
+      return this.commandQueuePromise;
+    }
+    let resolveHandle!: () => void;
+    this.commandQueuePromise = new Promise((resolve) => {
+      resolveHandle = resolve;
+    });
+    try {
+      while (this.commandQueue.length > 0) {
+        const command = this.commandQueue.shift()!;
+        await onCommand(command);
+      }
+    } finally {
+      this.commandQueuePromise = undefined;
+      resolveHandle();
+    }
+  }
+
   dispatch(userEvent: UserEvent) {
     // Every user event should have an event handler,
     // except for the ones below:

mesop/web/src/shell/shell.ts

@@ -115,13 +115,13 @@ export class Shell {
             this.componentConfigs = componentConfigs;
           }
         },
-        onCommand: (command) => {
+        onCommand: async (command) => {
           if (command.hasNavigate()) {
             const url = command.getNavigate()!.getUrl()!;
             if (url.startsWith('http://') || url.startsWith('https://')) {
               window.location.href = url;
             } else {
-              this.router.navigateByUrl(command.getNavigate()!.getUrl()!);
+              await this.router.navigateByUrl(command.getNavigate()!.getUrl()!);
               this.channel.resetOverridedTitle();
             }
           } else if (command.hasScrollIntoView()) {