Berry: [Feature] Add a dedicated type for portable paths

Created on 10 May 2019  路  7Comments  路  Source: yarnpkg/berry

Describe the context

In Yarn 2, we use something called "portable paths". The idea is simple: instead of using different APIs depending on the underlying filesystem (and having to deal with potentially different separators, etc), we instead always work with posix paths - even on Windows! On those platforms we simply work with a slightly different path than the regular ones - /d:/foo instead of d:\foo, for example. Then, before they get sent to the actual consumers that need native paths, we just convert them back into their native form.

It works pretty well, but it's not always clear which APIs expect portable paths and which APIs expect native paths. We should fix that by adding types to @berry/fslib.

Describe the solution you'd like

The @berry/fslib package should export a new type called PortablePath. This type should use the same trick as the one to type our hashes, which would prevent them from being silently casted back and from native paths. Then the functions that manipulate paths would be updated to match the true type they expect rather than the generic string type (we would keep string, but only for native paths).

In order to do this, we'll also have to turn FakeFS into FakeFS<PathType>. Most implementations (for example ZipFS) would extends FakeFS<PortablePath>, but some of them (for example PosixFS) would extend FakeFS<string>.

Describe the drawbacks of your solution

It's a significant amount of changes in the source code, and might lead to some conflicts before merge, but at the same time the changes are fairly well understood and have very little risks - since we're talking about the type system and not the runtime implementation, if it typechecks, it works.

Describe alternatives you've considered

We could keep things as they are, but we've already made mistakes multiple times with a path being modified when it shouldn't have been (or the other way around), and it's definitely something our type system should prevent.

enhancement help wanted in progress

All 7 comments

Some quick notes:

  • We'll need a NativePath as well, because as PortablePath extends string our code can pass in PortablePath where APIs expect string. We can ensure string extends NativePath to allow strings to be used in the APIs that expect NativePath.
  • Berry uses path.posix _a lot_ to manipulate PortablePaths. We should provide a utility that's basically path.posix with updated typings, unless we want to cast every value returned by path.posix.foo to PortablePath everywhere

It would really be useful!

I can look at it this week if no one else is already on it

I think @bgotink started a draft exactly at the same time you posted your comment 馃槃

Yep, indeed 馃槃

I'll take a look at the PR today (more likely tonight)

Talk about timing 馃槄 I'd greatly appreciate a review! There were _a lot_ of changes, so we can use any help catching mistakes

I think we can close this now @arcanis?

Yup!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

chrisands picture chrisands  路  3Comments

Santas picture Santas  路  3Comments

thealjey picture thealjey  路  4Comments

Bessonov picture Bessonov  路  4Comments

mormahr picture mormahr  路  3Comments