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

Multi level "exports" not supported? #250

Closed
achingbrain opened this issue Jul 13, 2021 · 4 comments
Closed

Multi level "exports" not supported? #250

achingbrain opened this issue Jul 13, 2021 · 4 comments

Comments

@achingbrain
Copy link

achingbrain commented Jul 13, 2021

The multiformats module has the following fields in it's package.json:

{
  // ...other stuff
  "main": "./cjs/src/index.js",
  "browser": {
    ".": "./cjs/src/index.js",
    "./basics": "./cjs/src/basics.js",
    "./bases/base16": "./cjs/src/bases/base16.js"
    // ...other stuff
  },
  "exports": {
    ".": {
      "browser": "./esm/src/index.js",
      "require": "./cjs/src/index.js",
      "import": "./esm/src/index.js"
    },
    "./basics": {
      "browser": "./esm/src/basics.js",
      "require": "./cjs/src/basics.js",
      "import": "./esm/src/basics.js"
    },
    "./bases/base16": {
      "browser": "./esm/src/bases/base16.js",
      "require": "./cjs/src/bases/base16.js",
      "import": "./esm/src/bases/base16.js"
    }
    // ...other stuff
  }
}

and the following directory structure:

multiformats
|-- cjs
|    +-- src
|         |-- index.js
|         |-- basics.js
|         +-- bases
|              +-- base16.js
|-- esm
|    +-- src
|         |-- index.js
|         |-- basics.js
|         +-- bases
|              +-- base16.js
|-- src
|    |-- index.js
|    |-- basics.js
|    +-- bases
|         +-- base16.js

When using browserify on a project that depends on multiformats, I see errors when trying to require anything greater than one level deep in the module:

const mf = require('multiformats')
// ok
const basics = require('multiformats/basics')
// ok
const basics = require('multiformats/basics/base16')
// Error: Can't walk dependency graph: Cannot find module 'multiformats/bases/base16' from '/path/to/index.js'
//     required by /path/to/index.js
//     at /path/to/node_modules/resolve/lib/async.js:137:35
//     at processDirs (/path/to/node_modules/resolve/lib/async.js:291:39)
//     at isdir (/path/to/node_modules/resolve/lib/async.js:300:32)
//     at /path/to/node_modules/resolve/lib/async.js:25:69
//     at FSReqCallback.oncomplete (fs.js:183:21)

Is this intentional? I can't see where this module consults the exports field (though it's been a long day) or is it unsupported?

@ljharb
Copy link
Member

ljharb commented Jul 13, 2021

resolve does not yet support the "exports" field in any way; see #224.

Regardless, for backwards compatibility, packages like multiformats should be mimicking the "exports" structure in a CJS-compatible directory structure - at which point, tools like resolve will Just Work.

@achingbrain
Copy link
Author

After digging a bit deeper this has nothing to do with the "exports" field. Browserify won't consult the browser map in package.json to see if a file should be overridden if it can't resolve the original file in the first place.

That is, you can't use browser values as a hint to Browserify as to where to look for the file for a given path that's been required in the same way you can with "exports".

One thing is that the file at the given path doesn't need to actually have any content, it just needs to exist, and node will take exports over browser so there's no danger of it importing the wrong file.

achingbrain added a commit to achingbrain/ipjs that referenced this issue Jul 14, 2021
Browserify [does not support](browserify/resolve#224) `"exports"`
in a `package.json`, and nor can you use `"browser"` in `package.json`
[as a hint to Browserify to look in a certain place for a file](browserify/resolve#250 (comment)).

This means the output of `ipjs` is not compatible with Browserify since
it expects the runtime/bundler to use the `"exports"` field to look up
files paths.

If Browserify finds a file path, it will then use the `"browser"` values
to apply any overrides, which `ipjs` uses to direct it to the `/cjs` folder.

The problem is if it can't find the file in the first place it won't use
the `"browser"` map to get the overrides.

Handily we're generating the `"browser"` field values from the `"exports"`
values so we know we have the complete set of files that the user wants to
expose to the outside world, and the paths we want people to use to access
them.

The change in this PR is to use the `"browser"` field values to [mimc the `"exports"` structure in a CJS-compatible directory structure](browserify/resolve#250 (comment))
as per @ljharb's suggestion.

For any file that we are overriding with `"browser"` values, we create an
empty file (where a resolvable file does not already exist) a the path
Browserify expects it to be at, then it'll dutifully use the `"browser"`
field to pull the actual file in.
@ljharb
Copy link
Member

ljharb commented Jul 14, 2021

New node will, yes, but old node won't use exports or browser - so the "exports" structure should still be represented in actual requireable CJS files in the filesystem.

mikeal pushed a commit to mikeal/ipjs that referenced this issue Jul 15, 2021
Browserify [does not support](browserify/resolve#224) `"exports"`
in a `package.json`, and nor can you use `"browser"` in `package.json`
[as a hint to Browserify to look in a certain place for a file](browserify/resolve#250 (comment)).

This means the output of `ipjs` is not compatible with Browserify since
it expects the runtime/bundler to use the `"exports"` field to look up
files paths.

If Browserify finds a file path, it will then use the `"browser"` values
to apply any overrides, which `ipjs` uses to direct it to the `/cjs` folder.

The problem is if it can't find the file in the first place it won't use
the `"browser"` map to get the overrides.

Handily we're generating the `"browser"` field values from the `"exports"`
values so we know we have the complete set of files that the user wants to
expose to the outside world, and the paths we want people to use to access
them.

The change in this PR is to use the `"browser"` field values to [mimc the `"exports"` structure in a CJS-compatible directory structure](browserify/resolve#250 (comment))
as per @ljharb's suggestion.

For any file that we are overriding with `"browser"` values, we create an
empty file (where a resolvable file does not already exist) a the path
Browserify expects it to be at, then it'll dutifully use the `"browser"`
field to pull the actual file in.
@achingbrain
Copy link
Author

This has been fixed by the linked ipjs PR

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

No branches or pull requests

2 participants