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

Cypress's Vite Dev Server exits when bundling cjs on the fly #6654

Closed
7 tasks done
JessicaSachs opened this issue Jan 27, 2022 · 18 comments
Closed
7 tasks done

Cypress's Vite Dev Server exits when bundling cjs on the fly #6654

JessicaSachs opened this issue Jan 27, 2022 · 18 comments
Labels
enhancement New feature or request p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)

Comments

@JessicaSachs
Copy link
Contributor

JessicaSachs commented Jan 27, 2022

Describe the bug

Disclaimer: @patak-dev told me to put this issue in here rather than in Cypress's repo to start. I understand this issue is likely framework-specific. Happy to transfer it whenever necessary.

Vite x Cypress for Component Testing

👋🏻 Hello, I'm the author of @cypress/vite-dev-server, a package I wrote around March 2021 for use with the Cypress Component Test Runner. Until Vitest, using Cypress was practically the only way to test Vue/React components written and bundled with Vite.

The first work-around

In preparation for releasing Cypress 10, we've adopted Vite internally and have been heavy users of Vite over the last 8 months. Early on, we noticed that if Vite discovered a non-ESM module, it would reload the page and this would break Cypress when we used our Vite Dev Server plugin. It would fail to compile the cjs code and crash. We came up with a work-around:

We were able to ask users to use @antfu's vite-plugin-optimize-persist and vite-plugin-package-config plugins to get a working dev server.

Using optimizeDeps kind of worked

Unfortunately, as we're scaling up and integrating into other projects, we've found that those two plugins aren't enough, and we're seeing additional edge cases that are causing instability. We were treating one of the symptoms of Cy + Vite's instability. The Cy dev server wasn't crashing immediately, but there's a deeper issue surrounding HMR and full page reloads. For example, in projects that use unocss, the way the styles are pushing onto the client causes another full page reload.

Reproduction

I'm currently blocked from merging a PR to add Cypress Component Testing for Vitest's UI because of the flake. vitest-dev/vitest#590

Cypress's own monorepo has this issue and a 100% reproducible test case, but working in the Cypress monorepo can be a bit of a challenge. Here are some instructions...

  1. Clone the main Cypress repo on the 10.0-release branch
  2. yarn in the root of the repository
  3. cd packages/app
  4. rm -rf node_modules/.vite && yarn cypress:run:ct --browser chrome --headed --config video=false --spec **/SettingsContainer.cy.tsx

As a last resort...

