fix: do not wait for navigation event in navigate steps (#859)

page.goto() already does it
puppeteer · Aug 9, 2024 · 2e48a56 · 2e48a56
1 parent 223e2f0
commit 2e48a56
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 30 deletions.
diff --git a/__snapshots__/LighthouseStringifyExtension.test.ts.js b/__snapshots__/LighthouseStringifyExtension.test.ts.js
@@ -26,13 +26,7 @@ const puppeteer = require('puppeteer'); // v23.0.0 or later
   await lhFlow.startNavigation();
   {
     const targetPage = page;
-    const promises = [];
-    const startWaitingForEvents = () => {
-      promises.push(targetPage.waitForNavigation());
-    }
-    startWaitingForEvents();
     await targetPage.goto('https://example.com');
-    await Promise.all(promises);
   }
   await lhFlow.endNavigation();
   await lhFlow.startTimespan();
@@ -84,13 +78,7 @@ const puppeteer = require('puppeteer'); // v23.0.0 or later
   await lhFlow.startNavigation();
   {
     const targetPage = page;
-    const promises = [];
-    const startWaitingForEvents = () => {
-      promises.push(targetPage.waitForNavigation());
-    }
-    startWaitingForEvents();
     await targetPage.goto('https://example.com');
-    await Promise.all(promises);
   }
   await lhFlow.endNavigation();
   await lhFlow.startTimespan();
@@ -111,13 +99,7 @@ const puppeteer = require('puppeteer'); // v23.0.0 or later
   await lhFlow.startNavigation();
   {
     const targetPage = page;
-    const promises = [];
-    const startWaitingForEvents = () => {
-      promises.push(targetPage.waitForNavigation());
-    }
-    startWaitingForEvents();
     await targetPage.goto('https://example.com/page/');
-    await Promise.all(promises);
   }
   await lhFlow.endNavigation();
   const lhFlowReport = await lhFlow.generateReport();
@@ -156,13 +138,7 @@ const puppeteer = require('puppeteer'); // v23.0.0 or later
   await lhFlow.startNavigation();
   {
     const targetPage = page;
-    const promises = [];
-    const startWaitingForEvents = () => {
-      promises.push(targetPage.waitForNavigation());
-    }
-    startWaitingForEvents();
     await targetPage.goto('https://example.com');
-    await Promise.all(promises);
   }
   await lhFlow.endNavigation();
   await lhFlow.startNavigation();

diff --git a/src/PuppeteerStringifyExtension.ts b/src/PuppeteerStringifyExtension.ts
@@ -96,10 +96,12 @@ export class PuppeteerStringifyExtension extends StringifyExtension {
       out.appendLine(`const timeout = ${step.timeout};`);
     }
     this.#appendContext(out, step);
-    if (step.assertedEvents) {
+    const waitForEvents =
+      step.assertedEvents && step.type !== StepType.Navigate;
+    if (waitForEvents) {
       out.appendLine('const promises = [];');
       out.appendLine('const startWaitingForEvents = () => {').startBlock();
-      for (const event of step.assertedEvents) {
+      for (const event of step.assertedEvents!) {
         switch (event.type) {
           case AssertedEventType.Navigation: {
             out.appendLine(
@@ -118,7 +120,7 @@ export class PuppeteerStringifyExtension extends StringifyExtension {
 
     this.#appendStepType(out, step);
 
-    if (step.assertedEvents) {
+    if (waitForEvents) {
       out.appendLine('await Promise.all(promises);');
     }
 
@@ -326,9 +328,6 @@ export class PuppeteerStringifyExtension extends StringifyExtension {
   }
 
   #appendNavigationStep(out: LineWriter, step: NavigateStep): void {
-    if (step.assertedEvents?.length) {
-      out.appendLine(`startWaitingForEvents();`);
-    }
     out.appendLine(
       `await targetPage.goto(${formatJSONAsJS(step.url, out.getIndent())});`
     );