puppeteer: waitForNavigation doesn't work after clicking a link

@ebidel Sorry if this is not the place to discuss standard-javascript implementation/understanding. What suggested @vsemozhetbyt works for me. But I still think your suggestion has the same behavior as my code, I would like to clarify if am I wrong for not to confuse other readers before I close the issue.

OPTION-1 (@leo020588) - my original code doesn’t work as expected (Error: Navigation Timeout)

// I read this as:
// - wait for page.click() to resolve, then after resolves ...
// - wait for page.waitForNavigation() to resolve, then after resolves ...
// - console.log
await page.click('a');
await page.waitForNavigation();
console.log('The wait is over 1');

OPTION-2 (@vsemozhetbyt) - works as expected (“The wait is over 2”)

// I read this as:
// - trigger page.click(), don't wait to resolve, inmediately after ...
// - wait for page.waitForNavigation() to resolve, then after resolves ...
// - then console.log"
page.click('a');
await page.waitForNavigation();
console.log('The wait is over 2');

OPTION-3 (@ebidel) - doesn’t work as expected (Error: Navigation Timeout)

// I read this exactly the same as OPTION-1
await page.click('a').then(() => page.waitForNavigation());
console.log('The wait is over 3');

Edit: I like @aslushnikov’s approach better, it makes more sense:

await Promise.all([
  page.waitForNavigation(),
  page.click('a')
]);

@leo020588 another way to think of waitForNavigation is that it waits for the navigation to start and complete.

In your case, the await page.click('a') returns control to the script too late - the navigation has already started.

The handy way to do this is:

await Promise.all([
  page.click('a'),
  page.waitForNavigation()
]);

FWIW, if the

		await page.click('a');

is replaced by the

		page.click('a');

the code seems to work as expected. It may be a race between page.click() and page.waitForNavigation().

@vsemozhetbyt is correct, this is a race condition (or really an async orchestration problem), so you’ll need to have the page.waitForNavigation() “ready” before the click action completes.

FWIW I came across this in the docs and it seems to be the perfect solution, avoiding race conditions and ordering the execution correctly:

// Set up the wait for navigation before clicking the link.
const navigationPromise = page.waitForNavigation();

// Clicking the link will indirectly cause a navigation
await page.click('a.my-link');

// The navigationPromise resolves after navigation has finished
await navigationPromise;

So if I understand correctly one needs to start the waitForNavigation before clicking the item like this:

const promise: Promise<puppeteer.Response> = page.waitForNavigation(waitOptions)
await page.click(selector, clickOptions)
return promise

which is better expressed as

await Promise.all([
  page.waitForNavigation() 
  page.click('a')
]);

as explained earlier.

This is somewhat counter intuitive, as we humans expect to act and then wait for something. Thus it would be nice, if the framework could provide method for this with signature

async clickAndWaitForNavigation(selector: string, clickOptions?: puppeteer.ClickOptions, waitOptions?: puppeteer.NavigationOptions): Promise<puppeteer.Response>

Currently I have this in my project:

async function clickAndWaitForNavigation(page: puppeteer.Page, selector: string, clickOptions?: puppeteer.ClickOptions, waitOptions?: puppeteer.NavigationOptions): Promise<puppeteer.Response> {
  return Promise.all([
    page.waitForNavigation(waitOptions),
    page.click(selector, clickOptions)
  ]).then((value: [puppeteer.Response, void]) => {
    return value[0]
  })
}

Which gives you all the power to click and wait, but none of the details how it should be orchestrated.

I think all projects need this so every project out there have some form of this helper method duplicated, which makes it prime candidate to be included in the library.

@leo020588 please close if that addresses the issue.

You could also do this if it’s easier to reason about the sequence:

await page.click('a').then(() => page.waitForNavigation({waitUntil: 'load'}));
...

This still does not work for me. In my use case, I click on the login button, it starts a spinner, makes a network request, on a successful response, takes the user to a dashboard page.

Should I be using a different approach?

@dowsanjack looks like you’re dealing with a single-page application. The reason the pattern doesn’t work for you is because browser doesn’t do any “navigation” - instead, current page DOM is modified. Thus, using waitForNavigation won’t work.

It’s not always clear if a website is doing navigation or not. A good rule of a thumb is to rely on page state instead, using methods waitForSelector and waitForFunction.

