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

Package.json exports field: Differentiate between ./* and ./*/ #39994

Open
ctjlewis opened this issue Sep 4, 2021 · 18 comments
Open

Package.json exports field: Differentiate between ./* and ./*/ #39994

ctjlewis opened this issue Sep 4, 2021 · 18 comments

Comments

@ctjlewis
Copy link

ctjlewis commented Sep 4, 2021

Is your feature request related to a problem? Please describe.

Yes. I would like to be able to statically resolve entry points to path/*/index.js and path/*.js, similar to how is done in CJS, e.g. by adding a trailing slash to the match pattern like so:

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*/": "./dist/*/index.js",
  "./*": "./dist/*.js"
},

This would allow interop with traditional indexing:

import 'my-package/path/to/dir/'
// resolves to `my-package/path/to/dir/index.js`

import 'my-package/path/to/dir/module'`
// resolves to `my-package/path/to/dir/module.js`

This is currently not feasible as I understand it, and it would be necessary to import via import '.../index.js' regardless of how the exports field is configured.

Describe the solution you'd like

Modify exports field logic to differentiate between ./* and ./*/.

Allow a single pattern to have multiple matches, either via:

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*": "./dist/*/index.js",
  "./*": "./dist/*.js"
},

Or:

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*": ["./dist/*/index.js", "./dist/*.js"]
},
@ctjlewis
Copy link
Author

ctjlewis commented Sep 4, 2021

Current options:

Only resolve index

Cannot import from my-package/.../manual-entry.js , only directories with a specified index.js:

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*": "./dist/*/index.js"
},
import 'my-package/path/to/dir'
// resolves to `my-package/path/to/dir/index.js`

import 'my-package/path/to/dir/test.js'
// fails, tries to resolve to `my-package/path/to/dir/test.js/index.js` 
// would otherwise be a perfectly valid import source

Only resolve non-index (fine for now)

This approach means it is not possible to export members from some path/to/dir/index.js without needing to manually import from path/to/dir/index (thus, would be best to have some folder with a.js, b.js, and c.js instead of a folder with an index exporting members, which sucks because the latter is generally cleaner).

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*": "./dist/*.js"
},
import 'my-package/path/to/source'
// resolves to `my-package/path/to/source.js`

import 'my-package/path/to/source/index'
// only way to import from an index

@ctjlewis
Copy link
Author

ctjlewis commented Sep 4, 2021

Tbh, it is clear that the trailing slash has an effect on the logic, but not the desired one. It's possible that this may be possible, but as far as I can tell, it is not (even if you combine different specifications, like ./* and ./*/).

Maybe @ljharb can add thoughts?

WICG/import-maps#244 (comment)

@ctjlewis
Copy link
Author

ctjlewis commented Sep 4, 2021

Having reviewed the related discussions and specification a bit, I can tell the request for trailing slash support will be rejected out of hand since removing that seems to have been a core feature of ES imports.

However, we should still think about how to achieve the setup described here.

Interestingly, using a prefix works, though is super unintuitive?

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*": "./dist/*.js",
  "./index/*": "./dist/*/index.js"
},
import { test } from 'my-package/test'
// resolves to `./dist/test.js'

import { test } from 'my-package/index/path/to/module'
// resolves to `my-package/path/to/module/index.js`

This is a potential solution for now, but perhaps a better long-term solution would be allowing multiple match attempts:

Allow a single pattern to have multiple matches, either via:

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*": "./dist/*/index.js",
  "./*": "./dist/*.js"
},

Or:

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*": ["./dist/*/index.js", "./dist/*.js"]
},

@ljharb
Copy link
Member

ljharb commented Sep 4, 2021

If you're looking to auto-resolve to index.js, that's sadly a feature that's been intentionally removed when using exports. The entire design of "exports" is that there aren't "attempts", there's just a 1:1 mapping for a specifier (modulo given conditions) that will either work or fail.

@ctjlewis
Copy link
Author

ctjlewis commented Sep 4, 2021

@ljharb Not auto-resolve, but manually configure to achieve this. I have found a workaround, it is just highly un-ergonomic (see last comment).

Also, just theoretically, 100% of existing ESM imports right now resolve on the first try (by definition). Allowing an opt-in for a second try on the first failure in order to achieve this result directly would be well-worth the trade-off IMO.

I am not saying we change how ESM resolution happens. I am saying we consider allowing opt-in behavior that could achieve this, or examine the current logic for a way to achieve something similar. Right now, it is possible via the example above:

import { test } from 'my-package/test'
// resolves to `./dist/test.js'

import { test } from 'my-package/index/path/to/module'
// resolves to `my-package/path/to/module/index.js`

But it's pretty confusing. My original concept of having a trailing slash was a way to specify it statically on the first try, but that is also ruled out. This is the only way that it is currently possible and, based on the response, the only way it would ever be possible.

@ctjlewis
Copy link
Author

ctjlewis commented Sep 4, 2021

I have decided on the ./dist/*/index.js pattern for the purposes of ESM module bundling—it is more intuitive to say "only indexes are exported; refactor path/to/module.js to /path/to/module/index.js to export it" than use the above workaround.

@aduh95
Copy link
Contributor

aduh95 commented Sep 6, 2021

Allowing an opt-in for a second try on the first failure in order to achieve this result directly would be well-worth the trade-off IMO.

The idea behind this design decision IIRC was to allow someone to create a tool that reads a package.json file to generate an import maps without needing to make other lookup calls (which can be very expensive if done over the network).

Not sure if that helps, but you can make import 'my-package/path/to/dir' work if you add a file ./dist/path/to/dir.js that contains:

export * from './dir/index.js';
export { default } from './dir/index.js';

