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

fix(zone.js): add missing APIs to Node.js fs patch #54396

Closed
wants to merge 1 commit into from

Conversation

arturovt
Copy link
Contributor

This commit updates the list of Node.js fs APIs to be patched because
they haven't been updated for a long time. It adds opendir,lutimes,writev.
For example, the opendir method was added to Node.js in version 12.12.0 in
2019, causing some of the APIs to potentially be always called within the
<root> context.

Note: There are missing unit tests for these changes because in unit tests,
fs is patched by Bazel's Node.js rules and its node_patches.cjs. However,
the APIs are successfully patched in the real production code and are called
with the correct context.

@arturovt arturovt marked this pull request as ready for review February 12, 2024 18:45
@dylhunn dylhunn added area: zones area: docs Related to the documentation labels Feb 27, 2024
@ngbot ngbot bot added this to the Backlog milestone Feb 27, 2024
@arturovt arturovt force-pushed the fix/zone.js-fs-missing-apis branch from 2354b5b to f49a7ac Compare April 14, 2024 20:17
@angular-robot angular-robot bot removed the area: docs Related to the documentation label Apr 14, 2024
Copy link
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks.

This commit updates the list of Node.js `fs` APIs to be patched because
they haven't been updated for a long time. It adds `opendir,lutimes,writev`.
For example, the `opendir` method was added to Node.js in version 12.12.0 in
2019, causing some of the APIs to potentially be always called within the
`<root>` context.

**Note:** There are missing unit tests for these changes because in unit tests,
`fs` is patched by Bazel's Node.js rules and its `node_patches.cjs`. However,
the APIs are successfully patched in the real production code and are called
with the correct context.
@arturovt arturovt force-pushed the fix/zone.js-fs-missing-apis branch from f49a7ac to 2a83d24 Compare May 2, 2024 19:05
@AndrewKushnir AndrewKushnir added target: patch This PR is targeted for the next patch release target: rc This PR is targeted for the next release-candidate action: global presubmit The PR is in need of a google3 global presubmit and removed target: rc This PR is targeted for the next release-candidate labels May 3, 2024
@AndrewKushnir
Copy link
Contributor

Presubmit (TGP).

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: global presubmit The PR is in need of a google3 global presubmit labels May 3, 2024
@AndrewKushnir
Copy link
Contributor

Caretaker note: TGP is "green", this PR is ready for merge.

@AndrewKushnir AndrewKushnir added target: rc This PR is targeted for the next release-candidate and removed target: patch This PR is targeted for the next patch release labels May 3, 2024
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 9e07b62.

AndrewKushnir pushed a commit that referenced this pull request May 3, 2024
This commit updates the list of Node.js `fs` APIs to be patched because
they haven't been updated for a long time. It adds `opendir,lutimes,writev`.
For example, the `opendir` method was added to Node.js in version 12.12.0 in
2019, causing some of the APIs to potentially be always called within the
`<root>` context.

**Note:** There are missing unit tests for these changes because in unit tests,
`fs` is patched by Bazel's Node.js rules and its `node_patches.cjs`. However,
the APIs are successfully patched in the real production code and are called
with the correct context.

PR Close #54396
@arturovt arturovt deleted the fix/zone.js-fs-missing-apis branch May 3, 2024 15:19
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: zones merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants