Add exports field to package.json

[Mrtenz]

Following discussion in #203, I removed the exports field from the package.json because it seemed unnecessary. Unfortunately, not having it breaks when importing a package when Node.js is running in ESM mode, because Node.js doesn't look at the module field at all. This causes issues like in MetaMask/key-tree#144.

This adds a two exports fields:

• .: The main export, which is used when importing a package from the root. This means that we cannot reliably import from subpaths anymore.
• ./package.json: To allow resolving the package.json path (and importing it, if desired), we also export this.

Since Browserify does not support the exports field, we can't use ./* exports (i.e., package/foo resolves to package/dist/esm/foo or package/dist/cjs/foo depending on the environment), as it would break in the extension.

[mcmire]

Just so I understand this correctly from your comment here, because Browserify ignores exports, the paths on the left-hand side of exports (aside from ".") need to be written so that they resolve to real files in the filesystem. We do this so that if the extension uses a library that contains a subpath import e.g., @metamask/key-tree/dist/cjs/some/path, will continue to work.

However, this seems to imply two things: 1) the responsibility for resolving an import path shifts from the consumer to the library itself, and 2) we cannot create fully ESM-compatible libraries.

Say we publish a library that contains a subpath import of another library. Ideally we could use a shortcut for that import (e.g. @metamask/key-tree/some/path) and it would resolve to either the CJS or ESM version depending on what the consumer is using (a bundler or straight Node). But we can't do that, so we have to be explicit and use the CJS version (@metamask/key-tree/dist/cjs/some/path).

But if we do that, wouldn't this mean that in the compiled version of our library, both the CJS and ESM builds will contain this import? So even if in our ESM build, we'd potentially have a mixture of ESM and CJS?

[Gudahtt]

Could you give an example of "a subpath import of another library"? I'm not sure what that means, or why we'd want to support it.

[Mrtenz]

But if we do that, wouldn't this mean that in the compiled version of our library, both the CJS and ESM builds will contain this import? So even if in our ESM build, we'd potentially have a mixture of ESM and CJS?

Yeah, that's the downside of doing this. I thought about doing something like this:

{
  "./dist/cjs/*": {
     "import": "./dist/esm/*.js",
     "require": "./dist/cjs/*.js"
  }
}

So even if you import ./dist/cjs/foo, and your bundler supports exports, it will resolve to ./dist/esm/foo. But this feels very hacky.

I wonder if there's a way to support exports in Browserify through a plugin, but I couldn't find anything about this. That would solve all problems I think.

[Mrtenz]

Could you give an example of "a subpath import of another library"? I'm not sure what that means, or why we'd want to support it.

For example:

• utils imports abi-utils/dist/cjs/foo
• snaps-utils imports utils.
• snaps-utils is now indirectly using abi-utils/dist/cjs/foo, even though the bundler may support ESM.

[Gudahtt]

Thanks, my brain somehow read "subpath import" as "subpath export". That makes more sense.

[Gudahtt]

Perhaps we could omit the dist/* exports, and commit ourselves to not relying on file imports anymore? We can be more diligent about updating the package exports for our libraries. That's something we've been trying to do anyway, this can serve as a good excuse.

[Mrtenz]

Perhaps we could omit the dist/* exports, and commit ourselves to not relying on file imports anymore? We can be more diligent about updating the package exports for our libraries. That's something we've been trying to do anyway, this can serve as a good excuse.

I'm not sure how much work would be involved in this, but if it's not too much, that seems like a good option too. A regex search in the Snaps repo (import .* from '@metamask/.*/.*') shows there's a couple of cases where we do this, which can be easily addressed.

[Mrtenz]

This is useful for resolving the root of the package. I'm not sure if it should be added to the module template, but it doesn't hurt I suppose?

We use this in the Snaps repo several times, which doesn't work without this line:

path.dirname(require.resolve('package/package.json'));

[Gudahtt]

Interesting. I guess it doesn't hurt 🤷

[Mrtenz]

Apparently Node.js doesn't allow using .js for both the CJS and ESM version in the same project (??), but you can work around it by adding a package.json to the ESM folder.

[mcmire]

Perhaps we could omit the dist/* exports, and commit ourselves to not relying on file imports anymore? We can be more diligent about updating the package exports for our libraries. That's something we've been trying to do anyway, this can serve as a good excuse.

I like this. Maybe it shouldn't be a blocker for this PR? But I do think it's a good idea.

[mcmire]

Looks good to me.