You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pouchdb.apache.org by "alxndrsn (via GitHub)" <gi...@apache.org> on 2025/11/20 07:12:44 UTC
[PR] Add explicit events dependency [pouchdb]
alxndrsn opened a new pull request, #9141:
URL: https://github.com/apache/pouchdb/pull/9141
This seems to be a common issue:
* https://github.com/apache/pouchdb/issues/8968
* https://github.com/apache/pouchdb/issues/8985
* https://github.com/apache/pouchdb/issues/8989
* https://github.com/apache/pouchdb/issues/9072
* https://github.com/apache/pouchdb/issues/9078
Adding this dependency in Node _seems_ to be safe (although I can't find it documented):
```sh
$ (dir="$(mktemp --directory)" && cd "$dir"; git init; yarn init; yarn add events; node -e 'const builtin = require("node:events"); const lib = require("events"); console.log(builtin===lib)')
Initialized empty Git repository in /tmp/tmp.tkpBomXAIR/.git/
➤ YN0000: · Yarn 4.9.2
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed
➤ YN0000: · Done in 0s 56ms
➤ YN0000: · Yarn 4.9.2
➤ YN0000: ┌ Resolution step
➤ YN0085: │ + events@npm:3.3.0
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed
➤ YN0000: · Done in 0s 62ms
true
```
Closes https://github.com/apache/pouchdb/issues/8968
Closes https://github.com/apache/pouchdb/issues/8985
Closes https://github.com/apache/pouchdb/issues/8989
Closes https://github.com/apache/pouchdb/issues/9072
Closes https://github.com/apache/pouchdb/issues/9078
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pouchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add browser shim for `node:events` module [pouchdb]
Posted by "alxndrsn (via GitHub)" <gi...@apache.org>.
alxndrsn commented on PR #9141:
URL: https://github.com/apache/pouchdb/pull/9141#issuecomment-4295405610
Thanks @espy, I'll see if that can be turned into a test case for this PR. Feel free to submit your own if you're motivated though - I think the repro and automated test case are likely much more significant work than the code changes that will result from the test case and fix the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pouchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add explicit events dependency [pouchdb]
Posted by "alxndrsn (via GitHub)" <gi...@apache.org>.
alxndrsn commented on PR #9141:
URL: https://github.com/apache/pouchdb/pull/9141#issuecomment-3557429957
Less convinced now:
```
> e1 = require('node:events'); e2 = require('./node_modules/events');
<ref *1> [Function: EventEmitter] {
once: [Function: once],
EventEmitter: [Circular *1],
defaultMaxListeners: [Getter/Setter],
init: [Function (anonymous)],
listenerCount: [Function (anonymous)]
}
> e1
<ref *1> [Function: EventEmitter] {
addAbortListener: [Function: addAbortListener],
once: [AsyncFunction: once],
on: [Function: on],
getEventListeners: [Function: getEventListeners],
getMaxListeners: [Function: getMaxListeners],
EventEmitter: [Circular *1],
usingDomains: true,
captureRejectionSymbol: Symbol(nodejs.rejection),
captureRejections: [Getter/Setter],
EventEmitterAsyncResource: [Getter],
errorMonitor: Symbol(events.errorMonitor),
defaultMaxListeners: [Getter/Setter],
setMaxListeners: [Function (anonymous)],
init: [Function (anonymous)],
listenerCount: [Function (anonymous)]
}
> e2
<ref *1> [Function: EventEmitter] {
once: [Function: once],
EventEmitter: [Circular *1],
defaultMaxListeners: [Getter/Setter],
init: [Function (anonymous)],
listenerCount: [Function (anonymous)]
}
> e1.EventEmitter === e2.EventEmitter
false
```
# TODO
* handle this like other browser-specific polyfills, using `import 'events'` vs `import 'node:events'`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pouchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add browser shim for `node:events` module [pouchdb]
Posted by "espy (via GitHub)" <gi...@apache.org>.
espy commented on PR #9141:
URL: https://github.com/apache/pouchdb/pull/9141#issuecomment-4252570826
Hello! 👋
We’ve extensively looked at your PR and manually tested it, and it turned out that, surprisingly, `events` is still not included as a dependency in `npm i pouchdb-browser`. We eventually found that the `events` dependency is _actively removed_ by PouchDB’s build process, because it has the same name as a node.js builtin (via `builtin-modules`), even if it is explicitly a dependency in `package.json` and not just a reference to the builtin.
You can test this via `npm run test-webpack` (which composes the individual modules’ `dependency` field in their `package.json`s), and inspect the files in `packages/node_modules/pouchdb-browser`:
- the `lib/index*` files do some form of `import events from 'events';`, so all imports of `pouchdb-events` in the various modules have been transformed into imports for `events`. However:
- the generated `package.json` depends on _neither_ `events` _nor_ `pouchdb-events`, so the `events` package is still not present in the browser.
We came up with a very hardcoded fix, left for your consideration in [this branch](https://github.com/neighbourhoodie/pouchdb/commits/add-explicit-events-dep-review-updates/). This allows `events` to be added to the `dependency` field in `package.json` files, effectively that means it gets added to `pouchdb-events` and `pouchdb-browser`. After some additional testing, we found that node will prefer its builtin, even if `events` is a proper dependency, so this approach seems fine to us. To test this specific behaviour:
```sh
mkdir tmp
cd tmp
mkdir -p node_modules/events
# edit node_modules/events/index.js to print/return something
# edit index.js to contain `const e = require('events') ; console.log(e)`
node index.js
```
This is also true for `index.mjs` and `import e from 'events'`.
To test that the fix itself actually works and `events` is now installed with `npm i pouchdb-browser` (`npm pack` is the closest equivalent for local testing, `npm i <FILEPATH>` and `npm link` both don’t do anything useful here):
```sh
# in your pouchdb repo with our change:
$ npm run test-webpack
$ cd packages/node_modules/pouchdb-browser
$ npm pack
# copy resulting tarball to some other empty directory and cd there
$ npm init # and yes all the things
$ npm i < TARBALL_FILENAME >
# inspect the `node_modules` folder, `events` is present
```
Aside from the fix, the branch also contains another commit with a small cleanup change.
We hope this helps to land this fix in the next release! Thanks for your continued work on PouchDB 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pouchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org