-
Notifications
You must be signed in to change notification settings - Fork 4.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
Build tooling: Remove support for IE11 #31110
Conversation
Size Change: -96.1 kB (-6%) ✅ Total Size: 1.39 MB
ℹ️ View Unchanged
|
I'm just wondering whether this statement includes the default front-end block styles that Gutenberg/Core provides? If a theme is using those styles this change may affect a site's appearance for visitors. |
2d8953b
to
f0d33dd
Compare
Apparently the issues here seem to be related to webpack not understanding some ESnext syntax (since now it's not being transpiled). By looking on the internet, it seems to be something that is generally fixed by upgrading webpack. Any news about webpack 5 upgrade cc @gziolo @jsnajdr Also, if you have any idea about how we could solve this without upgrade :) |
It looks like webpack doesn't understand |
The challenge is that we will have to upgrade webpack from v4 to v5 in both Gutenberg and WordPress core because they both use the same code from |
I'm testing whether enforcing transpilation of include: [ 'proposal-nullish-coalescing-operator' ], |
The issue is that webpack uses the I'm afraid we'll need to transpile optional chaining until the webpack 5 upgrade is done. @scinos Do you have any interesting knowledge to share here? I think you have direct on-hands experience from solving exactly the same issue in Calypso. |
It looks like this is the way to go. There is one failing unit test but it's expected since Babel generates now a bit different output. We only need to regenerate the snapshot produced. |
Another issue to resolve, but an expected one: There are two ways we can approach it:
|
E2e tests also fail, so there are most likely more features missing in Chromium that's used there. We have fairly outdated version of Chromium, so we can try to upgrade |
@gziolo we should remove that check. |
There are 3 or 4 failing tests with the puppeteer upgrade (without the browserlist change), I could use some help debugging these. |
Thanks everyone for working through this!
@talldan When @Clorith reviewed the post, he did flag this. It is a good question that we need to discuss. I think there are a few paths we could take here:
I think proceeding with the first option for now is best, and we can explore the others if there is enough of a need that presents itself. But if anyone has other ideas or thoughts, please do share! |
There are still test failures:
There is no such thing as |
I've fixed the issue, it was actually a valid bug that was "fixed"/"hidden" by the transpiration of let/const |
It seems like we're good to do, e2e tests are passing. |
Should we document this change somewhere? The changelog of the presets package maybe? |
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.
Confirmed it all looks good here 👍
In most of the packages we have the following note:
It's no longer valid with the changes introduced. I don't think we should consider it a blocker, but we should discuss a path moving forward separately. We still have nearly two weeks before all these changes get published to npm. I only mentioned it earlier, but we need also to think if we want to keep both |
Thanks everyone! |
For cross referencing, here are some relevant Trac tickets:
|
I opened a follow-up #31270 that documents the changes applied for the consumers of WordPress packages. |
Description
For context, see: