fs/promises FileHandle does not exist

[lucacasonato]

Deno:

> const fs = await import("node:fs/promises");
undefined
> const file3 = await fs.open("./README.md");
undefined
> file3
3
> file3
3
> file3.read
undefined

Node:

> const fs = await import("fs/promises");
undefined
>   const file3 = await fs.open("./tests/e2e_unit/testdata/file.txt");
undefined
> file3
FileHandle {
  _events: [Object: null prototype] {},
  _eventsCount: 0,
  _maxListeners: undefined,
  close: [Function: close],
  [Symbol(kCapture)]: false,
  [Symbol(kHandle)]: FileHandle {},
  [Symbol(kFd)]: 24,
  [Symbol(kRefs)]: 1,
  [Symbol(kClosePromise)]: null
}
> file3.read
[Function: read]

[k-nasa]

I will work on this

[k-nasa]

Hello,

This is my first time contributing to Deno's OSS and I have a few questions.

Initially, I thought the issue could be resolved by simply adding something to the polyfill. However, I realized that FileHandle is not yet present. I understand that we need to add this and establish a sequence of steps for use with fs/promises and other.

However, I'm currently struggling to understand how Deno achieves Node compatibility. Could you please give me an overview of the architecture regarding this?

Also, I believe this document (https://github.com/denoland/deno/tree/main/ext/node/polyfills) is outdated and may not function adequately in its current state. I felt an update might be necessary concerning this.

I would like to propose working on fixing this issue. I would appreciate any advice or feedback on how best to proceed.

Thank you in advance.

[bartlomieju]

Hey @k-nasa, thanks for interest in contributing. I will write up a longer rundown of things that need to happen to polyfill this API. I'll get back to you later tonight.

[k-nasa]

Thank you for your reply. looking forward to it. @bartlomieju

[bartlomieju]

@k-nasa so I would suggest to start very small - focus on adding FileHandle class an implementing FileHandle.readFile call.

The actual FileHandle class in Node can be seen here:
https://github.com/nodejs/node/blob/959142a4652f7b6e90743be874fe9c065ed31d99/lib/internal/fs/promises.js#L132-L247

I suggest to skip the EventEmitterMixin(JSTransferable) part for now and only make it extend EventEmitter class. You can add it in ext/node/polyfills/internal/fs/handle.ts.

Then in ext/node/polyfills/_fs/_fs_readFile.ts in readFile function you will need to start with updating the checks for path variable to check if it is an instance of FileHandle.

Once you get that set up we can think about how to actually implement the API - right now I'm thinking that FileHandle should just call Deno.open internally and store the returned resource ID that we will later use for various operations.

Let me know if that's helpful or do you need more pointers to get started.

[k-nasa]

@bartlomieju Thank you! I think this is enough information to get started. I will try to work on it tonight.

From your explanation, it seems that "node compatibility" is not achieved by using Node internally, but by a TypeScript program that provides the same interface and behaves the same way as Node. Is the Rust language included in ext/node closely related to the Deno runtime?

[bartlomieju]

From your explanation, it seems that "node compatibility" is not achieved by using Node internally, but by a TypeScript program that provides the same interface and behaves the same way as Node.

That's correct, we do not use Node.js internally, but instead provide a set of APIs that are available in Node.js but are implemented internally by Deno - the implementation is different, but the API surface is the same and we try to achieve the same behavior as in Node (though that's not always the case).

Is the Rust language included in ext/node closely related to the Deno runtime?

ext/node is using the same "op system" as the rest of Deno - if you take a look at ext/node/ops and runtime/ops you will see that the functions decorated with #[op] attribute look similar in both places. They do different things but they share a common API for calling from JavaScript to Rust.

[k-nasa]

I am working here
I'm working on a task list like this:

• Change the return value of the open function to FileHandle
  • FileHandle
    • Add type
    • Implement methods
      • readFile
  • Change the return value of open
• Change the return values of other functions that should return a FileHandle

[k-nasa]

@bartlomieju
I was wondering what is the reason for accepting FileHandle in readFile?
I thought it would be better to call ext/node/polyfills/_fs/_fs_readFile.ts from FileHandle when implementing the readFile method in FileHandle.

[k-nasa]

FileHandle has this many methods
ref https://nodejs.org/api/fs.html#class-filehandle

FileHandle needs to implement these.

• Event: 'close'
• filehandle.appendFile(data[, options])
• filehandle.chmod(mode)
• filehandle.chown(uid, gid)
• filehandle.close() feat(node_compat): Add a close method to the FileHandle class. #19357
• filehandle.createReadStream([options])
• filehandle.createWriteStream([options])
• filehandle.datasync()
• filehandle.fd
• filehandle.read(buffer, offset, length, position) feat(node_compat): Add a read method to the FileHandle class #19359
• filehandle.read([options]) feat(node_compat): Add a read method to the FileHandle class #19359
• filehandle.read(buffer[, options]) feat(node_compat): Add a read method to the FileHandle class #19359
• filehandle.readableWebStream(options)
• filehandle.readFile(options)
• filehandle.readLines([options])
• filehandle.readv(buffers[, position])
• filehandle.stat([options])
• filehandle.sync()
• filehandle.truncate(len)
• filehandle.utimes(atime, mtime)
• filehandle.write(buffer, offset[, length[, position]]) feat(node_compat): Add a write method to the FileHandle class #19385
• filehandle.write(string[, position[, encoding]]) feat(node_compat): Add a write method to the FileHandle class #19385
• filehandle.writeFile(data, options)
• filehandle.writev(buffers[, position])

[kt3k]

Created #25554 for better visibility. Closing this one as the original issue has been resolved