@ctjlewis
Copy link
Author

ctjlewis commented Sep 6, 2021

Thanks @aduh95—I've decided to embrace it instead, and am exporting only index resolvers.

Under this paradigm, src/member.js is "internal"; to mark it "external" and export it, it would become src/member/index.js.

As a final note, under a proposal where we recognize the trailing slash, there would be no need for multiple resolution attempts. 'importSource/' would resolve differently than 'importSource' under package.json exports.

@ljharb
Copy link
Member

ljharb commented Sep 6, 2021

@ctjlewis i'm confused why you'd ever want index.js to appear in import specifiers in external consumers though; "exports" lets you transparently map those.

@ctjlewis
Copy link
Author

ctjlewis commented Sep 6, 2021

@ljharb I don't, it's explicitly what I want to avoid. When I say "I have decided on the ./dist/*/index.js pattern", I mean:

"exports": {
  "./*": "./dist/*/index.js"
}
// resolves to my-package/dist/a/index.js
import { something } from 'my-package/a'

This is actually the only logic that lets me avoid having consumers ever manually type my-package/path/to/subdir/index.

This is all work related to a TS-ESM compiler I'm building, a TSDX fork at @tszip/tszip. From the README:

Internal vs External entry points

An import from your-package/path/to/submodule only works if
src/path/to/submodule is a folder with an index file.

tszip projects leverage package.json exports logic to automatically resolve
subdir imports for your package, which mimics something like an optimized
version of legacy resolve() logic.

Consider the following typical project structure:

src/
├── a
│   ├── index.ts
│   └── utils.ts
├── b
│   ├── index.ts
│   └── utils.ts
├── c
│   ├── index.ts
│   └── utils.ts
├── constants.ts
├── index.ts
└── utils.ts

tszip will build each of these files to output in dist/, like
dist/a/index.js, dist/a/utils.js etc. The exports configuration provides
for the following behavior:

  • modules at index files:

    • my-package/index.js
    • my-package/a/index.js
    • my-package/b/index.js, etc.

    can be imported easily via:

    • my-package
    • my-package/a
    • my-package/b, etc.
  • whereas non-index files:

    • my-package/constants.js
    • my-package/a/utils.js
    • my-package/b/utils.js, etc.

    cannot be imported, though can still be exposed by re-exporting at an index.

The main result is that index files are said to be external in that you
can import them from another ES module, and non-index files are internal
in that they are emitted as output, but cannot be imported without re-exporting
at an index.

See the following examples, where your-package is the name of the package in package.json:

/** This is a downstream module importing your package. */

// your-package is always exported as the root index
import { whatever } from 'your-package'

// your-package/a is an index file, and is exported
import { whatever } from 'your-package/a'

// error! this entry point is not an index, and is not exported
import { whatever } from 'your-package/a/utils'

This logic is an efficient compromise given the way Node.js resolves the
exports field:
#39994

See the Node.js docs for more info about conditional exports:
https://nodejs.org/api/packages.html#packages_subpath_patterns

@guybedford
Copy link
Contributor

@ctjlewis this was implemented recently in the pattern trailers PR in #39635 which was just released in 16.9.0. If you try your example in 16.9.0 it works out.

@guybedford
Copy link
Contributor

Specifically, the first example in this issue now works:

"exports": {
  "./package.json": "./package.json",
  ".": "./dist/index.js",
  "./*/": "./dist/*/index.js",
  "./*": "./dist/*.js"
},

@guybedford
Copy link
Contributor

The problem with the trailing slash pattern though is that it is not supported in import maps per the issue linked above.

For this reason I'd strongly discourage using the trailing slash here, and I'm considering if it might be worth a follow-up PR to ban this.

@ljharb
Copy link
Member

ljharb commented Sep 8, 2021

Why should node limit itself just because browsers haven't decided to support it? It's a common pattern in the JS ecosystem, and we should support it.

@guybedford
Copy link
Contributor

guybedford commented Sep 8, 2021

We do support it - via require. We haven't supported it for ES modules at any point until trailing patterns were added and released two days ago.

Correction: Actually this was supported in patterns of the form "./*": "./*index.js supporting ./x/ -> ./x/index.js mappings.

@ljharb
Copy link
Member

ljharb commented Sep 8, 2021

Thanks for clarifying. Why wouldn't we want to support it in ESM, since we support it in require?

@guybedford
Copy link
Contributor

I've corrected my comment above, there was a previous patterns edge case that did support this as well, although I don't expect it would have been widely known.

This discussion is off-topic for this thread at this point, I'm going to post up a PR shortly with the motivation and we can continue discussion there.

@ctjlewis
Copy link
Author

ctjlewis commented Sep 8, 2021

Absolutely astounding work @guybedford, crazy this just landed two days ago, had no idea! Thank you also @ljharb for your input, I agree.

I'm just trying to standardize subdir imports for ES modules compiled from TS with --module esnext, which leaves in relative imports (import stuff from './module') that they are committed to never rewriting. Would appreciate review of the Internal vs external entry points section of this compiler, if anyone has time.

If it were possible to resolve ./module to ./module.js and ./module/index.js, TypeScript's ESNext module output could be executed directly. Currently the trade-off seems to be choosing between forcing consumers to specify indexes via your-package/subdir/index for "./*": "./*.js" config and only exporting indices via "./*": "./*/index.js" config.

It does seem based off your response though Guy that, on Node 16.9+, we could export ./* as *.js and ./*/ as ./*/index.js and at least achieve both your-package/fileDotJs and your-package/indexDotJs/ resolutions. Discourages index exporting because it's weird, probably lean towards existing index resolvers only ("internal" vs "external") for sake of pure ergonomics.

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

No branches or pull requests

4 participants