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

Does not work when imported as a module. #202

Closed
MicahZoltu opened this issue Sep 21, 2019 · 7 comments · Fixed by #216
Closed

Does not work when imported as a module. #202

MicahZoltu opened this issue Sep 21, 2019 · 7 comments · Fixed by #216

Comments

@MicahZoltu
Copy link

If you attempt to load the webextension-polyfill as a module, you get a runtime failure in both Chrome and Firefox due to the utilization of this as the global parameter. this is undefined in the context of an ES module, so when the root UMD module function is called and this is passed to the global parameter, it ends up passing undefined. Then when global.browser is referenced you get a type error.

The same problem occurs with the minified source.

Error message in Firefox:

TypeError: global is undefined

Recommendation

Use globalThis || window || this instead of this as the first parameter. I believe any browser that supports ES modules also supports either globalThis or window, so this will only be hit in the rare case.

Repro Case

<!DOCTYPE html>
<html lang='en'>
	<head>
		<meta charset='utf-8'>
	</head>
	<body>
		<script type='module' src='browser-polyfill.js'></script>
	</body>
</html>
{
	"manifest_version": 2,
	"name": "ModuleFailureRepro",
	"version": "1.0",
	"description": "Repro case for loading a polyfill as an ES module",
	"background": {
		"page": "background.html"
	}
}
@MicahZoltu
Copy link
Author

A quick attempt to submit a PR shows that the problem is that the build scripts appear to be turning the script into a UMD module, and the output of that script is the problem. Fixing this will likely require either upgrading the build script tooling, or getting the bug fixed in the build script tooling.

@MicahZoltu
Copy link
Author

It appears the issue is coming from this buildscript step:

["transform-es2015-modules-umd", {
globals: {
"webextension-polyfill": "browser",
},
exactGlobals: true,
}],

@MicahZoltu
Copy link
Author

I have filed an issue with Babel about this, TBD whether anything will come from that.

@rpl
Copy link
Member

rpl commented Oct 24, 2019

@MicahZoltu We (@Rob--W and I) discussed about this and we are open to fix this on the polyfill side by building a separate .mjs version that is going to be a pure ES6 module (and make it part of the npm package release).

@MicahZoltu
Copy link
Author

FWIW babel/babel#10477 has been merged. I believe it will be available in Babel 7.7.0, whenever that release gets cut.

@rpl
Copy link
Member

rpl commented Nov 13, 2019

@MicahZoltu that's great (also thanks for filing it upstream).

we should definitely look into updating this repo to use a more recent version of babel (which was going to be necessary even besides this issue), I just filed #211 to keep track on it.

@rpl
Copy link
Member

rpl commented Dec 12, 2019

We have just landed a fix for this in #214

I'm not closing this yet because we should also add some details about how to use the polyfill by loading as a JS module in the README.md file (and I just added a "needs: docs" label to make it visible that this is pending a pull request to do that).

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

Successfully merging a pull request may close this issue.

2 participants