Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: rewrite process as class #450

Merged
merged 7 commits into from
Feb 12, 2025
Merged

refactor!: rewrite process as class #450

merged 7 commits into from
Feb 12, 2025

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Feb 12, 2025

We have implemented node:process with named exports to mimic global.process and allow hybrid implementation interface however real process in Node.js extends EventEmitter (process instanceof events.EventEmitter === true) and we cannot have features such as lazy getters or warning on access with just named exports. current process polyfill is also kinda messy!

with This PR:

  • process extends EventEmitter
  • stdin/stdout/stderr are lazily initialized for perf on first access (named exports from node:process deoptimizes!)
  • Dummy getters clearly separated and dummy objects are empty object
  • All optional props (that only exist in certain situations) + _* internals marked as undefined to avoid implicit behavior and also reduce bundle
  • Internal exposes single UnenvProcess class accepting env, hrtime and nextTick implementation from constructor.
  • Main entry handles named exports and provides implementation.
  • unenv/polyfill/process extends globalThis.process (if exists) with a Proxy that only fallbacks to unenv one if does not has same prop (relevant to Nitro)

hybrid presets

With this new implementation, hybrid presets such as clouflare can directly pass internal implementation to factory class:

import { Process as UnenvProcess } from "unenv/node/internal/process/process";

const process = new UnenvProcess({
  env: /* custom */,
  hrtime: /* custom */,
  nextTick: /* custom */,
});

@pi0 pi0 requested review from vicb and anonrig February 12, 2025 18:05
@pi0 pi0 marked this pull request as ready for review February 12, 2025 18:28
@pi0
Copy link
Member Author

pi0 commented Feb 12, 2025

merging to validate this method.

@vicb if you have any feedback (or was thinking to revert this altogether, we can discuss, don't worry 👍🏼 )

@pi0 pi0 merged commit a263447 into main Feb 12, 2025
2 checks passed
@pi0 pi0 deleted the feat/process-rewrite branch February 12, 2025 19:01
@pi0 pi0 mentioned this pull request Feb 12, 2025
6 tasks
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.

1 participant