bug: undeclared events dependency breaks vite builds #2110
Comments
|
I don't think the steps to reproduce this issue are correct given we have multiple examples that use vite with libp2p, such as libp2p in the browser that don't have this problem, do you have another example of this occuring independently of this issue? Our build tool builds browser bundles separately and so I think this may be related to how the consumer is using rollup's tree shaking, but I can't say definitively since they are also using svelte. |
maschad
added
need/author-input
maschad
moved this from 🥞Weekly Candidates/Discuss🎙
to 🤨Needs Investigation
in js-libp2p
|
More plainly the problem is that we're using the nodejs build-in As far as I know, the eg: // add to `@libp2p/interface/events` ??
// noop if in browser
let setMaxListeners = (n?: number, ...eventTargets: (DOMEventTarget | EventEmitter)[]) => {}
try {
const {setMaxListeners: s} = await import('node:events')
setMaxListeners = s
} catch (e) {}
export {setMaxListeners}then in libp2p import {setMaxListeners} from '@libp2p/interface/events'
...
setMaxListeners(Infinity, signal) |
Normally one wouldn't list a nodejs built-in events package in one's dependencies, regardless, our bundles are generated by esbuild which outputs code for the browser by default so the consumer doesn't need to resolve that package. What's happening here is that there is extra-bundler configuration via Rollup that causing conflation, otherwise this issue would have shown up much earlier in one of our many vite projects. |
|
I think we need to add events as a dep to the modules that use it. It will not be imported in node since it doesn't allow overriding the names of built-in modules. This problem doesn't occur when we build the browser bundle in the monorepo with esbuild because kad-dht declares events as a dependency so it is hoisted and as such present and resolvable in |
Bundlers do not include node core modules any more so we need to depend on polyfills in order for downstream consumers to build their apps successfully. Fixes #2110
Bundlers do not include node core modules any more so we need to depend on polyfills in order for downstream consumers to build their apps successfully. Fixes #2110
github-project-automation
bot
moved this from 🤨Needs Investigation
to 🎉Done
in js-libp2p
The examples are self contained, and as a sanity check I ran the webRTC example independently of the monorepo with the latest libp2p and this issue still doesn't occur, and That being said, I can see the benefit of what @wemeetagain suggests in moving towards handling native ESM support on our side such that we wouldn't need to rely on bundlers with the ultimate goal of having greater portability across environments, but that would require a bit more of a discussion. |
Just do |
It's not being pulled in |
|
With vite I see a warning in the browser console: so I think they're doing a more gentle version of webpack 5's rug-pull. |
Severity:
Medium - A non-essential functionality does not work, performance issues, etc.
Description:
Since
connectionManager/utilshas events dependency to nodejs package I assume and there is no alias or guard for the browser it creates problems for consumers that are using the lib without polyfills.In particular, it happens to
vitehttps://vitejs.dev/ which by default does not any kinds of polyfills.Steps to reproduce the error:
Take any project with
viteand install to itlibp2p, try building the project and observe the problem.Additional context can be found in issue on our side - waku-org/js-waku#1564
The text was updated successfully, but these errors were encountered: