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

doc: require node:process in Events examples #54221

Closed
wants to merge 1 commit into from

Conversation

mfdebian
Copy link
Contributor

@mfdebian mfdebian commented Aug 6, 2024

These CJS examples on the Events docs were missing their node:process require statements.

With this change they'd all match their ESM counterparts.

Best regards!

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Aug 6, 2024
@aduh95
Copy link
Contributor

aduh95 commented Aug 6, 2024

I don't think this is necessary, global.process and require('node:process') reference the same value, and contrarily to ESM in which we have lint rules to not rely on Node.js-specific globals, it seems fine for CJS to use those.

@mfdebian
Copy link
Contributor Author

mfdebian commented Aug 6, 2024

Oh ok! No problem! I just thought about making this change because I had previously made a similar PR that got merged, but if it's not really necessary I'll close it. Thanks for the review! 🙌

@mfdebian mfdebian closed this Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants