Move to FileHandle based fs implementation

[ronag]

Currently we use FileHandle for the promise based version of the fs api. Would it be possible to move the callback based api to this as well? It would allow us to guard against unsafe/undefined behaviors.

This would be breaking in terms of fd no longer being an integer but an object instead. So I guess if there are any implementations that depend on this it might be a problem, e.g. if storing in a object or array and using the fd as a key. Is there maybe some hack with a symbol that can make it automatically converted into an integer when trying to cast to string or number? e.g. Symbol.toPrimitive.

Refs: #34746

[ronag]

@nodejs/fs @addaleax @jasnell

[jasnell]

We cannot completely eliminate the use of the integer fd without a proper deprecation cycle, and it's questionable even then. There is so much code out there currently relying on the current way the fs module functions that we risk breaking the planet. It would have to be a slow, methodical transition. That's not to say we can't or shouldn't, just that it cannot be abrupt.

[mscdex]

Would there be a way to create a FileHandle from a file descriptor? I know I've used stdin/stdout/stderr fds before. I would think that you might also want to be able to interact with file descriptors (open or not) received from a parent process or a binding for example.

[mscdex]

Also, would there be a way to go the other direction? Get a fd from a FileHandle for bindings that utilize them?

[jasnell]

@mscdex:

Would there be a way to create a FileHandle from a file descriptor?

Currently no but there's no reason why we can't make it possible. The FileHandle is really just a wrapper around the native fd.

would there be a way to go the other direction? Get a fd from a FileHandle for bindings that utilize them?

There already is. The FileHandle object has an fd property to retrieve the native file descriptor.

[addaleax]

We cannot completely eliminate the use of the integer fd without a proper deprecation cycle, and it's questionable even then. There is so much code out there currently relying on the current way the fs module functions that we risk breaking the planet. It would have to be a slow, methodical transition. That's not to say we can't or shouldn't, just that it cannot be abrupt.

This is true, but … in practice, people are not using file descriptors as numbers, at least if they are doing it correctly. The numeric values don’t have any meaning besides being used as references, so it’s more of an opaque identifier that can only be used by passing it to fs module functions.

I would personally estimate that if we replace file descriptors with objects that wrap around FileHandle and that are implicitly convertible to number through Symbol.toPrimitive, that should go a long way and keep 99 % of applications working just fine.

[jasnell]

@ronag... in terms of a transition path... what I would recommend is:

1. Make it possible to use a FileHandle in every API that currently receives an fd as a semver-minor.
2. Provide sync and callback versions of the non-promisified fs.open()/fs.openSync() that provide FileHandle instead of fd (this in addition to fs.promises.open() which already does).
3. Separately, deprecate the use of raw fd in those APIs

We can't yet say how long that deprecation would need to be held for but...

@addaleax...

I would personally estimate that if we replace file descriptors with objects that wrap around FileHandle and that are implicitly convertible to number through Symbol.toPrimitive, that should go a long way and keep 99 % of applications working just fine.

Possibly, but the fact that it breaks strict equality can't be forgotten. That's a big difference.

[addaleax]

There’s no realistic way to deprecate fs.open() and fs.openSync() as they are. If we do want wide adoption of FileHandle by people who don’t explicitly opt into fs.promises, I think we need to actually change the return values of these functions in a clean break.

I would personally estimate that if we replace file descriptors with objects that wrap around FileHandle and that are implicitly convertible to number through Symbol.toPrimitive, that should go a long way and keep 99 % of applications working just fine.

Possibly, but the fact that it breaks strict equality can't be forgotten. That's a big difference.

I don’t think that’s such a huge issue – people don’t care so much about numeric equality of two values as they care about whether the values refer to the same fd, and they would usually only do that in a meaningful way if they come from the same fs.open() call.

That’s not to say there aren’t issues – for example, we would need some special handling for people who do transfer fds through MessageChannels/Workers.

[jasnell]

It might just be curmudgeonly ways and traumatic stress from the Great Buffer Constructor Deprecation of years past, but it definitely gives me the nervous shakes -- which, is not to say Don't Do It, but I am imagining much angst lol.

I think the most likely area of concern is going to be around the stable stdio fd's. You are, of course, correct that anyone who is using the fds correctly today should actually see little to no impact... Then again, packages like graceful-fs have caused us endless headaches so... I'm still concerned.

Perhaps an experiment with some popular fs centric libraries would be worthwhile? Maybe change fs.open() to return a FileHandle and see if/how npm breaks (for instance)?