suggestion(path): remove posix.toNamespacedPath()

[iuioiua]

No description provided.

[kt3k]

There was a feedback from @jollytoad that they like to see std/path, std/path/windows, and std/path/posix having the same export items. That was why we didn't remove toNamespacedPath from std/path/posix

[iuioiua]

I understand but disagree. It being consistent isn't a reasonable justification for having a no-op function.

[kt3k]

I still don't fully understand the benefit of std/path, std/path/windows, and std/path/posix having the same exports very well.

@jollytoad Could you elaborate on what can be achieved with it?

[jollytoad]

The idea was that you could remap std/path to std/path/posix using your import map when deploying to a known platform (such as Deploy), thus eliminating all windows modules and all of the default platform branching modules from your deployment.

[jollytoad]

It unfortunately doesn't work though, we really needed the platform specific modules to not be a sub-path of std/path as suggested in this previous PR... #3650

[kt3k]

Mapping per file still works with current structure:

{
  "imports": {
    "https://deno.land/[email protected]/path/mod.ts": "https://deno.land/[email protected]/path/posix/mod.ts"
  }
}

import { basename } from "https://deno.land/[email protected]/path/mod.ts";
console.log(basename.toString());

The above only maps mod.ts, but the user can list all API paths in the import map.

[iuioiua]

Remapping seems like an edge case that doesn't justify having a function that does nothing but return its argument.

[kt3k]

If remapping like the above works, that could be a part of best practice for deploying to prod environment, I guess (You can remove some dependency for free).

Also keeping posix.toNamespacedPath() doesn't cost anything.

[iuioiua]

Yes. Keeping posix.toNamespacedPath() doesn't cost anything. But I still don't see a use case involving import mapping that would justify keeping a function that does nothing. Even if there was one, one can remove that function call, and their code works fine.

Either way, feel free to close this issue if you feel strongly enough about it. It's no big deal, either way.

[jollytoad]

@kt3k remapping the single mod.ts isn't effective, as other modules use the individual modules directly, for remapping to be truely effective you need to remap the folder (trailing /), but the fact that the target is a sub-path path complicates things (ie. path/ -> path/posix/), if it were a sibling (ie. path/ -> path_posix/) it would make the remapping simple and effective.

import maps allow a very simple and efficient static dependency injection mechanism, i find it a shame that it hasn't been embraced more by the Deno community (maybe I need to write a blog!)

I'd love to see it become a standard practice in the std lib wherever a implementation choice may occur, although it would require another round of reorganisation (see the PR I linked to earlier).

Here's a more concrete example of a simple Deno library I've created that utilises import mapping to chose an implementation of a simple storage interface, the choice being Deno.KV, web storage, filesystem etc... https://github.com/jollytoad/deno_storage_modules

[iuioiua]

Sticking to the subject, are we fine with removing posix.toNamespacedPath()?

[kt3k]

I'm not convinced. The benefit by getting rid of it feels too minimal or nothing.

And remapping of std/path/* to std/path/<platform>/* is still possible by listing all entries in std/path/* in import map

{
  "imports": {
    "https://deno.land/[email protected]/path/mod.ts": "https://deno.land/[email protected]/path/posix/mod.ts",
    "https://deno.land/[email protected]/path/basename.ts": "https://deno.land/[email protected]/path/posix/basename.ts",
    "https://deno.land/[email protected]/path/common.ts": "https://deno.land/[email protected]/path/posix/common.ts",
    ...
  }
}

So this can be free perf improvement of any app when deployed to certain platform.

[iuioiua]

Ok. Closing.