Skip to content
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

scripts with type different than "module" using dynamic import. #156

Closed
youknowriad opened this issue Aug 20, 2021 · 10 comments
Closed

scripts with type different than "module" using dynamic import. #156

youknowriad opened this issue Aug 20, 2021 · 10 comments

Comments

@youknowriad
Copy link

Hi there and thanks for your work on this polyfill.

I noticed that in Chrome, we can do import( 'some module' ) in any script file, it doesn't matter if it's a type="module" or not. And after reading a bit here, it seems that the polyfill doesn't implement support for this. Is this a design choice or some kind of limitation?

Thanks.

@guybedford
Copy link
Owner

@youknowriad yes this polyfill doesn't touch or polyfill scripts so won't affect dynamic import in non modules.

This could be more clearly documented as a limitation of the approach certainly. If you'd like to suggest wording along these lines that could help.

@youknowriad
Copy link
Author

Hi thanks for the quick reply.

To be honest, I'm very bad with wording suggestions :P so I'd prefer to leave that one for you and other contributors :)

So no plans to consider this in the future right?

@guybedford
Copy link
Owner

Unless there's a really compelling use case for this I would rather avoid it just for predictability of the polyfilling as well. If we parse all scripts on the page that seems like it might be this project overstepping its boundaries to me possibly. There is also the problem of double execution since all code up to the dynamic import would execute twice.

If you need to use a shimmed dynamic import in scripts I would advise just manually referencing importShim(...) instead of import(...) and things should work out. It will still use the native loader as much as possible and avoid unnecessary work.

@youknowriad
Copy link
Author

Happy to clarify my use-case more:

I'm working on trying to introduce import maps and ES modules to WordPress WordPress/gutenberg#34172

The scripts that could potentially make use of new ES modules dependencies are existing non module WP scripts. And these are part of the public API of WordPress that plugins and themes are relying on, on their own JS scripts. Meaning, It's not possible for us (WordPress Core) to rewrite these existing scripts as type="module" without introducing breaking changes for third-party WP plugins and themes. (WordPress has almost 0 breaking change policy)

If you need to use a shimmed dynamic import in scripts I would advise just manually referencing importShim(...) instead of import(...) and things should work out. It will still use the native loader as much as possible and avoid unnecessary work.

Yes, this is the approach I'm considering for now, it has some drawbacks for us though, in the sense that in the future, that "shim" could get removed when dynamic import support is good enough (I guess we can still ship const importShim = import; at that point.

@guybedford
Copy link
Owner

Very interesting to hear and thanks for the background here.

Can you perhaps share a full code sample of the sort of code using dynamic import in scripts that you can come across here?

@youknowriad
Copy link
Author

Here's some pseudo code. Right now we have scripts that do this.

// components.js

wp.components.Button = SomeReactComponent;

generally loaded as regular blocking scripts.

third-party plugins can add their own scripts and make use of these APIs:

# somerandomplugin.js

React.render( <wp.components.Button />, someTarget );

What we'd like to achieve is add the possibility for us (Core WordPress) and plugins to start lazy-loading dependencies as needed. For instance we want to introduce a syntax highlighting module.

# @wordpress/syntax-highlighting module
export default init = function( element ) { // do something }

we want to be able to use that new module in both scripts above, so for instance we'd update the components script to do this.

# components.js
wp.components.CodeEditor = function() {
   useEffect(() => {
       import( '@wordpress/syntax-highlighting' ).then( mod => mod.init( ref.current ) )
   }, [] );
} 

@guybedford
Copy link
Owner

So if the component script was updated per your last code example, the polyfill approach would then double-execute that script.

So for example if you had:

<script>
console.log('registering code editor');
wp.components.CodeEditor = function() {
   useEffect(() => {
       import( '@wordpress/syntax-highlighting' ).then( mod => mod.init( ref.current ) )
   }, [] );
} 
</script>

Then you would see the log registering code editor twice because the script is using a dynamic import.

My concern is therefore that polyfilling scripts doesn't solve the use case for scripts.

An alternative might be to support a script-polyfill type:

<script type="script-polyfill">
console.log('registering code editor');
wp.components.CodeEditor = function() {
   useEffect(() => {
       import( '@wordpress/syntax-highlighting' ).then( mod => mod.init( ref.current ) )
   }, [] );
} 
</script>

where the lexer would then entirely be in charge of execution and only rewrite scripts using dynamic import, and then there would only be a single execution.

@guybedford
Copy link
Owner

One problem with the above is that the script-polyfill is no longer blocking with the same semantics as normal scripts though... although es-module-shims will make sure to implement execution ordering correctly, we can't stop other dom executions.

@youknowriad
Copy link
Author

Indeed, it seems there's no potential solution that could solve the use-cases without any breaking changes. I'm happy for the issue to be closed (if that works for you) and continue with the importShim direct usage for now. Thanks for your time and work on this polyfill/shim.

@guybedford
Copy link
Owner

Sure, I don't see an easy path forward on this either, but if you reconsider what might be useful to you here please do let me know.

Note that you can alias importShim to whatever you like there as well, eg wpImport or some such!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants