awesome-webextension / webpack-target-webextension

WebExtension Target for Webpack 5. Supports code-splitting and HMR.
MIT License
44 stars 5 forks source link

Use native `import()` calls #18

Closed fregante closed 3 years ago

fregante commented 3 years ago

I just tried import(chrome.runtime.getURL()) and it seems to work both in Chrome and Firefox, in content scripts and background pages 🎉

This is because the bug that your readme points to, has been marked as solved and shipped in the latest Firefox release: https://bugzilla.mozilla.org/show_bug.cgi?id=1536094

Could this plugin be modified to use import() instead of the current messaging mechanism?

Does Webpack even leave native import() calls in the bundle? In my test, import() is still converted to Webpack’s own loading mechanism and I don't see anything about raw import() calls in the docs:

https://webpack.js.org/guides/code-splitting/#dynamic-imports https://webpack.js.org/api/module-methods/#import-1

crimx commented 3 years ago

Yes. The README has been updated.

Nothing needs to be changed. The messaging mechanism is a fallback method which will not be triggered if native import works.

Does Webpack even leaving native import() calls in the bundle? In my test, import() is still converted to Webpack’s own loading mechanism and I don't see anything about raw import() calls in the docs

That's because by default Webpack sets the Web target which uses JSONP under the hood. The native import is implemented by this lib.

fregante commented 3 years ago

I might have to re-read the readme because it currently isn’t happening in my extension, otherwise I wouldn’t have run into #15

crimx commented 3 years ago

Are you using Webpack5?

crimx commented 3 years ago

https://github.com/crimx/webpack-target-webextension/blob/168aad81dffb650ad4569dd1ccb435c7be1881aa/lib/webpack5/LoadScriptRuntimeModule.js#L29-L36

@Jack-Works This should use native dynamic import instead right?

fregante commented 3 years ago

Yes I’m on v5.

That piece of code is the opposite of import(), which requires no HTML tags

crimx commented 3 years ago

Yes. It was suppose to be native dynamic import. The v5 code is mainly done by @Jack-Works . Let's see if that's the right place to add.

Jack-Works commented 3 years ago

https://github.com/crimx/webpack-target-webextension/blob/168aad81dffb650ad4569dd1ccb435c7be1881aa/lib/webpack5/LoadScriptRuntimeModule.js#L29-L36

@Jack-Works This should use native dynamic import instead right?

This normalLoader is used in chrome-extension://* page, which is a normal HTML page. Content script loader is in https://github.com/crimx/webpack-target-webextension/blob/168aad81dffb650ad4569dd1ccb435c7be1881aa/lib/webpack5/LoadScriptRuntimeModule.js#L25-L28

Jack-Works commented 3 years ago

It's cool to use the native import to load scripts, but we also need to support older versions (do we?). How can we fall back in such a situation?

crimx commented 3 years ago

V4 was already using native dynamic import and fallbacks to the runtime messaging.

crimx commented 3 years ago

https://github.com/crimx/webpack-target-webextension/blob/168aad81dffb650ad4569dd1ccb435c7be1881aa/lib/webpack4/WebExtMainTemplatePlugin.js#L249-L255

@Jack-Works Any idea how to translate this into v5? How can I check the existence of a chuck like installedChunks[chunkId].

fregante commented 3 years ago

It appears that Webpack 5.41 added support for native import calls in the output, if I'm reading this correctly: https://github.com/webpack/webpack/releases/tag/v5.41.0

So perhaps this plugin only needs to add chrome.runtime.getURL() to its URLs

Jack-Works commented 3 years ago

It appears that Webpack 5.41 added support for native import calls in the output, if I'm reading this correctly: https://github.com/webpack/webpack/releases/tag/v5.41.0

Implemented, but I'm not using ModuleLoader, still using jsonp loader. Because ModuleLoader requires output.module=true, output.chunkFormat="module" and output.chunkLoading="import", it will emit import.meta which cannot be loaded directly (require an extra file to load them as module).

crimx commented 3 years ago

npm published.

fregante commented 3 years ago

Thank you!

import.meta which cannot be loaded directly (require an extra file to load them as module).

Is this a limitation of web extensions? Does this mean output.module=true is still not possible here? In that case, should a new issue be opened to track that limitation? As more people shift to native ES support it's possible that they will also use that config and wonder why it doesn't work.

Jack-Works commented 3 years ago

Thank you!

import.meta which cannot be loaded directly (require an extra file to load them as module).

Is this a limitation of web extensions? Does this mean output.module=true is still not possible here? In that case, should a new issue be opened to track that limitation? As more people shift to native ES support it's possible that they will also use that config and wonder why it doesn't work.

To use module syntaxes import ... export ... or import.meta, you must be in the module mode of JavaScript. For the Web platform, there are two ways: <script type="module"> or import().

There is no way to declare the entrance of your extension file is a module in the manifest.json. If you want to use it, you need a script mode file and its content is import('./content-script-module.js').

fregante commented 3 years ago

I understand, but is this limit explained anywhere except this issue? It'd be great to have this plugin emit a warning if it detects such configuration (if possible) or at least have ESM support documented on the readme. Because I myself will forget how to set this up in just about 30 days 😅

Jack-Works commented 3 years ago

I understand, but is this limit explained anywhere except this issue? It'd be great to have this plugin emit a warning if it detects such configuration (if possible) or at least have ESM support documented on the readme. Because I myself will forget how to set this up in just about 30 days 😅

The ESM emit of Webpack is in the experimental stage and not being enabled by default (and it does not support HMR for now). If you enabled it, you must be aware of it. (and the key point is I don't have time to read Webpack source code to support that 🤣). I think the related document will appear when I (or someone else) add support for that.