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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support es modules without .mjs extension #34

Merged
merged 4 commits into from
Nov 8, 2021
Merged

Conversation

snyamathi
Copy link
Collaborator

This PR consolidates the import and require logic into a single function, loadModule which can load a config using import for node 12+ and require otherwise for legacy Node.

An import statement can reference an ES module or a CommonJS module. import statements are permitted only in ES modules, but dynamic import() expressions are supported in CommonJS for loading ES modules.
https://nodejs.org/api/esm.html#interoperability-with-commonjs

When importing a default export with dynamic imports, it works a bit differently. You need to destructure and rename the "default" key from the returned object.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#importing_defaults

This has a consequence with modules which are transpiled from ESM to CJS, often using something like babel-plugin-transform-modules-commonjs. This effectively adds a second layer of .default that we may need to dereference.

// src
export default [/* config */];

// dist
Object.defineProperty(exports, "__esModule", {
  value: true,
});

exports.default = [/* config */];

Let me do some final testing though - and let's not merge this on a Friday cause I'm done for the week 馃槃

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

} catch (e) {
callback(new ParseError(e, id));
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file and the one above it are intentionally formatted to look nearly identical. The only real difference between the two is that when doing a dynamic import we have to dereference the default export. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#importing_defaults

@@ -261,52 +222,56 @@ Config.prototype = {
addConfigContents: function (bundleName, configName, fullPath, contents, callback) {
var self = this;

contents = this._parseConfigContents(fullPath, contents);
this._parseConfigContents(fullPath, contents, function (err, contents) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snyamathi snyamathi merged commit b5c8dab into master Nov 8, 2021
@snyamathi snyamathi deleted the import branch November 8, 2021 19:48
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