fix(ext/node): add truncate method to the FileHandle class#27389
fix(ext/node): add truncate method to the FileHandle class#27389kt3k merged 6 commits intodenoland:mainfrom
truncate method to the FileHandle class#27389Conversation
|
|
truncate method to the FileHandle class
|
Hey @kt3k, sorry for pinging you, could you take a look at this PR? |
ext/node/polyfills/fs.ts
Outdated
| opendir: opendirPromise, | ||
| rename: renamePromise, | ||
| truncate: truncatePromise, | ||
| ftruncate: ftruncatePromise, |
There was a problem hiding this comment.
Node doesn't seem having fs.promises.ftruncate (No doc entry for it, and also not available in runtime) https://nodejs.org/api/fs.html
Let's not expose it
There was a problem hiding this comment.
You make a great point! I've noticed that fstat is also exposed but can't seem to find it in the doc, and fs.promises.fstat also doesn't seem to be available in the runtime. Is that an issue, and if so, is it something we would like to handle within this PR?
There was a problem hiding this comment.
fstat is also exposed but can't seem to find it in the doc, and fs.promises.fstat also doesn't seem to be available in the runtime. Is that an issue
Good point. Yes, that seems an issue to me.
, and if so, is it something we would like to handle within this PR?
Fixing it in this PR is welcome, but that's not absolutely necessary in my opinon. I created an issue for fs.promises.fstat for now #27423
There was a problem hiding this comment.
Awesome, I might take a deeper look into that one as well, see if we might have more to clean up :)
Also, the implementation for promises.ftruncate has been cleaned up 🧹
ext/node/polyfills/fs/promises.ts
Outdated
| export const opendir = fsPromises.opendir; | ||
| export const rename = fsPromises.rename; | ||
| export const truncate = fsPromises.truncate; | ||
| export const ftruncate = fsPromises.ftruncate; |
| } | ||
|
|
||
| truncate(len?: number): Promise<void> { | ||
| return fsCall(promises.ftruncate, this, len); |
There was a problem hiding this comment.
Probably we need to import ftruncatePromise directly from "ext:deno_node/_fs/_fs_ftruncate.ts"
|
Thanks for the PR! The test cases look good. I left some comments. |
truncate method to the FileHandle classtruncate method to the FileHandle class
kt3k
left a comment
There was a problem hiding this comment.
LGTM. Nice work! Thanks for your contribution
FileHandle.truncatefs.ftruncateunderfs/promiseswhich is used internally byFileHandle.ftruncate./tools/format.jsfilehandle.truncateNode documentation: click herepart of #25554