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.

Sign up for GitHub

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

Jump to bottom

Latest version is broken in browserify? #19

Closed
achingbrain opened this issue Apr 1, 2021 · 1 comment · Fixed by #20
Closed

Latest version is broken in browserify? #19

achingbrain opened this issue Apr 1, 2021 · 1 comment · Fixed by #20

Comments

@achingbrain
Copy link
Collaborator

Seeing this in CI:

example-browser-browserify: > example-browser-browserify@1.1.1 test /home/travis/build/ipfs/js-ipfs/examples/browser-browserify
example-browser-browserify: > test-ipfs-example
example-browser-browserify: > example-browser-browserify@1.1.1 bundle /home/travis/build/ipfs/js-ipfs/examples/browser-browserify
example-browser-browserify: > browserify src/index.js > public/bundle.js
example-browser-browserify: /home/travis/build/ipfs/js-ipfs/node_modules/web-encoding/src/lib.js:7
example-browser-browserify: export { Encoder as TextEncoder, Decoder as TextDecoder }
example-browser-browserify: ^
example-browser-browserify: ParseError: 'import' and 'export' may appear only with 'sourceType: module'
@achingbrain achingbrain mentioned this issue Apr 1, 2021
Merged
@achingbrain
Copy link
Collaborator Author

I think this is because browser points to esm and it needs to point to cjs, then use exports to override to esm for environments that support esm.

The 'node' key can be used in package.exports to supply something specifically to node versions that support package.exports - maybe that could be where the require('utils') lives?

https://nodejs.org/api/packages.html#packages_conditional_exports
browserify/browser-resolve#101 (comment)

@achingbrain achingbrain mentioned this issue Apr 1, 2021
Merged
achingbrain added a commit that referenced this issue Apr 6, 2021
@achingbrain
fix: browser field should point to cjs
eda0dcb 
> a top-level "browser" field should point to CJS files that, when bundled by a non-broken module bundler, work in a browser

browserify/browser-resolve#101 (comment)

I've changed the `browser` field to point to the `cjs` version of this module in
line with the above comment because this module is currently broken when used with
browserify.

I've also added the [util](https://www.npmjs.com/package/util) module as a dep since it's used
in the code. It would be ignored in most environments so the only cost is a slightly larger
bundle but it's likely to be included in any non-trivial bundle somewhere anyway so there's
likely to be no real-world impact.

Also:

- Adds typescript dep to tests that test typescript, otherwise tests fail with
  'cannot determine executable to run' - unless you have tsc installed globally I guess?
- Fixes a typo in a comment
- Simplifies .gitignore

Fixes #19
@achingbrain achingbrain mentioned this issue Apr 6, 2021
Merged
hugomrdias pushed a commit that referenced this issue Apr 6, 2021
@achingbrain
fix: browser field should point to cjs (#20)
3992702 
> a top-level "browser" field should point to CJS files that, when bundled by a non-broken module bundler, work in a browser

browserify/browser-resolve#101 (comment)

I've changed the `browser` field to point to the `cjs` version of this module in
line with the above comment because this module is currently broken when used with
browserify.

I've also added the [util](https://www.npmjs.com/package/util) module as a dep since it's used
in the code. It would be ignored in most environments so the only cost is a slightly larger
bundle but it's likely to be included in any non-trivial bundle somewhere anyway so there's
likely to be no real-world impact.

Also:

- Adds typescript dep to tests that test typescript, otherwise tests fail with
  'cannot determine executable to run' - unless you have tsc installed globally I guess?
- Fixes a typo in a comment
- Simplifies .gitignore

Fixes #19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: browser field should point to cjs Gozala/web-encoding
1 participant
@achingbrain