-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat: support vite v5 #29518
feat: support vite v5 #29518
Conversation
6 flaky tests on run #55508 ↗︎
Details:
scaffold-component-testing.cy.ts • 1 flaky test • launchpad-e2e
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-chrome:beta
commands/net_stubbing.cy.ts • 3 flaky tests • 5x-driver-webkit
|
025d554
to
8a913dd
Compare
2777140
to
f9cdf77
Compare
… provided [run ci]
f9cdf77
to
ac6e96d
Compare
438545f
to
43615a6
Compare
|
||
if (vite.version) { | ||
majorVersion = semverMajor(vite.version) | ||
debug(`Found vite version v${majorVersion}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only added for debugging purposes, which becomes more important as we add versions
@@ -56,10 +56,25 @@ We then merge the sourced config with the user's vite config, and layer on our o | |||
| <= v2 | <= v9 | | |||
| >= v3 | >= v10 | | |||
|
|||
#### `devServerPublicPathRoute` for Vite v5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems really weird to me that we don't have specific bundler docs for CT. Shouldn't this be documented as an option in the component config here? https://docs.cypress.io/guides/references/configuration#component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think so, but I am not sure why it isn't documented. Was it something we wanted as an option but not to be widely surfaced or was the documentation just missed?
We've been trying to get a component test running after upgrading Vite to 5.0 (node 20.13). The only test failures we're having are from selectFile, like this: |
|
||
const viteConfig = await createViteDevServerConfig(viteDevServerConfig, vite) | ||
const viteConfig = await createViteDevServerConfig(viteDevServerConfig, version === 5 ? vite5 : (vite4 as unknown as Vite)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than flexing this everywhere. You could potentially make MAJOR_VERSIONS an array of objects like:
[
{
version: 4,
vite: vite4
},
{
version: 5,
vite: vite5
}
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue right now is we implicitly support versions below 4 as well, including 2 and 3 since we have not deprecated them. But for a unit test this probably isn't a big deal so I'll get it updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 029f558
@jdenekat are you able to open an issue with a reproduction? |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Adds support for
vite@5
. To add support, the scaffolding config needs to support vite v4 as well as@cypress/vite-dev-server
. The current architecture for@cypress/vite-dev-server
is to roll forward and add support. Since significant changes aren't required and haven't been in the past, this works.Note: if users are referencing the absolute public path in their config, they will likely need to set
devServerPublicPathRoute
to the expected destination of their public path. Since this default is shipped with Cypress, we cannot actively change it. Since we don't have a dedicated section to CT bundlers in our documentation, I have added it in the README of the package.System tests are also added/updated. Unit/system tests are added for vite 5, and older system tests are updated to be on at least vite 4. This is NOT a breaking change, but gets us more in alignment with our unofficially supported last two versions.
Since I needed to change some of the path resolution behavior within the dev server, I added a new unit test,
initCypressTests
, since none existed and felt I needed to verify the new behavior. which additionally documents what is expected.After this PR goes in I will work on getting the other packages updated to latest vite in the monorepo using vite@4 currently.
Steps to test
Run the added unit/system tests as well as test the build binary against a vite 5 project
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?