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

feat(node): add ppid getter for node:process #22167

Merged
merged 4 commits into from
Feb 1, 2024

Conversation

magurotuna
Copy link
Member

This commit adds ppid getter for node:process to improve Node compatibility one step further.

There is one problem though, which is that Deno.ppid, which process.ppid internally calls, is actually of type bigint although it's supposed to be number. I filed an issue for this (#22166). For the time being, explciit type conversion from bigint to number is applied to match the Node.js behavior.

Comment on lines 128 to 131
// TODO(magurotuna): Deno.ppid now is of type bigint (although its type
// declaration says it's a number). Until it's fixed, we compare
// `process.ppid` with the casted value.
// https://github.com/denoland/deno/issues/22166
Copy link
Member

Choose a reason for hiding this comment

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

Probably can be removed now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, just removed :)

@littledivy littledivy merged commit 66e6ed6 into denoland:main Feb 1, 2024
15 checks passed
@magurotuna magurotuna deleted the node-polyfill-process-ppid branch February 1, 2024 04:56
littledivy pushed a commit that referenced this pull request Feb 1, 2024
This commit adds `ppid` getter for `node:process` to improve Node
compatibility one step further.

There is one problem though, which is that `Deno.ppid`, which
`process.ppid` internally calls, is actually of type `bigint` although
it's supposed to be `number`. I filed an issue for this (#22166). For
the time being, explciit type conversion from `bigint` to `number` is
applied to match the Node.js behavior.
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.

None yet

3 participants