feat(ext/node): implement FileHandle.appendFile(data[, options])#31301
feat(ext/node): implement FileHandle.appendFile(data[, options])#31301bartlomieju merged 3 commits intodenoland:mainfrom
Conversation
|
|
a52e548 to
43e57c6
Compare
|
My patch only modifies The CI failures seem to come from the lint/debug and test/debug jobs failing before my code is executed, so I believe they're unrelated to this change. Could you please confirm whether these CI issues are upstream? Thanks! |
|
@SravanthDev If you run |
43e57c6 to
275b663
Compare
| return fsCall(fsyncPromise, "fsync", this); | ||
| } | ||
|
|
||
| readLines(options: { encoding?: string } = {}): AsyncIterableIterator<string> { |
There was a problem hiding this comment.
Why did you change the readLines implementation? The original one seems working correctly.
There was a problem hiding this comment.
Please revert the deleted test files
| async appendFile( | ||
| data: string | ArrayBufferView | ArrayBuffer | DataView, | ||
| options?: string | { encoding?: string; mode?: number; flag?: string }, | ||
| ): Promise<void> { | ||
| const resolvedOptions = | ||
| typeof options === "string" ? { encoding: options } : (options ?? {}); | ||
|
|
||
| const optsWithAppend = { | ||
| ...resolvedOptions, | ||
| flag: resolvedOptions.flag ?? "a", | ||
| }; | ||
|
|
||
| return this.writeFile(data, optsWithAppend); | ||
| } |
There was a problem hiding this comment.
Would you please wrap this using fsCall like the other methods? I also prefer if the implementation mimics the Node'js one: https://github.com/nodejs/node/blob/9ca46de766c00d9a7b9e72b51004b8a08d854488/lib/internal/fs/promises.js#L1263-L1268
Thanks
b2fceaf to
a8f14a6
Compare
|
Updated the appendFile implementation as requested:
Everything is now aligned with your review comments. |
|
Thanks, looks good to me. Would you please add a relevant test on |
|
I've added a dedicated test case for FileHandle.appendFile under tests/unit_node/fs_test.ts as requested. |
…oland#31301) Part of denoland#25554 This PR adds support for the missing `FileHandle.appendFile()` API in the Node.js compatibility layer.
📌 Related Issue
Part of #25554
feat(ext/node): implement
FileHandle.appendFile(data[, options])This PR adds support for the missing
FileHandle.appendFile()API in the Node.jscompatibility layer.
What’s Implemented
FileHandle.appendFile()as a thin wrapper aroundFileHandle.writeFile().flag: "a") when no flag is provided.stringUint8ArrayArrayBufferViewoptions.flagfor full Node.js API parity.Tests Included
1) Text append
Start→Start Endusing: