Exports support, dynamic import from CJS support, package boundary emission #129
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,8 +78,8 @@ function getExportsTarget (exports, conditions, cjsResolve) { | |
| } | ||
| else if (Array.isArray(exports)) { | ||
| for (const item of exports) { | ||
| const target = getExportsTarget(item, conditions, cjsResolve); | ||
| if (target !== undefined && target !== null && target.startsWith('./')) | ||
guybedford marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return target; | ||
| } | ||
| } | ||
|
|
@@ -90,11 +90,14 @@ function getExportsTarget (exports, conditions, cjsResolve) { | |
| condition === 'import' && !cjsResolve || | ||
| conditions.includes(condition)) { | ||
| const target = getExportsTarget(exports[condition], conditions, cjsResolve); | ||
| if (target !== undefined && (target === null || target.startsWith('./'))) | ||
|
There was a problem hiding this comment. Is it expected that this There was a problem hiding this comment. Yes they should be the same - fixed. There was a problem hiding this comment. @guybedford They are a little different still. Why use There was a problem hiding this comment. Right, they are written differently but do the same thing. So the definition of the array fallback is that it will continue on invalid items, which is supposed to be different from the object conditions form. But I just realise now we're not actually doing that either! Done. Note the extreme exports edge cases get pretty complex but aren't needed in 99% of cases... so we are just rattling out the There was a problem hiding this comment. @guybedford I don't see a test for the array of There was a problem hiding this comment. Yes there is a test at https://github.com/vercel/node-file-trace/pull/129/files#diff-fb129c0544238468b2fbaf801b451a7f for the fallback support. |
||
| return target; | ||
| } | ||
| } | ||
| } | ||
| else if (exports === null) { | ||
| return exports; | ||
| } | ||
| } | ||
|
|
||
| function resolveExportsTarget (pkgPath, exports, subpath, job, cjsResolve) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| require('es-get-iterator'); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| require('pkg'); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| [ | ||
| "package.json", | ||
| "test/unit/exports-fallback/input.js", | ||
| "test/unit/exports-fallback/node_modules/pkg/index.js", | ||
| "test/unit/exports-fallback/node_modules/pkg/package.json", | ||
| "test/unit/exports-fallback/node_modules/pkg/require-main.cjs" | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be
devDependenciesand the version number should be pinned 🙂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not currently pinning any versions in this repo! I forget that's the preferred pattern, let's get a separate PR up for that.