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

fix: Update babel deps to babel 7 #214

Merged
merged 3 commits into from Dec 12, 2019
Merged

Conversation

rpl
Copy link
Member

@rpl rpl commented Nov 26, 2019

This PR updates babel to babel 7 (#211) and some other babel-related dependency (like babel-preset-babily -> babel-preset-minify, grunt-babel to version 8.0.0).

Quite unfortunately, this PR isn't ready yet because of an issue with the new version of the babel's transform-modules-umd plugin (which was one of the reasons why we wanted to update babel soon enough):

Even if the build process is completing successfully with the new version of the transform-modules-umd, our js file isn't being wrapped correctly as it was with previous versions of this babel transform plugins (and all the tests are failing, which is expected given that the wrapped module fails to export the browser API object).

In particular it seems that this babel plugin is now trying to discover the exports by parsing the module sources and it ends to don't find any and not passing any module object to the function that wraps our module source, here is an extract of the diff between the last version of the umd modules as released on npm and the one built from this branch:

--- ../browser-polyfill-umd-0.5.0.js    1985-10-26 09:15:00.000000000 +0100
+++ dist/browser-polyfill.js    2019-11-26 16:52:29.146299551 +0100
@@ -1,19 +1,22 @@
 (function (global, factory) {
   if (typeof define === "function" && define.amd) {
-    define("webextension-polyfill", ["module"], factory);
+    define("webextension-polyfill", [], factory);
   } else if (typeof exports !== "undefined") {
-    factory(module);
+    factory();
   } else {
     var mod = {
       exports: {}
     };
-    factory(mod);
+    factory();
     global.browser = mod.exports;
   }
-})(this, function (module) {
+})(typeof globalThis !== "undefined" ? globalThis : typeof self !== "undefined" ? self : this, function () {

I took a look to the trasform module sources and at a first glance it seems that it may be easier to actually replace the current strategy with a custom "less generic" transformation babel plugin (which we could write to specifically wrap our module correctly), or from a grunt task that would just wrap the module into a UMD template provided by us.

@rpl
Copy link
Member Author

rpl commented Dec 11, 2019

I've looked a bit more into how the new version of the transform-umd-modules works internally and, as I assumed, explicitly including an export statement in the polyfill sources makes the transform plugin to detect the exports, e.g.:

let extensionAPIs;
...
   // The build process adds a UMD wrapper around this file, which makes the
   // `module` variable available.
   extensionAPIs = wrapAPIs(chrome);
 } else {
   extensionAPIs = browser;
 }

export default {browser: extensionAPIs};

but the result is still not what we would like it to be, in particular because (when loaded as a regular script on Firefox) it is going to overwrite the original browser global with the default export object instead of the original browser object itself:

...
    var mod = {
      exports: {}
    };
    factory(mod.exports);
    global.browser = mod.exports;
...
    extensionAPIs = wrapAPIs(chrome);
  } else {
    extensionAPIs = browser;
  }

  var _default = {
    browser: extensionAPIs
  };
  _exports.default = _default;
});

And so I looked into the alternative strategy mentioned in the pull request description:

a custom "less generic" transformation babel plugin (which we could write to specifically wrap our module as we need it to be)

and updated this pull request accordingly.

(and I also added a smoke test to verify as part of the automated tests that the bundle generated with this new minimal custom transformer plugin is also going to fix #202, on both Chrome and Firefox).

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

Successfully merging this pull request may close these issues.

None yet

2 participants