Exports support, dynamic import from CJS support, package boundary emission #129

guybedford · 2020-06-26T03:30:35Z

This PR implements three new features

Support for the exports field as per Node.js, including conditional exports and package name self-resolution
Support for emitting package.json files when they inform the module format interpretation of a file - this results in more package.json files being emitted, as can be seen in the unit test updates here
Support for dynamic import in CJS files being traced

Exports support is by default disabled, and enabled by setting the exports: true option or providing a custom condition list.

For example, if building for Node.js 10, you'd want the exports support to be disabled since the "main" resolution would still apply - which would not be traced if enabling exports (when exports overrides it).

src/node-file-trace.js

src/resolve-dependency.js

styfle

The tests are only passing on Windows.

Both macOS and Linux appear to be stuck in an infinite loop.

Co-authored-by: Steven <steven@ceriously.com>

guybedford · 2020-06-27T01:41:00Z

@styfle I've implemented an alternative that separates "exports" from "exportsOnly" as a separate feature.

The default behaviour now will trace both exports and the main.

Setting exports allows changing the list of exports conditions that are traced.

Then setting exportsOnly: true is a mode for Node.js 12+ only which will no longer trace the legacy main.

guybedford · 2020-06-27T03:01:11Z

The remaining windows failures here are pretty odd - I can't replicate them on my Windows machine running the same Node.js version...

guybedford · 2020-06-27T06:09:03Z

Finally traced the issue... this should be good to merge.

styfle · 2020-06-28T18:17:50Z

@guybedford This doesn't work with es-get-iterator@1.1.0.

For example:

mkdir example && cd example
yarn init -y && yarn add es-get-iterator@1.1.0
echo 'require('es-get-iterator')' > start.js
nft print start.js

I would expect node_modules/es-get-iterator/index.js to be included but it is not.

Can we add an integration test for es-get-iterator?

guybedford · 2020-06-29T01:35:00Z

Nicely spotted, ok I've added the integration test and also included a unit test for the array fallback case.

styfle · 2020-06-29T13:07:57Z

package.json

@@ -26,6 +26,7 @@
    "acorn-numeric-separator": "^0.3.0",
    "acorn-static-class-features": "^0.2.1",
    "bindings": "^1.4.0",
+    "es-get-iterator": "^1.1.0",


Should be devDependencies and the version number should be pinned 🙂

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.

src/resolve-dependency.js

styfle · 2020-06-29T13:15:34Z

src/resolve-dependency.js

@@ -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)
+        if (target !== undefined && (target === null || target.startsWith('./')))


Is it expected that this if statement does not match the one on line 82 above?

Yes they should be the same - fixed.

@guybedford They are a little different still. Why use continue with one and not the other?

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 "exports": [{}, { "asdf": null }, null] kinds of odd behaviours here.

@guybedford I don't see a test for the array of exports like you mentioned above? They are all object exports as far as I see. Can we add a test for array?

Yes there is a test at https://github.com/vercel/node-file-trace/pull/129/files#diff-fb129c0544238468b2fbaf801b451a7f for the fallback support.

readme.md

Co-authored-by: Steven <steven@ceriously.com>

styfle

Great work!

styfle · 2020-07-15T20:16:38Z

test/unit/yarn-workspaces/output.js

  "test/unit/yarn-workspaces/input.js",
  "test/unit/yarn-workspaces/node_modules/x",
+  "test/unit/yarn-workspaces/node_modules/x/package.json",


@guybedford I think this file was incorrectly emitted since this line is a symlink pointing to the last line in the output here. See PR #135 for the fix.

PR #129 emits `package.json` files but it should not be emit if package.json is inside a symlinked directory. This is a common pattern for yarn workspaces. This PR fixes yarn workspaces and adds a new test for yarn workspaces using ESM imports/exports. Co-authored-by: Guy Bedford <guybedford@gmail.com>

This PR fixes a bug where TS input was not considering ESM with `type: module` in package.json Related to #129 which added package.json emission but only for JS input.

guybedford requested review from lucleray, styfle and Timer as code owners June 26, 2020 03:30

styfle reviewed Jun 26, 2020

