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

chore: update to Vite 4 and Rollup 3 #25101

Merged
merged 3 commits into from
Dec 13, 2022
Merged

Conversation

lmiller1990
Copy link
Contributor

@lmiller1990 lmiller1990 commented Dec 11, 2022

Vite 4 migration: https://vitejs.dev/guide/migration.html
Rollup 3 migration: https://rollupjs.org/guide/en/#migration

None of the breaking changes seems to impact us. We only use a very simple Rollup configuration to bundle things in npm/*, and we don't do anything too strange with Vite, either. Great!

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 11, 2022

Thanks for taking the time to open a PR!

@lmiller1990 lmiller1990 changed the title chore: try move to rollup 3 and vite 4 chore: update to Vite 4 and Rollup 3 Dec 11, 2022
@lmiller1990 lmiller1990 marked this pull request as ready for review December 11, 2022 11:34
@lmiller1990 lmiller1990 requested a review from a team December 11, 2022 11:34
@@ -11,14 +11,14 @@ declare namespace Cypress {
}

type ReferenceAlias = {
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 sure why these changed - all I did was run yarn. The changes are more in line with the style of the rest of the file, though, so I think it's okay.

@marktnoonan
Copy link
Contributor

Looks good, though I'd like to check the binary before we merge, could you run those jobs for this branch? I'd want to check live reloading in open mode to make sure dependency changes didn't introduce any issues.

Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

Looks good!

@lmiller1990
Copy link
Contributor Author

@marktnoonan this is just for our internal dependencies, we use the user's Vite version - we don't ship Vite or Rollup in production, neither of these dependencies is present in the binary.

Copy link
Contributor

@mike-plummer mike-plummer left a comment

Choose a reason for hiding this comment

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

Looks good, but just to cross the t's and dot the i's there have been a couple point releases in the last day:
vite@4.0.1
rollup@3.7.3
Worth bumping?

@lmiller1990
Copy link
Contributor Author

lmiller1990 commented Dec 12, 2022

@marktnoonan neither rollup or vite is in the binary, no need - can verify by doing cd /Users/<users>/Library/Caches/Cypress/11.0.1/Cypress.app/Contents/Resources/app and observing neither is there.

@mike-plummer we can, I'm not sure why renovate isn't doing this for us 🤔

Doing that now.

@lmiller1990 lmiller1990 merged commit 09d0879 into develop Dec 13, 2022
@lmiller1990 lmiller1990 deleted the lmiller/vite-4-and-rollup-3 branch December 13, 2022 00:35
flotwig pushed a commit that referenced this pull request Dec 13, 2022
* chore: try move to rollup 3 and vite 4

* bump to latest versions
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

Successfully merging this pull request may close these issues.

5 participants