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

breaking: remove support for vite 2 and 3 from @cypress/vite-dev-server #30489

Merged
merged 27 commits into from
Nov 3, 2024

Conversation

AtofStryker
Copy link
Contributor

@AtofStryker AtofStryker commented Oct 29, 2024

Additional details

Removes support for vite version 2 and 3 from @cypress/vite-dev-server. This doesn't have a tremendous effect on the @cypress/vite-dev-server package as far as actually supporting versions 2 and 3. It likely will still work, Cypress just no longer guarantees it and we will not detect the dependency when scaffolding CT. Therefore, all vite 2/3 tests are removed, or tests that are using older versions of vite/older dependencies are now updated.

Steps to test

How has the user experience changed?

PR Tasks

@AtofStryker AtofStryker force-pushed the breaking/remove_wds3_from_cypress_webpack_dev_server branch from dca2184 to c40d117 Compare October 29, 2024 21:38
@AtofStryker AtofStryker force-pushed the breaking/remove_vite_3_and_under_support branch from a2ee392 to 7e7a378 Compare October 30, 2024 00:17
@AtofStryker AtofStryker changed the title Breaking/remove vite 3 and under support breaking: remove support for vite 2 and 3 from @cypress/vite-dev-server Oct 30, 2024
Copy link

cypress bot commented Oct 30, 2024

cypress    Run #58034

Run Properties:  status check passed Passed #58034  •  git commit 149f5923b5: fix failing tests in vite-dev-server by fixing source map references (expected s...
Project cypress
Branch Review breaking/remove_vite_3_and_under_support
Run status status check passed Passed #58034
Run duration 21m 07s
Commit git commit 149f5923b5: fix failing tests in vite-dev-server by fixing source map references (expected s...
Committer AtofStryker
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 5
Tests that did not run due to a developer annotating a test with .skip  Pending 1327
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 29369
View all changes introduced in this branch ↗︎
UI Coverage  46.24%
  Untested elements 188  
  Tested elements 166  
Accessibility  92.55%
  Failed rules  3 critical   8 serious   2 moderate   2 minor
  Failed elements 899  

@AtofStryker AtofStryker added the npm: @cypress/vite-dev-server @cypress/vite-dev-server package issues label Oct 30, 2024
@AtofStryker AtofStryker self-assigned this Oct 30, 2024
@AtofStryker AtofStryker added the type: breaking change Requires a new major release version label Oct 30, 2024
Base automatically changed from breaking/remove_wds3_from_cypress_webpack_dev_server to release/14.0.0 October 30, 2024 22:37
@AtofStryker AtofStryker force-pushed the breaking/remove_vite_3_and_under_support branch from 93bffde to 149f592 Compare October 30, 2024 22:41
cy.startAppServer('component')

cy.visitApp()
cy.specsPageIsVisible()
cy.contains('App.cy.jsx').click()
cy.waitForSpecToFinish()
cy.get('.passed > .num').should('contain', 1)
// no support file means there is no mount function registered, so all tests should fail
cy.get('.failed > .num').should('contain', 2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly not sure why this ever passed initially, but I would argue this is the correct behavior since there is no mount function.

@@ -112,6 +112,11 @@ describe('scaffolding component testing', {
cy.findByTestId('dependency-react-dom').within(() => {
cy.get('[aria-label="installed"]').should('exist')
})

// now clean up the state that we mutated
cy.withCtx(async (ctx) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a problem in CI but when rerunning this spec locally it fails due to polluted state in the project under test's dependencies

expect(actual.bundler).to.eq('vite')
})

it(`React with Vite using pre-release version`, async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we support installing a pre release if we support that major version, which is kind of weird because we don't know if we will be compatible with the next major version, so we shouldn't actually support this OOTB and instead let users skip the install step to try the test harness

@AtofStryker AtofStryker merged commit dfd296e into release/14.0.0 Nov 3, 2024
83 of 85 checks passed
@AtofStryker AtofStryker deleted the breaking/remove_vite_3_and_under_support branch November 3, 2024 19:57
@jennifer-shehane jennifer-shehane mentioned this pull request Dec 3, 2024
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm: @cypress/vite-dev-server @cypress/vite-dev-server package issues type: breaking change Requires a new major release version
Projects
None yet
3 participants