Proposal: Break the dependencies on node_io and dart:io by using package:file.

[jakemac53]

Today we leak the FileSystemEntity api from dart:io or package:node_io respectively out to users, even though they don't all need this functionality. This also means the public api of this package actually changes on the node platform, which is less than ideal.

I propose that we migrate the list api to use package:file, and accept an optional require a file system as a parameter. We would also rename these apis to listFileSystem By default it would use the dart:io file system and if not available it would throw.

We will also provide an extension method in a separate library in this package, which can be used to glob the local file system without providing it explicitly, and this would match exactly the current api (list, listSync).

This would be a breaking change, and would require a new dependency on package:file. We would release this with null safety. The premise is that package:file is at least a more general package, and better than leaking a node specific dep to all downstream users.

cc @nex3 for thoughts as well since I believe that Sass is the primary user of the Node functionality.

[jakemac53]

The primary alternative here is to drop Node support entirely by default. I would also move the list api to a new library in this package, which would be added to Glob via an extension.

Then some other package could add a similar extension for Node using node_api.

[nex3]

Moving Glob.list() to another package would presumably degrade the performance as well as developer experience, since the ListTree instance could no longer be cached on the glob itself across multiple invocations.

Why not just have this package depend on file?

[natebosch]

Moving Glob.list() to another package would presumably degrade the performance as well as developer experience, since the ListTree instance could no longer be cached on the glob itself across multiple invocations.

We're going to move it to a different library either way as an extension method. If we need to we'll use an Expando for caching.

Being able to have a real cross-platform implementation of Glob.matches is a priority.

Why not just have this package depend on file?

We're hoping to avoid additional dependencies, although that one is certainly easier for us to handle than node_io.

[nex3]

We're going to move it to a different library either way as an extension method. If we need to we'll use an Expando for caching.

Expandos are fairly expensive in their own right.

We're hoping to avoid additional dependencies, although that one is certainly easier for us to handle than node_io.

Why avoid additional dependencies? It's not like file brings in any huge number of transitive deps. The whole point of having a package management system is to make the marginal cost of a single additional dependency low enough to encourage code to be shared, especially for packages like file that define a kind of protocol for sharing logic across packages and platforms.

[jakemac53]

Moving it out to another library means that users don't have to pay the build time penalty for that dependency during development unless they import the extra library.

I think the expando solution would be a pretty reasonable compromise, although I haven't looked at all the details here.

[nex3]

Breaking up every package into absolutely minimal functional units in the name of "only downloading what you actually use" will ultimately produce much more developer pain than the marginal cost of an extra 300kb, as you can see in the npm ecosystem. Do what you're going to do, but you asked for my thoughts and they're "don't split out this functionality".

[jakemac53]

Thanks for your feedback.

I am in favor of the package:file approach here generally, I need to look a bit more into what that actually entails so I will start investigating the impacts that would have on the API.

As far as a separate library goes there are other reasons to prefer that as well, such as moving runtime failures to static failures. Today on the web you can use the list api but it will throw at runtime. I would like to make that fail statically, and the easiest way to do that is by making the library require dart:io, which isn't allowed on the web.

So this is less about splitting it up into small parts and more about separating out the platform specific bits in a way that is easier to reason about.

[nex3]

But if you use package:file, .list() isn't platform-specific anymore... it could just as easily be used with a JS FileSystem.

[natebosch]

But if you use package:file, .list() isn't platform-specific anymore... it could just as easily be used with a JS FileSystem.

The current design leaves .listFileSystem on Glob which takes a FileSystem argument. The .list() API which specializes it to use the disk and not require a FileSystem argument would be in the extension method with that approach.

[nex3]

I definitely like that better than just having no listing at all in the base package. At that point, though, why even bother having a separate package that just contains a one-line extension method?

[jakemac53]

The extension exists in a new library under the glob package, as I have it now. The local file system from package:file unconditionally imports dart:io and wouldn't be compatible on the web, node, etc.

The extension method library doesn't have a ton of value but it does make the migration easier and eliminates some boilerplate of constantly creating and passing a local file system.

[nex3]

Oh, I see—in that case, I'm on board with the design 😃.

[jakemac53]

Closing this as it has landed in the null safety branch.