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

WIP: named: commonjs exports #1226

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vikr01
Copy link
Contributor

@vikr01 vikr01 commented Nov 2, 2018

This + #1222 closes #1145

This adds a new schema to named:

commonjs: boolean | { require: boolean, exports: boolean }

The option commonjs accepts either a boolean or a shape with require and exports booleans within. Defaults to false.

The reason why the shape is allowed is to avoid having commonjs require statements linted. Using the boolean only option will apply to both require and exports.

The following cases of assigning exports are checked:

Object.defineProperty(exports, "key", { value: "myValue" });
Object.defineProperty(module.exports, "key", { value: "myValue" });
Object.defineProperty(module["exports"], "key", { value: "myValue" });
Object.defineProperty(exports, "key", { "value": "myValue" });
Object.defineProperty(module.exports, "key", { "value": "myValue" });
Object.defineProperty(module["exports"], "key", { "value": "myValue" });

exports = {...};
module.exports = {...};
module["exports"] = {...};

exports.key = ...;
module.exports.key = ...;
module["exports"].key = ....;

All declarations must be at the top level. For example, this will not be checked:

if (foo) {
  exports.foo = foo;
}

The reason why we check Object.defineProperty is that babel will build es6 modules that way, i.e.:

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

We need to handle those (truthy) __esModule properties correctly -- as in the default export will be the entire module.exports if that's not set.

ExportMap.for and ExportMap.parse now take an options parameter (defaults to {}), with the keys useCommonjsExports and noInterop. Reason for noInterop is that plain require statements should not have .default being the entire module.exports if it doesn't have a truthy __esModule value.

TODO: Configure for babel-plugin-add-module-exports built files. (Check for module.exports = exports["default"] at the end.)

@vikr01 vikr01 force-pushed the commonjs-named-exports branch 2 times, most recently from 90fe2ed to 0fc26db Compare November 2, 2018 08:44
@coveralls
Copy link

coveralls commented Nov 2, 2018

Coverage Status

Coverage decreased (-1.2%) to 96.195% when pulling 322d11e on vikr01:commonjs-named-exports into db471a8 on benmosher:master.

@benmosher
Copy link
Member

Ah cool! It actually used to do this back in v0.x or v1, but I removed it for efficiency and to narrow the scope. I will have to think about this carefully before reintroducing.

@ljharb ljharb requested a review from benmosher January 22, 2019 15:38
@edi9999
Copy link

edi9999 commented Feb 11, 2019

+1 on this "feature", I mostly use exports like this

module.exports = {
   foo: true,
   ...
}

and it is annoying that I have some unnoticed wrong imports.

@vikr01 vikr01 changed the title named: commonjs exports WIP: named: commonjs exports Apr 28, 2019
@ljharb ljharb marked this pull request as draft January 31, 2021 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

import/named for module.exports?
4 participants