Skip to content
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

suggestion(fs): make std/fs Node-compatible #4313

Open
iuioiua opened this issue Feb 12, 2024 · 7 comments
Open

suggestion(fs): make std/fs Node-compatible #4313

iuioiua opened this issue Feb 12, 2024 · 7 comments
Labels
feedback welcome We want community's feedback on this issue or PR

Comments

@iuioiua
Copy link
Contributor

iuioiua commented Feb 12, 2024

The Standard Library is now on JSR, and JSR now lists the runtimes compatible with any given package. A recent request was that std/fs be made Node-compatible. We should investigate the viability of this idea and how it can be done. Ideas are welcome.

@iuioiua iuioiua added the feedback welcome We want community's feedback on this issue or PR label Feb 12, 2024
@Leokuma
Copy link

Leokuma commented Feb 14, 2024

What's the goal?

Even if std/fs came to be Node-compatible, the main FS functions (e.g. Deno.readTextFile) are not Node-compatible. So if the goal is to achieve cross-platform code, this will fall short. The only way to write cross-platform code is to write Node code, and if they are writing Node code they can use node:fs functions.

Adding Node dependencies to Deno's Std just so folks can use walkSync() doesn't sound very good to me.

@SampaioLeal
Copy link

I saw some mentions about the std scope should change to @deno/ and i agree because deno_std is made for deno and other runtimes should worry about their std modules.

Also i guess deno team is making this @std scope only because deno doesn't have modules binded to the runtime. And i think that's a good idea.

@iuioiua iuioiua changed the title todo(fs): make std/fs Node-compatible suggestion(fs): make std/fs Node-compatible Apr 9, 2024
@lowlighter
Copy link
Contributor

Imho I think it would be nice. People using std are looking for abstraction and simplicity.

It's a shame we're not able to use expandGlob for cross-runtimes compatible apps without having to use a npm package or reimplementing the hundreds of lines that std already provide. Same for other useful functions like ensureDir, emptyDir or copy.

Apart from the io calls these algorithms aren't intrinsically connected to deno itself, most of the logic is about iterating over paths, states and handling errors, etc.

The low-level calls like Deno.readDir and Deno.lstat could be abstracted instead of being called directly and it would make most of fs package compatible with node/bun without much hassle.

Also I disagree on the stance we should force developers to write node code to make compatible apps, it reinforces the idea that node is the standard and encourages people to write node-only code

@Leokuma
Copy link

Leokuma commented Oct 7, 2024

Let me summarize my only concern.

If I'm using std/fs (or any other Deno package) from inside Deno, I wouldn't like my app to pull Node dependencies, because I know they are not used and I wouldn't be able get rid of them.

As long as that concern is addressed, I'm fine.

@lowlighter
Copy link
Contributor

Maybe we could do what std/path is doing for different OS, by splitting posix/windows + an abstraction layer that auto-detects it. Here std/fs would be split by runtime, node (+ bun, indirectly) / deno + an abstraction layer.


This current issue is specifically about node compatibility in std/fs, which I think makes sense and do bring value (and it'd be nice at the same time to add utilitary functions to read/write files without being bound by runtime.). Reading, writing and listing files with cli apps is really a common use-case, and currently it's kind of pain to deal with cross-runtimes although these seem pretty basic operations.

I do agree however that each package should not introduce node dependencies just for the sake of being compatible with it, especially when there are well-defined standards (which is not the case for fs atm).

@kt3k
Copy link
Member

kt3k commented Nov 13, 2024

What's the goal?

Even if std/fs came to be Node-compatible, the main FS functions (e.g. Deno.readTextFile) are not Node-compatible.

I would suggest we would add FS APIs from Deno namespace ( https://docs.deno.com/api/deno/file-system ) to std/fs like std/fs/read-file, std/fs/read-text-file, etc

If I'm using std/fs (or any other Deno package) from inside Deno, I wouldn't like my app to pull Node dependencies

Node.js now has process.getBuiltinModule(name) which dynamically loads Node.js APIs in runtime. I think we can avoid loading Node dependencies in Deno runtime by the following structure:

if (isDeno) {
  // existing code for deno
} else if (isNode) {
  const fs = process.getBuiltinModule("node:fs");
  // ... code for node using node:fs
}

@vrugtehagel
Copy link

What's the goal?

To clarify further on this with a practical example - if I want to write a JSR package that is compatible with both Deno and Node (and Bun), then I am essentially forced to use node:fs or I need to maintain two separate implementations. Given JSR is not Deno-first (or so it seems), I found it surprising that @std/fs only supports Deno.

In other words, the backwards compatibility with Node that Deno has makes it much easier to just write a Node-first package and Deno'll probably be able to run it. Node doesn't have Deno compatibility, and so having this Deno-only @std/fs package is kind of useless for writing cross-runtime code.

The fact that @std/fs uses Deno APIs under the hood and therefore doesn't work in Node is a very valid issue, but I don't think this is something that should end up being a problem for users of the standard library. This kind of issue is exactly what package users are trying to avoid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback welcome We want community's feedback on this issue or PR
Projects
None yet
Development

No branches or pull requests

6 participants