Properly wait for router in navigate command to avoid race with query…

… param
google · Oct 27, 2024 · 2b33578 · 2b33578
1 parent fcb1c61
commit 2b33578
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 9 deletions.
diff --git a/mesop/examples/query_params.py b/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")
diff --git a/mesop/tests/e2e/query_params_test.ts b/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}) => {

diff --git a/mesop/web/src/services/channel.ts b/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 {
@@ -237,7 +237,7 @@ export class Channel {
     };
   }
 
-  private handleUiResponse(
+  private async handleUiResponse(
     request: UiRequest,
     uiResponse: UiResponse,
     initParams: InitParams,
@@ -284,7 +284,7 @@ export class Channel {
         const componentDiff = uiResponse.getRender()!.getComponentDiff()!;
 
         for (const command of uiResponse.getRender()!.getCommandsList()) {
-          onCommand(command);
+          await onCommand(command);
         }
 
         const title = this.overridedTitle || uiResponse.getRender()!.getTitle();

diff --git a/mesop/web/src/shell/shell.ts b/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()) {