path: Make os an option argument

[nayeemrmn]

The export structure of the path module looks like this:

export const posix = {
  basename,
  dirname,
  extname,
  ...
};

export const win32 = {
  basename,
  dirname,
  extname,
  ...
};

export {
  basename,
  dirname, 
  extname,
  ...,
  globToRegExp,
  normalizeGlob,
  SEP,
  SEP_PATTERN,
  ...
};

This seems archaic and overly bound to where we vendored the implementation. It forces us to put group every function by OS in just path/posix.ts and path/win32.ts, rather than having modules like path/join.ts, path/file_url.ts, etc. It's hard to maintain and the test modules don't match it.

We should instead export one set of functions which each have an options interface containing the OS. They all have few enough positional arguments. This is modelled already by path/glob.ts, which gets to have its own module, containing:

export interface GlobOptions {
  ...,
  /** Operating system. Defaults to the native OS. */
  os?: typeof Deno.build.os;
}

These functions can branch by OS individually.

The current structure allows importing one namespace of functions for another OS, but this is niche enough that users should make such an abstraction themselves if they need it (in reality they can just import it from the node/ polyfill).

Ref https://github.com/denoland/deno/discussions/8405.

[lowlighter]

I can take a look at this later if no one else is working on it 👍🏼

I can see 2 ways to do it though, not sure which one is best:

1. Creating a new single file which abstract each function with the additional os option

//paths.ts
import * as _win32 from "./win32.ts";
import * as _posix from "./posix.ts";

export function basename(..., {os = Deno.build.os}:{os?:typeof Deno.build.os}) {
    return (os === "windows") ? _win32.basename(...arguments) : posix.basename(...arguments)
}

And we export * from "./path.ts" in mod.ts

2. Splitting win32.ts and posix.ts and reorder each function into a single file (like for test files)

//basename.ts
export function basename(..., {os = Deno.build.os}:{os?:typeof Deno.build.os}) {
    return (os === "windows") ? win32Basename(...arguments) : posixBasename(...arguments)
}

function _win32Basename(...) { /* old basename function from win32.ts */ }
function _posixBasename(...) { /* old basename function from posix.ts */ }

//other files for each path function which has a named file test

This breaks retro-compatibility with people importing directly from win32.ts or posix.ts instead of mod.ts

[bartlomieju]

@lowlighter are you still interested in working on this?

[lowlighter]

@bartlomieju I can work on it, just need to know which approach I should take

Personally I'm more for the 2nd one to be consistent with other std sections, but it breaks retro-compatibility. Or else I guess I could turn win32.ts and posix.ts into "virtual files" which re-exports by os again each function

[bartlomieju]

I don't have a strong opinion regarding this, @nayeemrmn what do you think?

[nayeemrmn]

Assuming breakages are okay, my vision was just removing win32.ts and posix.ts, shoving them all into mod.ts and giving each function a branch for os. We can split them further with demand down the line. But preserve the ones which are already split out, like common.ts, glob.ts and separator.ts.

// mod.ts
export * from "./common.ts";
export * from "./glob.ts";
export * from "./separator.ts";

export function basename(..., {os = Deno.build.os}:{os?:typeof Deno.build.os}) {
  if (Deno.build.os == "windows") {
    // windows implementation...
  } else {
    // unix implementation...
  }
}

export function dirname(..., {os = Deno.build.os}:{os?:typeof Deno.build.os}) {
  if (Deno.build.os == "windows") {
    // windows implementation...
  } else {
    // unix implementation...
  }
}

The reason I don't want to split the modules up front (even though that's one of the justifications of this design) is that the structure of those files could be in question whereas this is a forward-compatible option that could be pushed through slightly faster. Plus, I'm still interested in renaming the functions based on Rust stdlib and the file names would be based on those.

[lowlighter]

@nayeemrmn Ok so basically it's just merging win32 and posix to provide an abstraction to ease a future splitting?

Shoving them all in mod.ts seems strange, maybe it's just a misconception of mine, but I always thought that mod.ts were intended to be quite short. Should I go with a _path.ts instead, or do I just use the mod.ts directly ?

[nayeemrmn]

maybe it's just a misconception of mine, but I always thought that mod.ts were intended to be quite short. Should I go with a _path.ts instead, or do I just use the mod.ts directly ?

Having mod.ts import _path.ts which contains everything would be the exact same thing to the user as what I proposed, but redundant. Code splitting out of mod.ts only matters if those deps are public, which can be done later.

[lowlighter]

Seems that they are already tests files splitted for each function (basename_test.ts, dirname_test.ts, extname_test.ts, from_file_url_test.ts, isabsolute_test.ts, join_test.ts, parse_format_test.ts, relative_test.ts, resolve_test.ts, to_file_url_test.ts) so wouldn't it feel more natural to split everything directly following tests's current structure (and eventually rename them later) ? Or else I can put everything in the mod.ts like said above

Also I noticed that some functions are actually quite similar, should I merge their implementations to minimize code ?
For example, extname is actually the same for both posix and windows, except that the latter have special processing for drive letter, and there is also a minor difference for path seperator but 90% is the same. I won't do any refactor though, it'll just be factorization when it's adequate

[lino-levan]

I'll open a PR tomorrow to tackle this. This needs to be done before 1.0 imo.

[lino-levan]

We need to discuss requirements for std/path. What are the use-cases we envision for this module? Essentially, it gets broken down into these decisions (which do overlap a little bit):

• Do we want an option for specifying the os?
• Do we want to expose a posixX() and a windowsX() version of each X()?
• Do we keep posix.ts and win32.ts?
• Do we care about code duplication?
• How do we want APIs with ...args to function? Should those just accept an array instead?

These questions can only really be answered with concrete, supported use cases imo. I'm not really sure how to go about answering these.