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

Distribute an EcmaScript Module #231

Closed
federicocarboni opened this issue Jun 1, 2020 · 5 comments
Closed

Distribute an EcmaScript Module #231

federicocarboni opened this issue Jun 1, 2020 · 5 comments

Comments

@federicocarboni
Copy link

Since the web is moving towards EcmaScript modules (and most browsers supported by this polyfill also support modules) I believe that an ESM version of the script should be included in the dist folder, e.g. browser-polyfill.esm.js.
This could be easily done by wrapping the script with a wrapper for CommonJS (like Rollup's CommonJS plugin does it), and wouldn't be a breaking change.

@eritbh
Copy link

eritbh commented Mar 23, 2021

Looks like this was added in 0.6.0 a while back: https://github.com/mozilla/webextension-polyfill/releases/tag/0.6.0

@Rob--W
Copy link
Member

Rob--W commented Mar 24, 2021

Fixed by #202

@Rob--W Rob--W closed this as completed Mar 24, 2021
@federicocarboni
Copy link
Author

federicocarboni commented Mar 25, 2021

@eritbh Making it "loadable as an ES module" doesn't make it an actual ES module.

If it can be loaded with

// with webpack and other similar bundlers
var browser = require("webextension-polyfill");
// or using requirejs without a build step
require(["webextension-polyfill"], function (browser) {
    console.log(browser);
});

I would also expect the following to work

import browser from "./webextension-polyfill.js";

From the README

Be aware that the polyfill module does not export the browser API object, but defines the browser object in the global namespace (i.e. window).

It would be better if there was a proper ES module in the dist folder so that it wouldn't have to define browser globally, and it would also make it easier to use the polyfill in code-splitting bundles.

Interop between commonjs/AMD and ESM is inconsistent because there is no way to create an ES module that can be loaded as a script. I don't think it would be too difficult to add a proper ES module to the dist folder and point to it using the exports field in package.json.

It could just be a simple wrapper like the following

export default ((factory) => {
  var mod = { exports: {} };
  return factory(mod, mod.exports);
})((module, exports) => {
  // ...
});

@eritbh
Copy link

eritbh commented Mar 26, 2021

That's interesting - what sort of situation might cause you to want browser as an imported value rather than a global object? I can't think of anything, especially since extension scripts run in isolated worlds and global namespace pollution isn't a huge concern. Your solution does seem easy to implement though, and it does make sense given the exported value is available to other paradigms.

In any event, it looks like I was wrong about this being fixed already - I wasn't clear on what you were asking for, and the issue @Rob--W linked as "fixing" this one is from before this issue was even opened.

@federicocarboni
Copy link
Author

Distributing a proper ES module makes it easier to work with modern build systems and it would make more sense if the polyfill behaved the same with all module formats.

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

3 participants