One of the options we have is to switch to using vite build and statically serving the dist'd files. We don't want to do this for a number of reasons: production parity, loss of HMR, and changes in compilation options (for example, Vitesse's SSG requires <ClientOnly> on vite build, but not on vite dev. We want consistency between build environments).

Call for help

I'd really appreciate guidance and discussion around how to improve the stability of Cypress Component Testing with Vite.

Steps to reproduce

  1. Checkout the PR on Vitest
  2. pnpm install in the root of the project
  3. cd packages/ui
  4. rm -rf ./node_modules/.vite
  5. pnpm ci

Afterwards, you can check the packages/ui/cypress/videos directory to see a video of the failed test.

When you re-run the test, it will pass (some of the time) because the packages have been prebundled. To reproduce, make sure you're running rm -rf ./node_modules/.vite before running pnpm ci

Reproduction

I wasn't able to minimize the reproduction further than the already broken PR in Vitest: vitest-dev/vitest#590

System Info

System:
    OS: macOS 11.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 3.06 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.0 - ~/.nvm/versions/node/v14.17.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.13 - ~/.nvm/versions/node/v14.17.0/bin/npm
  Browsers:
    Chrome Canary: 99.0.4840.0
    Firefox: 95.0
    Safari: 14.1.1
  npmPackages:
    @vitejs/plugin-vue: ^2.0.1 => 2.0.1 
    @vitejs/plugin-vue-jsx: ^1.3.3 => 1.3.3

Used Package Manager

pnpm

Logs

😅 if there were instructions for how to turn --debug logs on from within the JavaScript API, that'd be great.

Validations

@lmiller1990
Copy link

Vite and Cypress, the dream. Let me know if I can do anything to help!

@brian-mann
Copy link

If its helpful - here is a screenshot of filtered network requests. As @JessicaSachs said - when there is a request to the entrypoint, vite serves the file but then immediately sends a "full page refresh" signal so in the network requests you will see the same requests made for the same files. Cypress does not expect a spec to be loaded in, and then immediately unload and load. We could work around this in core, but hopefully there is a better approach we could take directly in vite.

Screenshot 10 -01

@JessicaSachs JessicaSachs changed the title Cypress's Vite Dev Server is unstable. Cypress's Vite Dev Server exits when bundling cjs on the fly Jan 27, 2022
@patak-dev
Copy link
Member

Adding context from Evan, that @JessicaSachs shared when we discussed about it last time:

The reason it needs to reload is because the dep may have cross dependencies with other already optimized deps - so to ensure correctness we have to re-run the whole pre-bundle pass and reload to invalidate imports to the other deps

At that point, @JessicaSachs, you mentioned that you will like to try to make cypress resilient to page reloads. We could check what events are needed for you to be able to implement this. Vite could send a fully-loaded event to the client once everything is done, but what does this mean when dealing with dynamic imports and the user interacting with the page while it is being loaded?

I don't think that full-page reloads is a good DX here, but I don't understand exactly the cross dependencies issue that forces Vite to issue a full reload. The code is here if others want to check:


Maybe there is a way to rework this by accepting different tradeoffs

@patak-dev
Copy link
Member

Maybe there could be an option in Vite to accept that missed dependencies aren't going to be optimized? (or they can be queued for the next full page reload, instead of forcing it). Here you can see a node dependency that isn't yet optimized and so it is registered as missing:

server._registerMissingImport?.(id, resolved, ssr)

Currently, this means that it will return synchronously the path to node_modules, and enqueue and optimizeDeps step that will end up in a full reload because IIUC your page will be pointing to the original dep in node_modules instead of the optimized path that leaves in the cacheDir (by default node_modules/.vite).
Maybe we could continue using the original dep for this run... and have a deps queue in the cache. Next time optimizeDeps is called on server start, it also reads these missing deps and processes them. The first-page load will be slower, but there will not be full reloads needed. Just thinking out loud here.

@JessicaSachs
Copy link
Contributor Author

So... maybe an aside, but I'm genuinely wondering how Vitest handles this issue.

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Jan 27, 2022

More of a meta discussion for another time, but I think that Vitest and Cy's Vite support should be as === as possible so that swapping between headless/headed tests doesn't have any foot-guns WRT module loading, deps, and execution context.

@JessicaSachs
Copy link
Contributor Author

Maybe we could continue using the original dep for this run... and have a deps queue in the cache. Next time optimizeDeps is called on server start, it also reads these missing deps and processes them. The first-page load will be slower, but there will not be full reloads needed. Just thinking out loud here.

So IIUC, this would cause the first page that encounters a dependency to be slower to load, but subsequent pages/reloads would be at Vite-speed. Yes?

@patak-dev
Copy link
Member

So... maybe an aside, but I'm genuinely wondering how Vitest handles this issue.

I think it directly disables deps optimization? vitest-dev/vitest@e379f64#diff-0196708c26b0c02a910c665662539b335e23bf22c303bae9a1b28b3fa1b64cf6
There isn't a browser in the middle requesting thousands of files for lodash, so I think it just gets in the way there. You may need to ask Anthony about it.

@patak-dev
Copy link
Member

patak-dev commented Jan 27, 2022

Maybe we could continue using the original dep for this run... and have a deps queue in the cache. Next time optimizeDeps is called on server start, it also reads these missing deps and processes them. The first-page load will be slower, but there will not be full reloads needed. Just thinking out loud here.

So IIUC, this would cause the first page that encounters a dependency to be slower to load, but subsequent pages/reloads would be at Vite-speed. Yes?

Yes, and I don't know if the first page will really be that much slower, or it may be even faster? Because you are comparing loading with a few deps unoptimized against loading with possible several full page reloads in the middle so the last reload has all of the deps optimized? I wonder if the later isn't slower, and this shouldn't be an option but the default

@JessicaSachs
Copy link
Contributor Author

Maybe we could continue using the original dep for this run... and have a deps queue in the cache. Next time optimizeDeps is called on server start, it also reads these missing deps and processes them. The first-page load will be slower, but there will not be full reloads needed. Just thinking out loud here.

KK. Gonna prototype this right now.

@JessicaSachs
Copy link
Contributor Author

TL;DR so far

👋🏻 It's been a big struggle to get the environment up. That's what I spent most of my day doing -- working toward the goal of being able to prototype the suggestions that @patak-dev and I spoke about.

What I need is a test fixture or project/page that has many optimizeDeps cache misses and ends up responding with a lot of 408s. Ultimately, triggering this block of code:

if (
server._pendingReload &&
// always allow vite client requests so that it can trigger page reload
!req.url?.startsWith(CLIENT_PUBLIC_PATH) &&
!req.url?.includes('vite/dist/client')
) {
try {
// missing dep pending reload, hold request until reload happens
await Promise.race([
server._pendingReload,
// If the refresh has not happened after timeout, Vite considers
// something unexpected has happened. In this case, Vite
// returns an empty response that will error.
new Promise((_, reject) =>
setTimeout(reject, NEW_DEPENDENCY_BUILD_TIMEOUT)
)
])
} catch {
// Don't do anything if response has already been sent
if (!res.writableEnded) {
// status code request timeout
res.statusCode = 408
res.end(
`<h1>[vite] Something unexpected happened while optimizing "${req.url}"<h1>` +
`<p>The current page should have reloaded by now</p>`
)
}
return
}

Next steps

I will work on this tomorrow to try and simplify the reproduction further and move it out of Cypress-land. If someone has a simplified project I can use that triggers the failure state I'm talking about, that would be super helpful.

For a better demonstration of the symptoms we're seeing, I've attached a video. It's split into two parts. Essentially we're getting a ton of 408's, reloads, and marvelous failures.

The initial failure. This has like 4 reloads.

cy-failing-optimize-deps-first.mov

Success when reloading

cy-failing-optimize-deps-second.mov

This only really happens for the first build of the first page for a user's particular machine when the project is large enough

So, basically, most CI runs.

Project Info More details about the project under here, including information for where to find the Vite configs and plugin code written.

The Vite config used for the page in the video is here in the Cypress App -- the web frontend, but most of the config is within the frontend-shared package's Vite config.

Package Arch

The Cypress + Vite Dev Server code lives here. The architecture is stupidly simple, we create a Vite dev server from the JavaScript API and serve each spec file as a "page" in Vite.

Everything functions fine outside of the full-page refresh behavior for the first spec.

Thanks for the help so far. I'm around to Discord for sync comms. My plan is to keep this issue updated at least daily because solving this is really critical to Cy and Cy's upcoming release.

Cheers, Jess

@JessicaSachs
Copy link
Contributor Author

@patak-dev and I spent a large portion of the morning pairing and working on an isolated reproduction within the Optimize Missing Deps playground. I learned how to develop in Vite, run the tests, and we were able to begin spiking on a possible fix.

We're intending to add a mode for blocking requests that require dependency resolution instead of enqueueing them. For Cypress specifically we believe it will significantly improve the performance.

We want to conditionally make this line blocking:

https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/resolve.ts#L599

Such that network requests for unoptimized modules do not queue and push a refresh when they are optimized, but instead synchronously perform the work and do not reload.

Next:
I'm in the middle of the spike and now that I have a reproduction and feel more comfortable developing + running the Vite test suite, I'll continue spiking and will post additional updates as I have them.

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Jan 28, 2022

Final update for now.

@patak-dev asked me to keep this issue open as a placeholder to discuss an architecture that does not have the below flaws and resolves all dependencies AND transient dependencies such that incremental reloads are not necessary.

Update on the previous approach:

  1. We've found the fatal flaw in the above strategy which is that when resolving dependencies, shared transient ones may resolve to a different version. (This is what Evan told me in October, lol, I just didn't fully understand how limited we were until now.)
  2. If we were to push up a new version for a transitive dep, we would have multiple versions of a dependency running within the same page context.
  3. Having multiple versions of certain packages running at the same time can cause very critical application errors (e.g. React can't run multiple versions on one page).
  4. Additionally, without doing a full-page reload, any side effects would not be cleaned up.

Therefor, unless we know all of the transient dependencies for all entries, we cannot safely prevent a full page reload.

Cypress's decision is to use vite build + vite serve in CI. This will reliably solve the issue for fresh machines that have never run the user's application before.

For local development, we will continue to use vite dev to benefit from all of the debug and development utilities that plugins like WindiCSS provide to users.

The surface area of risk for local development is restricted to users who have:

  1. Never run their application before
  2. Never run their tests before
  3. Deleted their cacheDir

In all of these local-only edge cases where a user has a completely clean cache, a simple Cmd + R will fix this issue.

Timeline

  1. After the Cypress v10 launch, we will improve the DX for users developing locally with cypress open.
  2. Immediately (today), we will make @cypress/vite-dev-server utilize vite build + vite serve when running Cypress in run mode (AKA, CI). This will fix the majority of the instability for all versions of Cypress that can run Component Testing v7+.

Future

If the Vite team ever has a way to turn off automatic reloads, that would allow us to create a more consistent experience between dev and prod.

@patak-dev
Copy link
Member

Thanks for the write up, lets keep this open. We may find ways to at least avoid some full reloads later.

A detail about the new CI approach. By vite serve I think you mean vite preview . Take into account the preview server is only for debugging locally, it may work for your CI case but it may be better to serve the build with a diff production ready server

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Jan 28, 2022

Thanks for the write up, lets keep this open. We may find ways to at least avoid some full reloads later.

A detail about the new CI approach. By vite serve I think you mean vite preview . Take into account the preview server is only for debugging locally, it may work for your CI case but it may be better to serve the build with a diff production ready server

Ah sorry I was thinking about vitepress's serve command. Yes. We'll use an express server, then. Cy already has one up.

@patak-dev
Copy link
Member

patak-dev commented Feb 4, 2022

@francisu
Copy link

francisu commented Feb 8, 2022

@JessicaSachs I read through this with interest as I think I'm hitting this same problem when using Vite and cypress run-ct. When running the first time, vite start has to figure out the dependencies, and this is triggered on the first test execution.

I was curious about your use of vite build and vite preview in CI. Does this mean you are able to execute the component tests from a vite preview environment without using the dev server? If so, can you point me to how this works (I looked through the docs and could not find anything).

My strategy for now is just to use run-ct on a single test case (which will likely fail -- I will ignore the failure), but this after this point, vite will have cached everything so the real run-ct will work.

A better solution is welcome.

@patak-dev
Copy link
Member

The cold start experience has been improved after #6758, and related PRs. These changes will be part of 2.9.
Discussed with @JessicaSachs that confirmed that the 2.9 beta is working better for Cypress. Let's close this issue, please open new ones if needed.

@patak-dev patak-dev added enhancement New feature or request p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) and removed pending triage labels Mar 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
Archived in project
Development

No branches or pull requests

5 participants