fix: work with browserify #13
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
I find this somewhat unpalatable, but it does work, so I'll understand if you'd prefer it to be behind a flag. At any rate we should be able to pull it out if browserify/resolve#224 ever lands. |
|
Even when that lands, you'd still want this structure present for backwards compatibility with non-ESM node versions. |
|
I think we are more ok with dropping support for node versions that are out of LTS than we are dropping support for bundlers. |
|
I'm sure; but why bother dropping support for anyone if it's so easy to keep it. |
achingbrain
added a commit
to achingbrain/ipjs
that referenced
this pull request
Jul 15, 2021
Jest [doesn't support package exports](jestjs/jest#9771), nor does it support `browser` overrides out of the box (though it [can be configured](https://jestjs.io/docs/configuration#resolver-string)). This means it parses the stubbed files introduced in mikeal#13 as javascript, so let's just require and export the file that the stub is stubbing. This has the added bonus of also supporting old nodes that don't understand package exports. Fixes achingbrain/uint8arrays#21
Merged
mikeal
pushed a commit
that referenced
this pull request
Jul 15, 2021
Jest [doesn't support package exports](jestjs/jest#9771), nor does it support `browser` overrides out of the box (though it [can be configured](https://jestjs.io/docs/configuration#resolver-string)). This means it parses the stubbed files introduced in #13 as javascript, so let's just require and export the file that the stub is stubbing. This has the added bonus of also supporting old nodes that don't understand package exports. Fixes achingbrain/uint8arrays#21
vasco-santos
pushed a commit
to vasco-santos/ipjs
that referenced
this pull request
Jul 29, 2021
Jest [doesn't support package exports](jestjs/jest#9771), nor does it support `browser` overrides out of the box (though it [can be configured](https://jestjs.io/docs/configuration#resolver-string)). This means it parses the stubbed files introduced in mikeal#13 as javascript, so let's just require and export the file that the stub is stubbing. This has the added bonus of also supporting old nodes that don't understand package exports. Fixes achingbrain/uint8arrays#21
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Browserify does not support
"exports"in apackage.json, and nor can you use"browser"inpackage.jsonas a hint to Browserify to look in a certain place for a file.This means the output of
ipjsis 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, whichipjsuses to direct it to the/cjsfolder.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 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.