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

fix: remove p-map dependency #335

Merged
merged 1 commit into from
May 16, 2022

Conversation

abetaev
Copy link
Contributor

@abetaev abetaev commented May 14, 2022

this module depends on node API making @libp2p/kad-dht unusable in browser.

Copy link
Member

@wemeetagain wemeetagain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wemeetagain wemeetagain merged commit b50039d into libp2p:master May 16, 2022
github-actions bot pushed a commit that referenced this pull request May 16, 2022
### [1.0.12](v1.0.11...v1.0.12) (2022-05-16)

### Bug Fixes

* remove p-map dependency ([#335](#335)) ([b50039d](b50039d))
@github-actions
Copy link

🎉 This PR is included in version 1.0.12 🎉

The release is available on:

Your semantic-release bot 📦🚀

@achingbrain
Copy link
Member

making @libp2p/kad-dht unusable in browser

This module is used in the browser by libp2p and ipfs, tested in CI, etc so this may not solve your problem. What's the error message you are seeing?

@abetaev
Copy link
Contributor Author

abetaev commented May 17, 2022

@achingbrain

this may not solve your problem

actually, it did,

What's the error message you are seeing?

p-map -> aggregate-error -> clean-stack - there's import of os module.
particularly in firefox i saw this stacktrace before change:

Uncaught Error: Module "os" has been externalized for browser compatibility and cannot be accessed in client code.
    get os:3
    <anonymous> index.js:6
[os:3:10](browser-external:os)
    get os:3
    <anonymous> index.js:6
    InnerModuleEvaluation self-hosted:2331
    InnerModuleEvaluation self-hosted:2331
    evaluation self-hosted:2292

This module is used in the browser by libp2p and ipfs, tested in CI

i did not look into browser tests, but i remember having a thought of surprise about that. all browser tests pass in libp2p and in this package, but i am getting errors in the real browser.

@achingbrain
Copy link
Member

achingbrain commented May 17, 2022

In clean-stack the os import is replaced with false by the browser key in package.json - how ever you are consuming the code from this module (bundler, etc) may not be respecting browser overrides.

@abetaev
Copy link
Contributor Author

abetaev commented May 18, 2022

@achingbrain you are right, i did a little digging...

vite replaces the actual import with that exception while in dev mode. in production mode it replaces it with an empty object.

it is, probably, a matter of taste, but the way vite handles "externalized" modules seems fair to me: if not treated properly in code, they might cause undefined behavior.

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

Successfully merging this pull request may close these issues.

3 participants