I tried so many things and after about an hour or so this is the only way that it worked for me:

await page.evaluate(() => {
  document.querySelector("your_selector").click()
})
await page.waitForNavigation()

@aslushnikov exposing the ability to wait for 0 network requests that doesn’t depend on navigation would solve this if it’s feasible:

page.waitFor({ waitUntil: 'networkidle0' })

Imagine you click a button and need to wait for images to load

Coming here to say that I was also bit by this sharp edge; maybe a little doc note would be helpful!

So from a “pure promises perspective” :p, I believe my explanation is correct but the reason 3 doesn’t work for the use case is b/c the click actually kicks off a page navigation and you’re waitForNavigation after that happens. So you hang forever.

What @joelgriffith suggested works great. Just go with that 😃

Thats slightly different. In my example you’re awaiting the resolution of the entire promise chain, not just the page.click promise. It’s the same as:

(async() => {
const promise1 = Promise.resolve(1);
const promise2 = Promise.resolve(2);
const val = await promise1.then(() => promise2);  
console.log(val); // logs 2
})();

So in the first example below, “2” will probably log beforeanotherPromiseThatTakesAwhileToResolve resolves:

page.click('a').then(() => anotherPromiseThatTakesAwhileToResolve);
console.log('2');

You can fix that with:

await page.click('a').then(() => anotherPromiseThatTakesAwhileToResolve);
console.log('2');

//or 

page.click('a')
  .then(() => anotherPromiseThatTakesAwhileToResolve)
  .then(() => console.log('2'));

//or use await

page.click('a').then(async() => {
  await anotherPromiseThatTakesAwhileToResolve;
  console.log('2');
});

Here is my function to wait for networkidle2 without waiting for navigation first:

async waitForNetworkIdle(page) {
    const timeoutError = new Error('Timeout while waiting for network idle');

    await new Promise((resolve, reject) => {
        let listener = (event) => {
            if (event.name == 'networkAlmostIdle') {
                page._client.removeListener('Page.lifecycleEvent', listener);
                resolve();
            }
        };

        page._client.on('Page.lifecycleEvent', listener);

        setTimeout(() => {
            page._client.removeListener('Page.lifecycleEvent', listener);
            reject(timeoutError);
        }, 30000);
    });
}

@ebidel But, are they not the same?

await page.click('a');
// and ...
page.click('a').then(() => {});

Both are waiting for page.click() to resolve. I was thinking if it would be a good idea to implement something like:

await page.clickLink('a', {waitForNavigationUntil: 'load'})

Because not waiting for page.click() to resolve, feels someway hackish to me.

@joelgriffith how can I do that? Is what @vsemozhetbyt suggested the right way to go?

page.click('a');
await page.waitForNavigation({waitUntil: 'load'});

In this way can be we sure there will be no race condition?

Version: 1.1.1

As mentioned in the linked issue, this seems not to work for an SPA that doesn’t trigger a load event:

await Promise.all([
  page.waitForNavigation({ waitUntil: 'networkidle0' }),
  link.click(),
]);

Can we perhaps expose something like: page.waitFor({ waitUntil: 'networkidle0' })?

I tried so many things and after about an hour or so this is the only way that it worked for me:

await page.evaluate(() => {
  document.querySelector("your_selector").click()
})
await page.waitForNavigation()

This worked for me as well

It would be nice to have the ClickOptions extend NavigationOptions as to be able to specify a waitUntil for the .click() method (similarly to the .goto() method)

I had the same problem. Here is the solution I went for:

      const sleep = (ms) => {
        return new Promise((resolve) =>
          setTimeout(resolve, ms)
        );
      }

      const requests = new Map();
      requests.set('first');
      page.on('request', (event) => {
        requests.set(event._requestId);
      })

      page.on('requestfinished', event => {
        if (requests.has('first')) {
          requests.delete('first');
        }
        requests.delete(event._requestId);
      })

      await page.click('#SOME_BYTTON');
      
      while (requests.size >1) {
        await sleep(1000);
      }

You can also filter requests by any criteria to make have a more efficient solution. In my case, i am interested in all requests.

await Promise.all([
  page.click('a'),
  page.waitForNavigation()
]);

This still does not work for me. In my use case, I click on the login button, it starts a spinner, makes a network request, on a successful response, takes the user to a dashboard page.

Should I be using a different approach?