Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

"exports" blocks require of package.json #445

Closed
ljharb opened this issue Nov 25, 2019 · 20 comments
Closed

"exports" blocks require of package.json #445

ljharb opened this issue Nov 25, 2019 · 20 comments

Comments

@ljharb
Copy link
Member

ljharb commented Nov 25, 2019

require('package/package.json') throws for me with a "package exports do not define subpath" error.

Was this intended by the feature? It seems to me like package.json is/should be a special case, and should be implicitly available by default.

@bmeck
Copy link
Member

bmeck commented Nov 25, 2019

Why is it exceptional?

@ljharb
Copy link
Member Author

ljharb commented Nov 25, 2019

It defines, for example, type: module, and exports, and a number of other things that affect behavior observed from outside the package.

@bmeck
Copy link
Member

bmeck commented Nov 25, 2019

@ljharb isn't the privacy feature of exports exactly to prevent such snooping?

@ljharb
Copy link
Member Author

ljharb commented Nov 25, 2019

on the code, yes - not imo on package.json itself. That's certainly an argument that could be made, ofc, but I think the ecosystem generally assumes that a package's package.json is requireable.

@MylesBorins
Copy link
Member

MylesBorins commented Nov 25, 2019 via email

@GeoffreyBooth
Copy link
Member

I think the spec is clear. Only the defined exports are available. If the package author wants package.json to be available, an export needs to be defined for it. I don't see why it should be a special case.

You can always work around it by using a non bare specifier path.

@ljharb
Copy link
Member Author

ljharb commented Nov 25, 2019

Alright, that seems reasonable; it just seemed strange to me, and I wanted to confirm it's intentional.

@jaydenseric
Copy link

jaydenseric commented Apr 3, 2020

Assuming you have exports like this:

{
  "module": "lib/index.mjs",
  "main": "lib",
  "exports": {
    "import": "./lib/index.mjs",
    "require": "./lib/index.js"
  }
}

So that consumers can do this:

require('extract-files/package.json')

And also if in the case of experimental JSON modules:

import pkg from './package.json'

What needs to be changed?

It would be good for this to be documented at https://nodejs.org/api/esm.html#esm_conditional_exports as this could become a major gotcha as the community adopts conditional exports; for example see jaydenseric/apollo-upload-client#186 .

I tried to figure it out reading the docs, but it's not exactly intuitive and it's good to be sure of the approach before rolling it out across dozens of packages.

@ljharb
Copy link
Member Author

ljharb commented Apr 3, 2020

you’d need to add a “./package” or “./package.json” key, or both, to exports.

@jaydenseric
Copy link

I already knew that, what was not obvious was exactly how it should look.

@ljharb
Copy link
Member Author

ljharb commented Apr 3, 2020

@jaydenseric
Copy link

So like this?

{
  "module": "lib/index.mjs",
  "main": "lib",
  "exports": {
    "import": "./lib/index.mjs",
    "require": "./lib/index.js",
    "./package": "./package.json",
    "./package.json": "./package.json"
  }
}

Are there any edge-cases or tradeoffs?

@jaydenseric
Copy link

If you have just the "./package.json" one, what happens if someone does:

require('extract-files/package')

Will it see there is no .js file to resolve, then try to check for .json before, or after the exports rules? Is the "./package" one really necessary?

@ljharb
Copy link
Member Author

ljharb commented Apr 3, 2020

When the exports field exists, it won’t do normal resolution, so yes, it’s necessary.

@guybedford
Copy link
Contributor

When specifying the main along with other subpaths, the main is referenced by ".":

{
  "module": "lib/index.mjs",
  "main": "lib",
  "exports": {
    ".": {
      "import": "./lib/index.mjs",
      "require": "./lib/index.js"
    },
    "./package": "./package.json",
    "./package.json": "./package.json"
  }
}

The exports map is a "source of truth" for resolution. Any further checking of the file system is not necessary to determine the final path.

@FredKSchott
Copy link

FredKSchott commented Nov 9, 2020

Wanted to bump this thread because we now have some real-world experience with packages causing unexpected breaking changes when they release an "exports" map upgrade. Here's the work flow that seems to repeat across all of them:

  • package releases export map, usually as a patch or minor release
  • tooling ecosystem immediately breaks
  • package authors are alerted to the break they just released
  • package authors add "package.json" to their export map as a quick fix, and re-release

I'd like to argue three points that I haven't seen brought up in this thread:

  1. The result of this is that package authors are adding "package.json" or "*"-type export map entries to their packages, which codifies an implicit side-effect as explicit behavior. This is a worse end-result than leaving this behavior undefined (as it is pre-export map).
  2. A non-trivial number of tools work by reading package manifests. This is unrelated to the defined interface of a package, and disconnected from the goals of the package author, which are to define their package interface. most of the confusion around this feature seems to stem from this misalignment of goals/understanding.
  3. I am sympathetic to the attempts to add a separate resolveRoot API, but given that no such workaround exists today I don't believe that this is a feasible solution unless it can be backported to Node v12 & Node v14.

In the interest of preventing constant ecosystem thrash over the next 1+ years as this feature is adopted, I would recommend either making the package.json an implicit member of the export map until a resolveRoot API lands OR allowing require('*/package.json') and/or require.resolve('*/package.json') to always succeed in CJS (can continue to fail in ESM imo, since this is mostly focused on backwards compat).

@ljharb
Copy link
Member Author

ljharb commented Nov 9, 2020

@FredKSchott adding "exports" should always be semver-major at this point; with the deprecation of slash exports, it's impossible to make it be non-breaking. Adding package.json to "exports" doesn't fix that.

@jkrems
Copy link
Contributor

jkrems commented Nov 9, 2020

I don't think that adding either a package.json exception or a new API will happen in time to really address ecosystem pain. My gut feeling is that an ecosystem package implementing "get package metadata relative to url" would be the more reliable strategy. That would also remove any dependency on exact node versions the user is running.

@FredKSchott
Copy link

FredKSchott commented Dec 13, 2020

FYI we may have uncovered a case where a userland fix is impossible (if package does not have . or ./package.json defined in their export map): FredKSchott/snowpack#1954 (comment). Would love any ideas to solve if this group has any

@ljharb
Copy link
Member Author

ljharb commented Dec 13, 2020

@FredKSchott you're correct; there's no robust solution there that I'm aware of when package.json, or a main, isn't specified in package.json.

While a package that lacks a "main" (or a . in exports) is exceedingly rare, it's certainly a nonzero occurrence.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants