-
Notifications
You must be signed in to change notification settings - Fork 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
chore: upgrade jest (and related packages) to v29 #78786
Conversation
This PR modifies the release build for editing-toolkit To test your changes on WordPress.com, run To deploy your changes after merging, see the documentation: PCYsg-mMA-p2 |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~113 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
77c2f42
to
f6fcc0f
Compare
This PR modifies the release build for notifications To test your changes on WordPress.com, run To deploy your changes after merging, see the documentation: PCYsg-elI-p2 |
This PR modifies the release build for wpcom-block-editor To test your changes on WordPress.com, run To deploy your changes after merging, see the documentation: PCYsg-l4k-p2 |
Migrated ETK and Odyssey stats to the shared testing config here: #79097 |
Looks great and works well in my testing. Looking forward to it unblocking this one 🚀 |
384f193
to
519c25d
Compare
Hey @flootr, I rebased this after merging #79097. I ran into some weird conflicts, so I ultimately removed a couple commits and did another dedupe after running While I was rebasing, I also forgot for a moment that I was rebasing and ended up finding a simpler fix for the e2e config type issue 😅 I also don't think we need |
Neat! Aside from this needing a rebase, I guess it should also wait for #79176 to land since #79097 got reverted. For the protocol, #79176 is approved and can be merged already. |
27d32c5
to
a6c43ca
Compare
done and marked as ready for final review! 🎉 |
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.
Looks good to me!
I noticed this line of code last week; we might be able to remove it:
wp-calypso/test/client/setup-test-framework.js
Lines 43 to 48 in 1043c24
// TODO: structuredClone wasn't available in Jest before verson 28 so this creates | |
// a version to be used for the tests. Once Jest is upgraded to 28 this should | |
// be removed. | |
global.structuredClone = jest.fn( ( value ) => { | |
return JSON.parse( JSON.stringify( value ) ); | |
} ); |
@@ -54,10 +54,6 @@ describe( '#announceSuccess()', () => { | |||
}, | |||
} ) ); | |||
} ); | |||
afterAll( () => { |
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.
Do we still need to restore the previous FormData implementation?
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 don't think we need it; just removed it.
Good spot, removed. |
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.
Works well for me and the codechanges make sense! Nice work y'all!
LGTM 🚢
I'll proceed to ship this one. |
Related to #78711
Proposed Changes
PR upgrades Jest (and related packages where necessary) to v29. We're currently on v27 which means we're jumping two major versions.
Testing Instructions
Verify all tests are green
Pre-merge Checklist