-
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
Update WordPress Monorepo #50922
Update WordPress Monorepo #50922
Conversation
4249cec
to
4be2f4b
Compare
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~4108 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~7040 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~1498 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. 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. |
Smoke tested Gutenboarding, everything seems to work but someone from that team should probably take a closer look. Also took a look through the WP components page in the devdocs and everything looks to be in good shape including our theme overrides. Lots of unit test failures, however, some of which appear to be legitimate bugs in core:
This error appears many many times:
I'm not sure what to make of it, however 🤔 |
Okay I have a fix for the |
Okay, @noahtallen and I spent some time 🍐 on the MutationObserver issue. It seems like the issue boils down to the fact that we've got two different The suggested solution is to either: (a) include a polyfill for the MutationObserver API via the Noah and I couldn't decide which of these was better so we're deferring to @scinos 😀 Additional note: The reason Gutenberg hasn't run into this issue yet is because they don't employ the Anyway, what do you think @scinos? |
Looks like adding an explicit dependency to As you said, this fails because we have two versions of
Yarn decides to hoist the former, so However, looking at Jest resolution for test environment, it tries to resolve it relative to the project dir (in this case Adding it as an explicit dependency in |
bd75439
to
8ddfbf1
Compare
Edit: I believe we've added back everything now, but feel free to take another look to be sure. |
Thanks for adding that again sara! I just pushed the eslint config fix, as will as a fix for the |
return ( | ||
<li className={ classes } data-tip-target={ props.tipTarget } data-post-type={ props.postType }> | ||
<a | ||
className="sidebar__menu-link" | ||
onClick={ props.onNavigate } | ||
href={ url } | ||
target={ showAsExternal ? '_blank' : null } | ||
rel={ isExternalLink ? 'noopener noreferrer' : null } |
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.
Note: noreferrer
includes noopener
behavior according to the HTML standard.
e9c4aef
to
0c8dcca
Compare
I've tried adding the dependency on jest-environment-jsdom by running At this point, I believe this should be rebased, deduped, and ready for another look. |
8a14e88
to
aea6599
Compare
Rebased and deduped. |
Can someone from @Automattic/luna take a look at this just to smoke test Gutenboarding once again? |
@sarayourfriend I just smoke tested Gutenboarding, lookin' good! Are the changes in the bundle sizes expected? |
I believe those are probably due to the new component system integration, though I didn't think they were actually being exported. @gziolo Is the published |
I think we assume by default it's disabled and in the Gutenberg plugin we enable it through |
How often do we get these from Renovate? Do we have to find a better tool if Renovate isn't working well enough? Thanks! |
The
I have no idea why that's happening, though :( |
Confirmed that the size increases are due to G2 being present in /cc @gziolo It doesn't seem like tree shaking is working for the current strategy... any ideas? |
aea6599
to
a498246
Compare
See my previous replay: #50922 (comment). You need to update the webpack config add ensure |
05ebefa
to
0bdc53f
Compare
I'm glad setting I thought that the default for |
0bdc53f
to
c7eabed
Compare
I bumped @wordpress/env back to v4. I think it had gotten changed back to v3 during a rebase, but it's needed to fix the phpunit tests :) |
What's left on this PR @noahtallen? Do we just need to request reviews from some individual teams? Are those e2e test failures legit? |
c7eabed
to
926f9df
Compare
Unsure about e2e tests, will investigate 👀 Otherwise, i think just smoke testing everything which consumes these packages. |
The e2e failure is a known broken test from what I can tell :) |
926f9df
to
0063ea8
Compare
I've tested ETK, gutenboarding (all the way through launch), and the calypso application and everything looks good so far. @sarayourfriend do you know what to test for blocks & components? |
We should just smoke test the WP Components Gallery but honestly testing Gutenboarding is even better. I'll smoke test the gallery now. Update: Components Gallery looks good to me. |
0063ea8
to
d6e3688
Compare
I've run through the failing e2e's manually and confirmed they are still working. Possibly flaky, or not waiting long enough for the UI element to display |
Changes proposed in this Pull Request
Alternative to #50470 with our own changes, such as deduplication.
Testing instructions
Some items we need to verify which consume this on some level: