-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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(ext/node): Promisify fstat
and implement FileHandle#stat
#26719
base: main
Are you sure you want to change the base?
Conversation
bd5b907
to
bc98058
Compare
@@ -152,7 +161,7 @@ function fsCall(fn, handle, ...args) { | |||
}); | |||
} | |||
|
|||
return fn(handle, ...args); | |||
return fn(handle.fd, ...args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a test as well for fh.writeFile()
to make sure this change is OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rather let fstat
accept FileHandle
, which improves the compatibility further (See this change for reference #25555 )
This fsCall
util comes from this part in Node.js ( https://github.com/nodejs/node/blob/ccac4ee19d508abaf38bbabb87288ddeec7fcc21/lib/internal/fs/promises.js#L452-L470 ). I think it would be better to keep the code as aligned to node.js as possible.
e3b22c0
to
e369177
Compare
e369177
to
a70a10c
Compare
fstat
and implement FileHandle#stat
Oops, seeing #24391. Feel free to close. Although, I don't think that PR handles checking the whether the file is open prior to executing the fs operations. Probably should use the |
Fixes #23301
Implements
FileHandle.stat
. This PR also adds a promisifiedfs.fstat
under 'fs/promises', which is used internally byFileHandle.stat
.