Skip to content

feat(node:events): Add EventEmitter.AsyncIterator, fix node:events.{on, once} - #2571

Closed
ThatOneBro wants to merge 6 commits into
mainfrom
derrick/fix/event-emitter-async-iter
Closed

ThatOneBro wants to merge 6 commits into
mainfrom
derrick/fix/event-emitter-async-iter

Conversation

@ThatOneBro

@ThatOneBro ThatOneBro commented Apr 5, 2023 •

Copy link
Copy Markdown
Contributor

This PR is pretty much ready to go.

There are a few more tests I'll take a few mins to uncomment but seems like for the most part they should easily pass after working through most of the kinks.

TODO: (in another PR)

  • Eventually use a lot more builtins.
  • node:events.once
  • Fix readline.AsyncIterator to use this under the hood. I don't think it will work currently until we merge Add process.stdin.setRawMode #2046 though, so I'll probably put that in that PR

@github-actions

github-actions Bot commented Apr 5, 2023 •

Copy link
Copy Markdown
Contributor

✅ prettier errors have been resolved. Thank you.

#e593b8c7ab570b9ba61631b34f1ee43425933878

@github-actions

github-actions Bot commented Apr 6, 2023 •

Copy link
Copy Markdown
Contributor

❌ @ThatOneBro 3 files with test failures on linux-x64:

  • test/js/bun/http/serve.test.ts
  • test/js/bun/resolve/png/test-png-import.test.js
  • test/js/node/child_process/child_process-node.test.js

View test output

#e593b8c7ab570b9ba61631b34f1ee43425933878

@github-actions

github-actions Bot commented Apr 6, 2023 •

Copy link
Copy Markdown
Contributor

❌ @ThatOneBro 3 files with test failures on linux-x64-baseline:

  • test/js/bun/http/serve.test.ts
  • test/js/bun/resolve/png/test-png-import.test.js
  • test/js/node/child_process/child_process-node.test.js

View test output

#e593b8c7ab570b9ba61631b34f1ee43425933878

@github-actions

github-actions Bot commented Apr 6, 2023 •

Copy link
Copy Markdown
Contributor

❌ @ThatOneBro 5 files with test failures on bun-darwin-x64-baseline:

  • test/js/bun/http/serve.test.ts
  • test/js/bun/resolve/png/test-png-import.test.js
  • test/js/bun/spawn/spawn-streaming-stdout.test.ts
  • test/js/bun/sqlite/sqlite.test.js
  • test/js/node/child_process/child_process-node.test.js

View test output

#e593b8c7ab570b9ba61631b34f1ee43425933878

@ThatOneBro
ThatOneBro requested a review from Jarred-Sumner April 6, 2023 22:33
@ThatOneBro
ThatOneBro force-pushed the derrick/fix/event-emitter-async-iter branch from 9b512c7 to f3d593c Compare April 6, 2023 22:34
@ThatOneBro

Copy link
Copy Markdown
Contributor Author

Ended up finishing events.once() implementation in this one as well. This should get node:events mostly in spec, although it seems getEventListeners() may be somewhat out of spec (see #2586). I'll look into fixing that in another PR if we want to go ahead and merge this one.

@ThatOneBro ThatOneBro changed the title feat(node:events): Add EventEmitter.AsyncIterator, fix node:events.on feat(node:events): Add EventEmitter.AsyncIterator, fix node:events.{ on, once }, Apr 7, 2023
@ThatOneBro ThatOneBro changed the title feat(node:events): Add EventEmitter.AsyncIterator, fix node:events.{ on, once }, feat(node:events): Add EventEmitter.AsyncIterator, fix node:events.{on, once} Apr 7, 2023
@paperclover

Copy link
Copy Markdown
Contributor

will be replaced by #2604

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants