-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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 vite to version 5 inside the monorepo #29617
Conversation
Passing run #55778 ↗︎Details:
Review all test suite changes for PR #29617 ↗︎ |
9f064c0
to
c3f4c77
Compare
…5, vite related vue plugins to version 5, and svg plugin to v5 update vitejs react to v5 updated vite plugins updates unplugin-vue-components and unplugin-icons for vite remove vite-plugin-copmponents as it is the same thing as unplugin-vue-components update vue-i18n from beta package to released v9 get unplugin-vue-i18n to latest add @babel/types to no rewrite [run ci] fix typing errors [run ci]
c3f4c77
to
c3a0918
Compare
@@ -65,7 +65,6 @@ describe('App Top Nav Workflows', () => { | |||
it('shows the current browser in the top nav browser list button', () => { | |||
cy.findByTestId('top-nav-active-browser-icon') | |||
.should('have.attr', 'src') | |||
.and('contain', 'chrome') |
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.
no longer applicable since the src doesn't contain chrome when transformed by updated vite-svg-loader
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.
Why was it checking for chrome previously? Is there something else we should be checking for instead?
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.
because chrome in this case is the active browser. the src
no longer contains chrome so we can't assert on it, but the below assertion I feel should be enough
@@ -19,6 +19,7 @@ export const VueI18n = createI18n() | |||
export function createI18n (opts = {}) { | |||
return _createI18n<MessageSchema, 'en-US'>({ | |||
locale: 'en-US', | |||
// @ts-expect-error |
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.
has to do with the key for the message value pair can be any string, where as the type is expecting en-us
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.
oof, this is a huge shortcoming of that library when it comes to typescript... how about we coerce the type instead? e.g.
/**
* precompiled messages from unplugin-vue-i18n do not include explicit keys derived from
* the filenames of the raw message sources, so it must be coerced.
*/
messages: compiledMessages as { 'en-US': MessageSchema },
[0m[31m[1m>[22m[39m[90m 1 |[39m describe([32m'fail'[39m[33m,[39m [33m-[39m[33m>[39m)[0m | ||
[0m [90m |[39m [31m[1m^[22m[39m[0m | ||
[0m [90m 2 |[39m[0m | ||
[0m[31m[1m>[22m[39m[90m 1 |[39m describe([32m'fail'[39m[33m,[39m [33m-[39m[33m>[39m) |
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.
percy changes look as expected. There is one snapshot that is showing double chevrons, but when I run the test locally I only see the 1 chevron as expected so I am not sure what is happening there. |
@@ -489,7 +489,7 @@ exports['e2e record passing passes 2'] = [ | |||
'originalFile': 'cypress/e2e/record_uncaught.cy.js', | |||
'relativeFile': 'cypress/e2e/record_uncaught.cy.js', | |||
'absoluteFile': '/foo/bar/.projects/e2e/cypress/e2e/record_uncaught.cy.js', | |||
'frame': '> 1 | throw new Error(\'instantly fails\')\n | ^\n 2 | ', | |||
'frame': '> 1 | throw new Error(\'instantly fails\')\n | ^\n 2 |', |
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.
snapshot update likely due to the babel code frame updates (change was a space).
|
||
"@babel/[email protected]", "@babel/core@^7.0.0", "@babel/core@^7.17.9", "@babel/core@^7.18.9", "@babel/core@^7.20.7", "@babel/core@^7.21.3", "@babel/core@^7.21.4", "@babel/core@^7.22.9": | ||
"@babel/[email protected]": |
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.
We used to resolve down to one version of @babel/core and now there are 3. Do we know why?
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'm not entirely sure, but I did install some of these in different commits and then squashed it down. I am wondering if it has to do with @vitejs/[email protected]
or some of the other packages needed a newer version of core, but I would think this could be deduped if needed? Not sure the easiest way to do that.
@@ -19,6 +19,7 @@ export const VueI18n = createI18n() | |||
export function createI18n (opts = {}) { | |||
return _createI18n<MessageSchema, 'en-US'>({ | |||
locale: 'en-US', | |||
// @ts-expect-error |
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.
oof, this is a huge shortcoming of that library when it comes to typescript... how about we coerce the type instead? e.g.
/**
* precompiled messages from unplugin-vue-i18n do not include explicit keys derived from
* the filenames of the raw message sources, so it must be coerced.
*/
messages: compiledMessages as { 'en-US': MessageSchema },
…n unplugin-vue-i18n and vue-i18n [run ci]
@cacieprins done in 8621571. |
#29634) * fix vite * chore: updating v8 snapshot cache * chore: updating v8 snapshot cache * chore: updating v8 snapshot cache --------- Co-authored-by: cypress-bot[bot] <+cypress-bot[bot]@users.noreply.github.com>
@AtofStryker There are some conflicts with the v8 snapshots |
@jennifer-shehane kicked off the job to update them. Then in conflicts are still present I will elect to take the changes on this branch |
@AtofStryker Still conflicts |
in this case its just the hash so thats good!. I'll push up in a few with the conflicts resolved |
…date_monorepo_to_vite_5
@AtofStryker Sorry 😅 more conflicts. |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
updates the monorepo from vite 4 to vite 5. additionally updates babel and vite related dependencies.
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?