Skip to content

Improve Node.js compatibility for process, fs, http, events, and navigator#6575

Merged
danlapid merged 3 commits intomainfrom
dlapid/moreNodeCompat
Apr 16, 2026
Merged

Improve Node.js compatibility for process, fs, http, events, and navigator#6575
danlapid merged 3 commits intomainfrom
dlapid/moreNodeCompat

Conversation

@danlapid
Copy link
Copy Markdown
Collaborator

Add missing properties and constants identified by the node_compat matrix to close gaps between workerd and Node.js. All changes are additive — they either populate previously missing properties with Node.js-compatible values or fix export wiring so existing values are reachable. I believe no compat flag should be needed because:

  • New properties on existing objects (process.config.variables, stdio socket-like properties, EventEmitter.domain, globalAgent options fields) cannot break existing code since the keys didn't exist before, and all values match Node.js defaults.
  • process.config previously had empty target_defaults/variables objects, so code reading specific config keys got undefined before and gets the correct value now. Code checking for emptiness is unlikely since the empty stub was useless.
  • fs.promises.constants was missing due to an export wiring issue — the constants existed in the internal module but weren't re-exported through the promises entry point.
  • The HTTP/HTTPS Agent constructor changes align with Node.js's actual initialization order (setting defaultPort/protocol/path/proxyEnv on this.options), which fixes missing keys on agent.options without changing any observable behavior for existing Agent usage.

Specific changes:

  • process.config: Populate with representative Node.js build config stubs
  • process.stdout/stderr/stdin: Add socket-like properties (bytesRead, connecting, readyState, writable*, readable*, etc.)
  • fs.promises.constants: Re-export constants from internal_fs_promises
  • http/https Agent: Match Node.js constructor flow for options.path, options.proxyEnv, options.defaultPort, options.protocol; add agentKeepAliveTimeoutBuffer and maxCachedSessions
  • EventEmitter: Set domain = null in init() matching Node.js
  • navigator.platform: Add read-only property returning "Linux"
  • module.prototype.isPreloading: Add boolean stub
  • events: Export captureRejections as named export

@danlapid danlapid requested review from a team as code owners April 13, 2026 23:09
@danlapid danlapid requested review from guybedford and jasnell April 13, 2026 23:11
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 13, 2026

UnknownError: ProviderInitError

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 13, 2026

@danlapid Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@danlapid danlapid force-pushed the dlapid/moreNodeCompat branch 2 times, most recently from 38e9619 to 29fe1d8 Compare April 13, 2026 23:48
@danlapid danlapid requested a review from a team as a code owner April 13, 2026 23:48
@danlapid danlapid requested a review from vicb April 13, 2026 23:48
@danlapid danlapid force-pushed the dlapid/moreNodeCompat branch from 29fe1d8 to 275eb25 Compare April 14, 2026 00:00
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 14, 2026

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing dlapid/moreNodeCompat (83053d0) with main (78dfd27)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@danlapid
Copy link
Copy Markdown
Collaborator Author

/bonk review

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

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

Review Summary

Overall this PR is a clean batch of additive Node.js compat stubs. A few items worth considering:

  • [LOW] navigator.platform: Added unconditionally while language/languages are gated behind enableNavigatorLanguage. Confirm this is intentional — the spec says platform is deprecated but returning "" is the recommended deprecation path, not "Linux".
  • [LOW] process.config.variables.node_shared_merve: Likely a typo — this field doesn't exist in Node.js's actual process.config.
  • [LOW] export { constants } placement in internal_fs_promises.ts: Interleaved between import statements — technically valid but unusual; consider grouping with other exports.
  • [LOW] process.config blob: ~100 lines of hardcoded values that may silently drift from Node.js over time.
  • [INFO] The CodSpeed regression on simpleStringBody[Response] (-25%) is flagged as "different runtime environments" — likely CI noise unrelated to this PR.
  • [INFO] The PR description mentions "events: Export captureRejections as named export" but this was already exported via export * in events.ts. No issue, just a documentation inaccuracy.

This review was generated by an AI assistant and may contain inaccuracies.

Comment thread src/workerd/api/global-scope.h
Comment thread src/node/internal/internal_process.ts
Comment thread src/node/internal/internal_process.ts
Comment thread src/node/internal/public_process.ts
Comment thread src/node/internal/internal_fs_promises.ts
Comment thread src/node/internal/internal_https_agent.ts
Comment thread src/node/internal/events.ts
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 14, 2026

