-
Notifications
You must be signed in to change notification settings - Fork 4.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
Add an example esm-example package and use it in edit-post #34172
Conversation
Size Change: +3.61 kB (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
packages/edit-post/src/index.js
Outdated
@@ -147,6 +147,9 @@ export function initializeEditor( | |||
); | |||
} | |||
|
|||
// esm-example | |||
import( '@wordpress/esm-example' ).then( ( mod ) => mod.run() ); |
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.
Is the following case in webpack plugin necessary for the import above?
if ( ESM_PACKAGES.includes( request ) ) {
return 'import ' + request;
}
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.
yes, this config ensures webpack keeps the import as is and do not try to bundle or resolve the @wordpress/esm-example
dependency.
Ideally, I'd like webpack to rewrite it importShim( '@wordpress/esm-example' ).then( ( mod ) => mod.run() );
but I was not able to do so.
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 have much to add to this review, but it looks super interesting. What are the implications exactly?
@sarahmonster the implication is that we can start extracting code to lazy-loaded modules and have them work both in npm packages and in Gutenberg itself. There are several things we could explore later once this lands:
|
That's awesome! Just a note that the heaviest part of the classic block isn't anything implemented in Gutenberg, but the dependency on TinyMCE itself (as initialized by WP core). Has that changed? Otherwise there'd still be a lot of work necessary to async load the classic/freeform block. |
No, I actually didn't look at the classic block at all, I was thinking that if we find a way to make tinymce available as an esm package somehow we could lazy load it using this PR. That may be utopia, I don't know but it's just one use-case. |
Yeah unfortunately it's a long shot... I spent a long time trying to figure out how to asynchronously load TinyMCE and it turns out there's a lot of work that WP core does to initialize TinyMCE (especially around i18n stuff) that become a blocker. Of course you could re-implement all that initialization client side. It'd be a HUGE win to async load TinyMCE. That thing is a tremendous bloat for something that is so rarely used. I think it would have an actual measurable environmental impact considering the size and how often Gutenberg is loaded, not to mention a significant speed increase. More details about it here: #29681 |
I have one good news and one bad news here:
So basically, I think if folks like the approach, we can consider starting to use this in core code for some time. |
For this point, I read a bit and tried several approaches, I think extension less files might not be a great solution here. Let's start with one entry per module (like the standard recommends). |
I do agree this is a problem, but I think we can try somewhat sell it better. If we state to plugin developers something like: "Hey! We are starting the process of modernizing our JS packages loading strategy, to use the latest and coolest ESM and import maps. Import-maps are not yet fully supported in all the browsers, so if you want to use the new ESM packages in your regular scripts, you would have to use
Now putting the plugin developer hat on, I'd read it as: Oh, there is a breeze of new features coming. But for the old scripts, everything works as before, so I feel safe and calm. Forgive me if I missed something important here. But for me personally, it does not look that bad. The trade of using a new Web feature in the WordPress platform does not require any refactor on the plugin end but just to use slightly WordPress-specific syntax instead of native. That's only 2 characters difference, and WP will keep its compatibility for a long. Plus, if I understood the OP correctly, this difference applies only to the new packages, that we will ship as ESM, not to the existing ones. |
@tomalec your reasoning makes total sense to me yes, I just wanted to avoid that temporary step. But in the end it's not that bad, once it's well supported we could just do |
Related to #36716.
A simpler version of #34140
The idea of this PR is simpler than the other one, instead of rewriting all the core modules to be esm modules and trying to be backward compatible, the following PR does this instead:
edit-post
(when loading the editor) as a proof of concept to ensure everything works as intended.This is very promising and is almost ready IMO. Here are some of the tasks that I'd like to do to land it:
import( something )
no, I think we might need a web pack config to rewriteimport( something )
toimportShim( something )
but failed to do so. I could use some help to achieve this item.@wordpress
esm packages. It might require generating files without the.js
extension and putting these files in the same folder.