View reviewed changes

src/node-file-trace.js Outdated Show resolved Hide resolved

styfle reviewed Jun 26, 2020

View reviewed changes

src/node-file-trace.js Outdated Show resolved Hide resolved

styfle reviewed Jun 26, 2020

View reviewed changes

src/resolve-dependency.js Outdated Show resolved Hide resolved

styfle requested changes Jun 26, 2020

View reviewed changes

styfle mentioned this pull request Jun 26, 2020

[ci] Add 15 minute timeout #130

Merged

guybedford and others added 10 commits June 26, 2020 18:35

exports support

dc1f3de

integration fix

f022284

strict require support

22e1c50

bump

4a64d0b

enusre unit test inclusion

891344a

remove logging

48ec691

Update src/resolve-dependency.js

884eae8

Co-authored-by: Steven <steven@ceriously.com>

refactor boundary check

3f2ba34

separate exportsOnly handling

03ffb07

rebase

2eb0f2a

guybedford force-pushed the exports branch from 61c439c to 2eb0f2a Compare June 27, 2020 01:36

add missing fixtures

5cf2057

guybedford added 8 commits June 26, 2020 20:02

try legacy fallback

fe0d4e2

disable exports for integration tests

d022668

reenable exports for self resolve only

6e277d4

reenable exports on integration tests, self resolve branching too

09793d0

next attempt

5869442

package exports

338781c

self-resolution update

058045d

next bisection

eb17ecf

guybedford added 3 commits June 26, 2020 22:24

bisect

ccbcff5

next bisection

2c4e209

self resolution shift

2e5dcfa

remove debugging

f346bec

array fallback support

7ce3301

styfle reviewed Jun 29, 2020

View reviewed changes

src/resolve-dependency.js Outdated Show resolved Hide resolved

styfle reviewed Jun 29, 2020

View reviewed changes

readme.md Outdated Show resolved Hide resolved

guybedford and others added 3 commits July 1, 2020 23:38

fixup null fallback return

1f5a844

Update readme.md

5fef2c9

Co-authored-by: Steven <steven@ceriously.com>

simplify logic further

771f6f4

styfle approved these changes Jul 4, 2020

View reviewed changes

styfle added the automerge Automatically merge PR once checks pass label Jul 4, 2020

kodiakhq bot merged commit c0cfb7a into master Jul 4, 2020

kodiakhq bot deleted the exports branch July 4, 2020 00:26

styfle mentioned this pull request Jul 15, 2020

Fix package.json emit for symlinked yarn workspaces #135

Merged

styfle reviewed Jul 15, 2020

View reviewed changes

styfle mentioned this pull request Oct 20, 2022

fix: TS input when targeting ESM with type: module #297

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exports support, dynamic import from CJS support, package boundary emission #129

Exports support, dynamic import from CJS support, package boundary emission #129

guybedford commented Jun 26, 2020 •

edited

Loading

styfle left a comment •

edited

Loading

guybedford commented Jun 27, 2020

guybedford commented Jun 27, 2020

guybedford commented Jun 27, 2020

styfle commented Jun 28, 2020

guybedford commented Jun 29, 2020

styfle Jun 29, 2020 •

edited

Loading

guybedford Jul 2, 2020

styfle Jun 29, 2020

guybedford Jul 2, 2020

styfle Jul 3, 2020

guybedford Jul 3, 2020

styfle Jul 3, 2020

guybedford Jul 4, 2020

styfle left a comment

styfle Jul 15, 2020

Exports support, dynamic import from CJS support, package boundary emission #129

Exports support, dynamic import from CJS support, package boundary emission #129

Conversation

guybedford commented Jun 26, 2020 • edited Loading

styfle left a comment • edited Loading

Choose a reason for hiding this comment

guybedford commented Jun 27, 2020

guybedford commented Jun 27, 2020

guybedford commented Jun 27, 2020

styfle commented Jun 28, 2020

guybedford commented Jun 29, 2020

styfle Jun 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

styfle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guybedford commented Jun 26, 2020 •

edited

Loading

styfle left a comment •

edited

Loading

styfle Jun 29, 2020 •

edited

Loading