Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: onBeforeLoad is not called when overwriting cy.visit #5633

Closed
nengelberth-aptera opened this issue Nov 7, 2019 · 8 comments
Closed

Comments

@nengelberth-aptera
Copy link

nengelberth-aptera commented Nov 7, 2019

Steps to reproduce: (app code and test code)

  1. Overwrite cy.visit to call the originalFunction and then do some extra waiting, such as:
function waitForPageToLoad() {
    cy.get('html').should('have.class', 'nprogress-busy');
    cy.get('html').should('not.have.class', 'nprogress-busy');
}

Cypress.Commands.overwrite('visit', (originalFn, url, options) => {
    originalFn(url, options);
    cy.location('href').should('contain', url);
    waitForPageToLoad();
})
  1. Pass in an onBeforeLoad option -OR- define an onBeforeLoad option within the overwrite -- e.g. add options.onBeforeLoad = (win) => { console.log('onBeforeLoad executed') } to the Cypress.Commands.overwrite code above

Current behavior:

onBeforeLoad does not execute in either case.

Desired behavior:

onBeforeLoad executes.

This appears to be a regression of #2196, but the workaround specified by henrikq-vipps in that issue no longer works either.

Versions

Cypress version: 3.4.1
Operating System: Windows 10.0.18362.418
Browser: Chrome 78, Electron 61

@nengelberth-aptera
Copy link
Author

nengelberth-aptera commented Nov 7, 2019

As a workaround, I seem to be able to define a new custom command in basically the exact same way as I am trying to overwrite visit, and in that case visit's onBeforeLoad does get called.

This works:

Cypress.Commands.add('visitAndWaitForLoad', (url, options = {}) => {
    options.onBeforeLoad = (win) => {
        console.log('calling onBeforeLoad');
    }
    options.onLoad = (win) => {
        console.log('calling onLoad');
    }
    console.log(`calling cy.visit with options ${JSON.stringify(options, null, 4)}`)
    cy.visit(url, options);
    cy.location('href').should('contain', url);
    waitForPageToLoad();
})

cy.visitAndWaitForLoad('http://my.url.example.com');

@jennifer-shehane
Copy link
Member

@nengelberth-aptera I see that you are using an older version of Cypress. Before I dive in to completely recreate the issue, could you update to the current version of Cypress and let me know if this is still happening for you? Your issue may have already been fixed. Thanks!

@jennifer-shehane jennifer-shehane added the stage: needs information Not enough info to reproduce the issue label Nov 7, 2019
@nengelberth-aptera
Copy link
Author

nengelberth-aptera commented Nov 8, 2019

With Cypress 3.6, I'm not seeing either onBeforeLoad or onLoad events firing.

While I was double checking my configuration against the documentation while verifying this, I noticed that there was a small discrepancy between my code and what the documentation recommends...

My code is:

Cypress.Commands.overwrite('visit', (originalFn, url, options) => {
    options = {
        onBeforeLoad: (win) => { console.log('onBeforeLoad executed') }, onLoad: (win) => { console.log('onLoad executed') }
    }
    console.log('calling cy.visit');
    originalFn(url, options);
    cy.location('href').should('contain', url);
    waitForPageToLoad();
})

cy.visit('/');

Whereas the end of the documentation for overwriting a function includes:

  // originalFn is the existing `visit` command that you need to call
  // and it will receive whatever you pass in here.
  //
  // make sure to add a return here!
  return originalFn(url, options)

And if I do add a return on the line with the originalFn call, then both onBeforeLoad and onLoad are firing. In my specific case, the reason I want to override the visit function is specifically to add those two lines after calling the base function, to cause Cypress to wait for my application to finish navigating before it continues. So I guess this may be less a bug and more of a "how do I do what I want to do" type issue... but I've also kind of worked around it by changing to a custom command rather than the override anyway, so this can probably be closed.

@flotwig
Copy link
Contributor

flotwig commented Nov 8, 2019

Sounds related to #4973 and #5595

@jennifer-shehane
Copy link
Member

Since this issue hasn't had activity in a while, we'll close the issue until we can confirm this is still happening. Please comment if there is new information to provide concerning the original issue and we'd be happy to reopen.

@jennifer-shehane jennifer-shehane removed the stage: needs information Not enough info to reproduce the issue label Oct 20, 2020
@cvolant
Copy link

cvolant commented Jan 14, 2021

I experience this issue on 6.2.1 as well, on my repo, but not all the time... Sometimes it works, sometimes it does not.
I tried to build a minimal reproduction starting from the official Cypress example, but I did not succeed to reproduce the bug.
The page load speed might be in cause: the Cypress example page is really fast, and mine is not since it is a local development deployment.

Another weird hypothesis: may the service worker cause some troubles? I have one running on my page.

@cvolant
Copy link

cvolant commented Jan 15, 2021

I tried a workaround with a cy.on('window:before:load', () => { ... }) before the originalVisit call, but I have the same behavior: it works well with the simple Cypress example, but in my repo, it works only sometimes, which is really annoying.

So, the issue comes from the common code on top of which onBeforeLoad and cy.on('window:before:load', ...) are both built.

@jennifer-shehane
Copy link
Member

@cvolant Service workers have been known to cause issues, I'm not sure if it's involved in your case. There are some workarounds for unregistering service workers in Cypress in this thread #702 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants