Skip to content

Commit

Permalink
module: bring back support for exports: false
Browse files Browse the repository at this point in the history
  • Loading branch information
MylesBorins committed Mar 6, 2020
1 parent 5cc0754 commit b66496d
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 18 deletions.
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ function applyExports(basePath, expansion) {
const mappingKey = `.${expansion}`;

let pkgExports = readPackageExports(basePath);
if (pkgExports === undefined || pkgExports === null)
if (pkgExports === undefined || pkgExports === null || pkgExports === false)

This comment has been minimized.

Copy link
@MylesBorins

MylesBorins Mar 6, 2020

Author Owner

This is wrong and should be ignored, but it is where the CJS codepath for the error is coming from

return path.resolve(basePath, mappingKey);

if (isConditionalDotExportSugar(pkgExports, basePath))
Expand Down
36 changes: 19 additions & 17 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1145,25 +1145,27 @@ Maybe<URL> PackageMainResolve(Environment* env,

if (!pcfg.exports.IsEmpty()) {
Local<Value> exports = pcfg.exports.Get(isolate);
Maybe<bool> isConditionalExportsMainSugar =
IsConditionalExportsMainSugar(env, exports, pjson_url, base);
if (isConditionalExportsMainSugar.IsNothing())
return Nothing<URL>();
if (isConditionalExportsMainSugar.FromJust()) {
return ResolveExportsTarget(env, pjson_url, exports, "", "", base);
} else if (exports->IsObject()) {
Local<Object> exports_obj = exports.As<Object>();
if (exports_obj->HasOwnProperty(env->context(), env->dot_string())
.FromJust()) {
Local<Value> target =
exports_obj->Get(env->context(), env->dot_string())
.ToLocalChecked();
return ResolveExportsTarget(env, pjson_url, target, "", "", base);
if (!exports->IsFalse()) {

This comment has been minimized.

Copy link
@MylesBorins

MylesBorins Mar 6, 2020

Author Owner

This is also incorrect but the MJS code path for the error is here.

Maybe<bool> isConditionalExportsMainSugar =
IsConditionalExportsMainSugar(env, exports, pjson_url, base);
if (isConditionalExportsMainSugar.IsNothing())
return Nothing<URL>();
if (isConditionalExportsMainSugar.FromJust()) {
return ResolveExportsTarget(env, pjson_url, exports, "", "", base);
} else if (exports->IsObject()) {
Local<Object> exports_obj = exports.As<Object>();
if (exports_obj->HasOwnProperty(env->context(), env->dot_string())
.FromJust()) {
Local<Value> target =
exports_obj->Get(env->context(), env->dot_string())
.ToLocalChecked();
return ResolveExportsTarget(env, pjson_url, target, "", "", base);
}
}
std::string msg = "No \"exports\" main resolved in " +
pjson_url.ToFilePath();
node::THROW_ERR_PACKAGE_PATH_NOT_EXPORTED(env, msg.c_str());
}
std::string msg = "No \"exports\" main resolved in " +
pjson_url.ToFilePath();
node::THROW_ERR_PACKAGE_PATH_NOT_EXPORTED(env, msg.c_str());
}
if (pcfg.has_main == HasMain::Yes) {
URL resolved(pcfg.main, pjson_url);
Expand Down
8 changes: 8 additions & 0 deletions test/es-module/test-esm-exports.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
{ default: 'self-cjs' } : { default: 'self-mjs' }],
// Resolve self sugar
['pkgexports-sugar', { default: 'main' }],
// Resolve exports false
isRequire ? [

This comment has been minimized.

Copy link
@MylesBorins

MylesBorins Mar 6, 2020

Author Owner

likely want to also test {} and that deep traversal is not possible

'pkgexports-false-cjs',
{ default: 'false-exports-cjs'
}] : [
'pkgexports-false-mjs',
{ default: 'false-exports-mjs'
}],
]);

for (const [validSpecifier, expected] of validSpecifiers) {
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/node_modules/pkgexports-false-cjs/main.cjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions test/fixtures/node_modules/pkgexports-false-cjs/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/fixtures/node_modules/pkgexports-false-mjs/main.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions test/fixtures/node_modules/pkgexports-false-mjs/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b66496d

Please sign in to comment.