Review posted successfully on PR #6575. Here's a summary of the findings:

7 inline comments across the changed files:

  1. global-scope.h:118navigator.platform is added unconditionally while language/languages are behind a compat flag. The spec deprecates platform and recommends returning "". Adding "Linux" unconditionally is a new observable property on all workers, not just nodejs_compat.

  2. internal_process.ts:284node_shared_merve is likely a typo. This key doesn't exist in Node.js's actual process.config.variables.

  3. internal_process.ts:233 — The ~100-line process.config blob would benefit from a comment noting which Node.js version it was sourced from, and some values may be misleading in the workerd context (e.g., v8_enable_webassembly, v8_enable_inspector).

  4. public_process.ts:107 — The addSocketProperties helper assigns readable-prefixed properties on SyncWriteStream (which extends Writable, not Duplex). Confirmed this shouldn't conflict since those properties don't exist on the Writable prototype, but asked for test confirmation.

  5. internal_fs_promises.ts:67export { constants } is interleaved between import statements. Suggested keeping the re-export in the public promises.ts module for cleaner module boundary separation.

  6. internal_https_agent.ts:23 — The ??= (nullish) vs || (falsy) inconsistency between HTTPS and HTTP agent defaultPort initialization. Also noted the shallow copy behavior.

  7. events.ts:256domain = null on every EventEmitter instance matches Node.js but asked about test coverage.

github run

Comment thread src/workerd/api/global-scope.h Outdated
Comment thread src/node/fs/promises.ts
Comment thread src/node/internal/public_process.ts
Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Approved for typings changes on behalf of Wrangler team

Comment thread src/node/internal/internal_process.ts Outdated
Comment thread src/node/internal/internal_process.ts Outdated
Comment thread src/node/internal/internal_process.ts Outdated
Comment thread src/node/internal/internal_process.ts Outdated
Comment thread src/node/module.ts Outdated
danlapid and others added 2 commits April 16, 2026 01:40
…gator

Add missing properties and constants identified by the node_compat matrix
to close gaps between workerd and Node.js. All changes are additive — they
either populate previously missing properties with Node.js-compatible values
or fix export wiring so existing values are reachable. I believe no
compat flag should be needed because:

- New properties on existing objects (process.config.variables, stdio
  socket-like properties, EventEmitter.domain, globalAgent options fields)
  cannot break existing code since the keys didn't exist before, and all
  values match Node.js defaults.
- process.config previously had empty target_defaults/variables objects,
  so code reading specific config keys got undefined before and gets the
  correct value now. Code checking for emptiness is unlikely since the
  empty stub was useless.
- fs.promises.constants was missing due to an export wiring issue — the
  constants existed in the internal module but weren't re-exported through
  the promises entry point.
- The HTTP/HTTPS Agent constructor changes align with Node.js's actual
  initialization order (setting defaultPort/protocol/path/proxyEnv on
  this.options), which fixes missing keys on agent.options without
  changing any observable behavior for existing Agent usage.

Specific changes:
- process.config: Populate with representative Node.js build config stubs
- process.stdout/stderr/stdin: Add socket-like properties (bytesRead,
  connecting, readyState, writable*, readable*, etc.)
- fs.promises.constants: Re-export constants from internal_fs_promises
- http/https Agent: Match Node.js constructor flow for options.path,
  options.proxyEnv, options.defaultPort, options.protocol; add
  agentKeepAliveTimeoutBuffer and maxCachedSessions
- EventEmitter: Set domain = null in init() matching Node.js
- navigator.platform: Add read-only property returning "Linux"
- module.prototype.isPreloading: Add boolean stub
- events: Export captureRejections as named export
@danlapid danlapid force-pushed the dlapid/moreNodeCompat branch from df658b4 to 95d305f Compare April 16, 2026 00:40
Co-authored-by: Guy Bedford <guybedford@gmail.com>
Co-authored-by: Dan Lapid <dan.lapid@gmail.com>
@danlapid danlapid force-pushed the dlapid/moreNodeCompat branch from 95d305f to 83053d0 Compare April 16, 2026 01:08
@danlapid danlapid enabled auto-merge (squash) April 16, 2026 01:35
@danlapid danlapid merged commit 22c1ca0 into main Apr 16, 2026
22 checks passed
@danlapid danlapid deleted the dlapid/moreNodeCompat branch April 16, 2026 01:40
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.

4 participants