FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: fix readdir recursive sync & callback by Ethan-Arrowood · Pull Request #48698 · nodejs/node · GitHub

/ node Public

fs: fix readdir recursive sync & callback - #48698

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Ethan-Arrowood:fix/readdir-recursive-sync-and-callback
Aug 12, 2023
ConversationCommits (1)ChecksFiles changed
Merged

fs: fix readdir recursive sync & callback#48698
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Ethan-Arrowood:fix/readdir-recursive-sync-and-callback

Conversation

Ethan-Arrowood commented Jul 7, 2023 •
edited by aduh95
Loading

Copy link
Copy Markdown
Contributor

This PR fixes the broken behavior for readdir recursive sync and callback. It updates the test assertions to catch the issue in the old code, and then adds the necessary fix in the implementation.

Refs: #48640
Fixes: #48858

nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jul 7, 2023

Ethan-Arrowood commented Jul 7, 2023

Copy link
Copy Markdown
Contributor Author

The issue noted that documentation is also missing. I want to keep that contribution available for a new contributor (some folks reached out in the thread). If we don't get a response back from them in a couple of days I'll add the docs fix in here too.

anonrig reviewed Jul 7, 2023
View reviewed changes
Comment thread test/sequential/test-fs-readdir-recursive.js Outdated
Comment thread lib/fs.js Outdated

chris-sophos commented Jul 8, 2023

Copy link
Copy Markdown

Does this also address the case where dirents are missing entirely as well?

Ethan-Arrowood mentioned this pull request Jul 18, 2023
Closed
Ethan-Arrowood force-pushed the fix/readdir-recursive-sync-and-callback branch from 7bcf27f to d636303 Compare July 19, 2023 16:45

Ethan-Arrowood commented Jul 19, 2023

Copy link
Copy Markdown
Contributor Author

@That-Guy977 yes I believe it does because it asserts the length of the look up matches the expected total length.

Ethan-Arrowood requested a review from anonrig July 19, 2023 16:46
MattIPv4 mentioned this pull request Jul 20, 2023
Closed
BridgeAR approved these changes Jul 26, 2023
View reviewed changes

BridgeAR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2023

nodejs-github-bot commented Jul 26, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52933/

nodejs-github-bot commented Jul 26, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52934/

nodejs-github-bot commented Jul 26, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52935/

nodejs-github-bot commented Jul 26, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52937/

This was referenced Jul 27, 2023
Open
Open
Open
Open
debadree25 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 30, 2023

nodejs-github-bot commented Jul 30, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52991/

This was referenced Jul 31, 2023
Open
Open
Open
Open

nodejs-github-bot commented Aug 12, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53228/

nodejs-github-bot commented Aug 12, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53247/

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 12, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 12, 2023
nodejs-github-bot merged commit 27cadf5 into nodejs:main Aug 12, 2023

nodejs-github-bot commented Aug 12, 2023

Copy link
Copy Markdown
Collaborator

Landed in 27cadf5

This was referenced Aug 13, 2023
Open
Open
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
fs: fix readdir recursive sync & callback
292b9c2 
Refs: nodejs#48640
PR-URL: nodejs#48698
Fixes: nodejs#48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
fs: fix readdir recursive sync & callback
acb6da3 
Refs: #48640
PR-URL: #48698
Fixes: #48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
UlisesGascon mentioned this pull request Aug 15, 2023
Merged
RafaelGSS pushed a commit to RafaelGSS/node that referenced this pull request Aug 15, 2023
fs: fix readdir recursive sync & callback
036e9fa 
Refs: nodejs#48640
PR-URL: nodejs#48698
Fixes: nodejs#48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 16, 2023
fs: fix readdir recursive sync & callback
d7269a3 
Refs: #48640
PR-URL: #48698
Fixes: #48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 17, 2023
fs: fix readdir recursive sync & callback
3d942d9 
Refs: #48640
PR-URL: #48698
Fixes: #48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
kachick mentioned this pull request Aug 28, 2023
Closed
lux01 mentioned this pull request Sep 11, 2023
Merged

MattIPv4 commented Oct 12, 2023

Copy link
Copy Markdown
Member

👋 Asked in slack, but asking here as well for visibility, is there intent to backport this fix to the 18.x line? #48858 was a report for 18.x, and this remains broken there currently.

targos pushed a commit that referenced this pull request Oct 28, 2023
fs: fix readdir recursive sync & callback
7c758f6 
Refs: #48640
PR-URL: #48698
Fixes: #48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
targos mentioned this pull request Nov 28, 2023
Merged
richardlau mentioned this pull request Feb 15, 2024
Closed
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
fs: fix readdir recursive sync & callback
c6dca02 
Refs: nodejs/node#48640
PR-URL: nodejs/node#48698
Fixes: nodejs/node#48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
fs: fix readdir recursive sync & callback
a45f06b 
Refs: nodejs/node#48640
PR-URL: nodejs/node#48698
Fixes: nodejs/node#48858
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
juanarbol mentioned this pull request Nov 4, 2024
Closed
coderabbitai Bot mentioned this pull request Jun 19, 2026
Merged
coderabbitai Bot mentioned this pull request Aug 1, 2026
Merged
coderabbitai Bot mentioned this pull request Aug 22, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

benjamingr benjamingr approved these changes
anonrig anonrig approved these changes
MoLow MoLow approved these changes
BridgeAR BridgeAR approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fs#readdir(Sync) breaks when recursive + withFileTypes combined
Back | FazBrowse Home | New Git URL