dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
115 stars 40 forks source link

Making build/link APIs use `String`s to represent filepaths instead of `Uri`s #1506

Open mkustermann opened 1 week ago

mkustermann commented 1 week ago

We should consider making build/link APIs use Strings to represent filepaths instead of Uris. Using Uris indicates that they can be arbitrary uris (e.g. http uris) when in fact they cannot - there's assumptions throughout the code that those are filepaths.

dcharkes commented 1 week ago

Pros for Uri:

Cons for Uri:

Pros for String:

Cons for String:

Maybe we should consider using File/Directory (possibly from package:file) instead.

Pros for File / Directory from package:file:

Con for File from package:file:

HosseinYousefi commented 1 week ago

I don't really have a strong opinion as long as we're consistent between packages.

To me Uris indicate that they can be arbitrary URIs as much as Strings indicate that they can be any arbitrary text.

dcharkes commented 1 week ago

To me Uris indicate that they can be arbitrary URIs as much as Strings indicate that they can be any arbitrary text.

So you prefer File and Directory? 😄

HosseinYousefi commented 1 week ago

I'm fine with either of the options. One con for File/Directory is the fact that we need to add dependency to another package.

mkustermann commented 1 week ago

Cons for String: Requires doc comments to even specify that they are a file path, could be a file name, or some other contents.

It's usually very obvious from the name of a variable/parameter (e.g. String get resourceUsesFile) - and it applies to Uris equally - one has to know it's a file:// uri. In both cases one also may need to specify whether it can be absolute or relative. So it's really not a downside of using String at all.

Cons for String: No help for resolution (very easy to get wrong on Windows and with absolute/relative paths).

We do have package:path that is pervasively used for path manipulation in Dart. So I don't see this as a downside either.

mosuem commented 1 week ago

+1 to package:file, I don't think Strings are a great interface for non-generic text. But I am fine with any of the options.

It's usually very obvious from the name of a variable/parameter

I thought the benefit of a typed language is to not have to infer the type from the name ;)

mkustermann commented 1 week ago

I thought the benefit of a typed language is to not have to infer the type from the name ;)

Name of variables should signal what the variable represents which may at times be very related to the type of a variable. Of course one shouldn't - just for the sake of it - repeat a type name in the variable name (e.g. a packageJson and packageJsonFile are two different things - the former indicates it holds the content of the packages.json file, the ladder that it contains a filepath to a file on disc that has the contents)

If you strongly prefer to signal it in the type you can consider making an extension type on top of String.

liamappelbe commented 1 week ago

Pros for Uri:

  • Predictable behavior when resolving.

I'd prefer not to use URIs. I think their resolving logic is actually pretty confusing, because it's very opinionated about whether a path to a directory must end with a '/': https://dartpad.dev/?id=3ef2336ad73993a8991e45b6ffd0dcf1

My top choice would be Strings + package:path. I haven't used File and Directory for handling paths, but if their resolution utils are as good as package:path I'm fine with them.

mosuem commented 1 week ago

If you strongly prefer to signal it in the type you can consider making an extension type on top of String.

https://pub.dev/packages/path_type seems to do that. Not suggesting we use a 3p package, just FYI.