Large increase in random CI failures using Cypress after upgrading to react-router v7
I'm using React Router as a...
library
Reproduction
https://github.com/almilo/react-router-7-ci-bug
There are no reproduction steps. Just an observation. Looking at video of failures, and corresponding code, it looks like a race condition during clicking. The code failing looks similar to this (in various places):
cy.contains('Text').should('exist')
cy.get('.class a').first().click()
The link is on the same page as the text. What happens is that Cypress executes the click, but react-router does not transition to the new path, like the click never happened.
I'm posting this mainly to see if I'm not alone with this.
System Info
react-router v7.1.4
cypress v14.0.1
Used Package Manager
npm
Expected Behavior
Not seeing random test failures.
Actual Behavior
Seeing random test failures.
Does it transition to the new page if you add some wait time to the test?
We use startTransition between navigations, so this introduces potentially async behavior to rendering. You may either need to wait a tick for the render to finish or have some way to wait for the render to finish before checking the DOM state. That may depend on how you have the test renderer hooked up and what libraries you use to do that.
It's hard to tell as the failures are very random across the whole test suite. I use cypress, and it has some waiting mechanisms built-in, so that there's no need to manually wait. Though they seem to fail occasionally when dealing with react-router v7. I'll keep investigating though.
I think the issue is that because routing updates are happening in a transition, Cypress or automated testing tools may interact with the UI too quickly before any routing side effects have completed. For example, the below test ensures that the page state and URL stay in the sync. This test now fails unless I ensure that the UI is up to date with the URL before the 'next range' button is clicked.
cy.findByRole('button', { name: /previous range/i }).click();
cy.url().should('include', 'date=2021-04-12');
cy.findByRole('textbox', { name: /date range/i }).should('have.value', '12/04/2021'); // Now fails without this line
cy.findByRole('button', { name: /next range/i }).click();
cy.url().should('include', 'date=2021-04-13');
There are other cases though where the routing state is not visible in the UI, for example a useEffect that updates some state when the page location changes.
Is there a way we can detect if the navigation transition is still in progress, to block any further navigation, or ensure the test waits?
Also ran into this. It's related to v7_startTransition part of the upgrade it seems.
PS: managed to get around this by avoiding cy.url() checks at all, but then after actually migrating to v7 a bunch of other issues pop up, like pages not loading in Cypress at all, so most tests fail.
We then realized that the problem was how we build routes: we create them based on some async data dynamically & it seems react-router v7 now doesn't handle this the same as v6 did. To "fix" it we now block rendering until all our routes are stable and then render the Router.
I am working on a project where we use Playwright instead of Cypress and, after upgrading to react-router 7, we started too observing a lot of flakiness in browser tests that were pretty stable before. Finding out what the cause was has been really difficult and, after finding this issue and reverting the upgrade, I can confirm too that there is a flaky behaviour during navigation in version 7. In our case, it seemed like only randomly (much more in CI than locally), some navigation transitions would just not happen and we could not observe any errors in the Playwright tests traces. The tests would fail after 10 seconds as the navigation just didn't seem to happen. It can definitely be noticeable only in browser test frameworks due to the speed at which they click, but it is definitely not a consistent behaviour and a clear showstopper for us to upgrade.
Thanks for the great work and the support!
Our CI has had a lot more flaky tests as well. The timing of navigation has definitely changed, probably due to the use of transitions, at least in part. I've had to knock most if it back by adding assertions to tests to confirm navigation to the new page is complete before proceeding with the test (checking the page title, asserting the clicked tab is now selected, etc).
Asserting the url has changed does NOT work. In my tests, the URL often updates now before the page content does, which was not the case in v6.
To align with our new Open Governance model, we are now requiring that all issues have a minimal and runnable reproduction. To that end, we're doing some housekeeping in the repo to clean up existing issues that do not have a valid reproduction. This should get us down to a more manageable number of issues and allow us to be more responsive to existing and newly filed issues.
We're using a Github actions script to identify issues without a reproduction by looking for a StackBlitz, CodeSandbox, or Github link in the issue body. This won't be perfect, so if this issue has a reproduction on another platform, please comment back on here and we can re-open the issue. Similarly, if there's a reproduction buried in a comment, please move the link into the description and comment back. Please tag @brophdawg11 or @brookslybrand in your comment so we get a notification as well π.
If this issue did not have a reproduction but is still valid, or if you wish to start with a fresh issue, please create a new issue with a fresh reproduction against v7 and link to this issue in the new description.
If this a feature request, please open a new Proposal Discussion in React Router, and if it gets enough community support it can be considered for implementation.
If you have any questions you can always reach out on Discord. Thanks again for providing feedback and helping us make React Router even better!
Also running into this. Our Cypress suite started to fail immediately after upgrading React Router to V7. Maybe due to the use of async transitions, Cypress is using old text in the UI when asserting.
Manually I cannot reproduce the issue. Anyone got a workaround?
@francisco-polaco I've downgraded to v6 and this problem went away.
@francisco-polaco I've downgraded to v6 and this problem went away.
@pjg We're planning on moving to V7 so we can move away from CRA. Any suggestion?
@francisco-polaco producing a reproducible scenario (perhaps with cpu throttling?) is probably the most immediate need to move fixing this issue forward.
This issue only happens on the CI server. We have transitioned from nextjs to vite and react router, however even a simple test that just clicks a Link from react-router fails. No url change or ui change happens when the Link is clicked.
A reproducible example can be made, but notice: the example must run on the app in GHA for the error to happen. When the app runs in this environment, it doesnβt react to link clicks.
Locally they run just fine, but this is not sufficient.
To align with our new Open Governance model, we are now requiring that all issues have a minimal and runnable reproduction. To that end, we're doing some housekeeping in the repo to clean up existing issues that do not have a valid reproduction. This should get us down to a more manageable number of issues and allow us to be more responsive to existing and newly filed issues.
We're using a Github actions script to identify issues without a reproduction by looking for a StackBlitz, CodeSandbox, or Github link in the issue body. This won't be perfect, so if this issue has a reproduction on another platform, please comment back on here and we can re-open the issue. Similarly, if there's a reproduction buried in a comment, please move the link into the description and comment back. Please tag
@brophdawg11or@brookslybrandin your comment so we get a notification as well π.If this issue did not have a reproduction but is still valid, or if you wish to start with a fresh issue, please create a new issue with a fresh reproduction against v7 and link to this issue in the new description.
If this a feature request, please open a new Proposal Discussion in React Router, and if it gets enough community support it can be considered for implementation.
If you have any questions you can always reach out on Discord. Thanks again for providing feedback and helping us make React Router even better!
@brophdawg11, @brookslybrand as I am not the original reporter, I cannot modify the description to add the link to a repository with a reproduction and neither can I re-open the issue. I did thouhg create this repository here: https://github.com/almilo/react-router-7-ci-bug/actions/workflows/playwright.yml with some Playwright tests exposing the potential behaviour causing the CI failures.
In the 3 workflow executions above (1 test repeat, 3 test repeats and 10 test repeats), one can see that one of the tests fails consistently while locally on a MacBook pro, the test passes consistently even with 10 repetitions. I am not 100% sure this is an obvious bug in react-router, but IMO worth having a look at it and providing guidance as this is a blocker for us for migration to v7 and it seems that other projects (even with other testing frameworks like Cypress) are seeing similar issues.
Thanks a lot in advance!
@brophdawg11, @brookslybrand β Can this issue then, by any chance, be reopened?
I'll re-open this but I'll be honest if this is not reproducible by a user in a live app, not reproducible when running playwright locally, and only happens in CI - it isn't going to be high on the priority list for us to go down a potential rabbit hole. There's just to much other more pressing things to work on at the moment. If anyone wants to try to dig deeper into this that would be greatly appreciated - if we can find some potential causes we'd be happy to look into them.
If this can be reproduced in a live app - please provide an updated reproduction and that will likely be much easier to triage on our end.
not reproducible when running playwright locally
I can confirm that it is possible. I started the migration on our end and had to abandon it due to this issue. If u have a hard time reproducing it locally due to a quick computer u can also try throttling the cpu to make it behave more similar to CI. Ex
// https://chromedevtools.github.io/devtools-protocol/tot/Emulation/#method-setCPUThrottlingRate
const context = page.context();
const cdpSession = await context.newCDPSession(page);
// rate 4-6x works well for Mac cpu
await cdpSession.send("Emulation.setCPUThrottlingRate", { rate });
@brophdawg11 looking at the tests, I don't think that they are showing the behaviour that we are chasing. I will need to re-try the migration and evaluate the failures in CI with a bit more detail to isolate the problem properly and hopefully make it reproducible.
Thanks a lot for re-opening the issue nonetheless.
We're also having this issue. When using react router v7, a lot of Cypress tests started failing, but only on CI. All of these tests rely on url changes and assertions on those changes. If this is caused by startTransition, then it's not specific to react-router, apart from just using the API, which means that a workaround won't most likely come from here.
We previously tried react-router dev with our Playwright tests and they would fail in CI.
I changed our setup a bit by building the application first before serving it with vite preview and our Playwright tests now pass in CI.
The reason it affected us so much is because we rely heavily on url params and queries for app functionality which always lagged behind in the useParams and useSearchParams hooks with startTransition
The reason it affected us so much is because we rely heavily on url params and queries for app functionality which always lagged behind in the
useParamsanduseSearchParamshooks withstartTransition
This is exactly our use case. We're now resorting to what was suggested here:
You may either need to wait a tick for the render to finish or have some way to wait for the render to finish before checking the DOM state.
We defer the url assertion with something like lodash defer or a setImmediate.
That might be fine in the tests, even if I don't like it π But this changed behaviour actually breaks some of our application logic for real users as well
We just had to revert our react-router upgrade for the second time due to this issue. It is producing too much instability in our application, even after dozens of hours invested in changing tests to work around it. We are now looking into alternative libraries to use instead of react router (probably wouter).
Our biggest problem stems from two components on the page at the same time (two separate page controlling components), both of which are trying to manage query strings. The component from the old page keeps clobbering the query strings set by the new page. This was not an issue in v6.
We're also spending a lot of time fixing flaky tests and it feels like a moving target at the moment. This wasn't happening with v6.
As stated in this comment, if anyone can provide a minimal reproduction of an issue in an application - we'd be happy to take a look.
It's difficult to create an example and justify what's happening, but I've had an open PR for two months migrating to React Router v7 and fixing failing tests with Cypress.
Perhaps if this repository implemented a quick E2E test using Cypress for a practical and lightweight example, comparing versions 6, 7 and ..., it would be great and we could identify this issue. Alternatively, a feat flag disabling startTransition in version 7.
We were having similar issues in our end to end tests after upgrading to v7_startTransition.
Turns out it was a race condition between our search param navigation and clicking on a link navigation.
We had a debounce on our search param navigation which made it more obvious locally. We just added a check to only set the search params if the value had actually changed, which prevented the extra navigation and the race condition.
useEffect(() => {
const handler = setTimeout(() => {
+ const newParams = new URLSearchParams(searchParams);
if (inputValue) {
- searchParams.set(name, inputValue);
+ newParams.set(name, inputValue);
} else {
- searchParams.delete(name);
+ newParams.delete(name);
}
-
- setSearchParams(searchParams);
+ if (newParams.toString() !== searchParams.toString()) {
+ setSearchParams(newParams);
+ }
}, debounceDelay);
Edit: We also switched to vite preview as was suggested above which helped resolve a bunch of [vite] connecting... errors we were getting if we used more than a few workers.