Skip to content

Improve Path.parse / Path.format combo #1999

Closed
@ivan-kleshnin

Description

@ivan-kleshnin

We have Path.format / Path.parse functions.
They can be chained which is very convenient.

import Path from "path";

let path = "/foo/bar/bazz.js";
let pathP = Path.format(Path.parse(path));

Currently parse converts string to an object with such structure

{ root: '/',
  dir: '/Users/ivankleshnin/Projects/demo',
  base: 'config.yml',
  ext: '.yml',
  name: 'config' }

This object contains denormalized data between base, name and ext key values.
Now let's try to replace file extension.

import Path from "path";
import {assoc} from "ramda"

let path = "/Users/ivankleshnin/Projects/demo/config.js";
let pathP = assoc("ext", ".json", Path.parse(path));
/* {...
  base: 'config.js',  -- these two are 
  ext: '.json',       -- unsynced!
...} */
console.log(Path.format(pathP)); // extension weren't changed :(

The simplest task is going to be not so simple?!
Now if format took into consideration ext and name rather than base this could lead to an equal problem with changes to base key being ignored.

Can we get rid of this base key? It's always parsed.name + parsed.ext formula, not a big deal to make it manually. Example of hidden file parse: { base: '.gitignore', ext: '', name: '.gitignore' } - same rule apply.

We can probably also implement it in a backward-compatibile way,
keeping base but using JS getter / setter for it's evaluation.

Activity

benjamingr

benjamingr commented on Jun 17, 2015

@benjamingr
Member

So your suggestion is that base becomes a getter/setter?

That doesn't sound too bad but I wonder what the impact on backwards compatibility is

ivan-kleshnin

ivan-kleshnin commented on Jun 17, 2015

@ivan-kleshnin
Author

Examples:

// CURRENT BEHAVIOR 
let path0 = "/Users/ivankleshnin/Projects/demo/config.yml";

let parsed0 = Path.parse(path0);
console.log(Path.format(parsed0)); // /Users/ivankleshnin/Projects/demo/config.yml (+)

parsed0.name = "xxx";
console.log(Path.format(parsed0)); // /Users/ivankleshnin/Projects/demo/config.yml (-)

parsed0.ext = ".json";
console.log(Path.format(parsed0)); // /Users/ivankleshnin/Projects/demo/config.yml (-)

parsed0.base = "test.html";
console.log(Path.format(parsed0)); // /Users/ivankleshnin/Projects/demo/test.html (+)

// NEW BEHAVIOR 
let path1 = "/Users/ivankleshnin/Projects/demo/config.yml";

let parsed1 = newParse(path1);
console.log(Path.format(parsed1)); // /Users/ivankleshnin/Projects/demo/config.yml (+)

parsed1.name = "xxx";
console.log(Path.format(parsed1)); // /Users/ivankleshnin/Projects/demo/xxx.yml (+)

parsed1.ext = ".json";
console.log(Path.format(parsed1)); // /Users/ivankleshnin/Projects/demo/xxx.json (+)

parsed1.base = "test.html";
console.log(Path.format(parsed1)); // /Users/ivankleshnin/Projects/demo/test.html (+)

Proof implementation:

...

function newParse(pathString) {
  ...

  Object.defineProperty(parsed, "base", {
    enumerable: true,
    configurable: false,
    get: function () {
      return this.name + this.ext;
    },
    set: function (value) {
      if (value.startsWith(".") || !value.includes(".")) {
        this.name = value;
        this.ext = "";
      } else {
        let [name, ext] = value.split(".");
        this.name = name;
        this.ext = "." + ext;
      }
    }
  });

  return parsed;
}

Self-contained working gist

Should be fully backward compatible, unless I miss something.

To be precise: will break code which depends on base changes not propagated to name and ext i.e. on the "buggy" aspect of current behavior. Hard to imagine such code IMO.

Platform Requirements

(of possible feature implementation, not a provided gist)

https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty#Configurable_attribute

Basically: IE 9+

Note: I'm not aware of platform support requirements of IO JS.

added
pathIssues and PRs related to the path subsystem.
on Jun 17, 2015
nwoltman

nwoltman commented on Feb 9, 2016

@nwoltman
Contributor

IMO this sort of functionality should be implemented in user-land (npm). The path.parse function is just a convenient way to split a path into its main components and path.format is mainly for completeness so we can return the path components to the original path string (see the original issue).

benjamingr

benjamingr commented on Apr 17, 2017

@benjamingr
Member

@nodejs/collaborators anyone wants to promote this or should we close the issue?

sam-github

sam-github commented on Apr 17, 2017

@sam-github
Contributor

The current behaviour is indeed bizarre, it one of the things I talk about in https://www.youtube.com/watch?v=jJaIwea8r2A

Its not only strange in and of itself, its also inconsistent with node's url.parse/url.format.

@nwoltman Is your suggestion to deprecate the path module and promote an npm module to take its place?

self-assigned this
on Apr 17, 2017
refack

refack commented on Apr 17, 2017

@refack
Contributor

I'm interested.

nwoltman

nwoltman commented on Apr 17, 2017

@nwoltman
Contributor

@sam-github In case it helps, the example you gave from your talk:

const path = require('path');
const bits = path.parse('some/dir/index.txt');
console.log(bits.base); // > index.txt
delete bits.base;
bits.ext = '.html';
console.log(path.format(bits));

does work like the url module now, so it outputs 'some/dir/index.html'. Also, path.format() is documented well enough now that people should know what to expect when using it.

@nwoltman Is your suggestion to deprecate the path module and promote an npm module to take its place?

I didn't mean to suggest to deprecate the path module. What I meant was that "extended" functionality (such as having getters/setters on the object returned by path.parse()) should be provided by an npm module. There's a similar situation with the querystring core module where there's an npm module called qs that provides more functionality than the core module.

refack

refack commented on Apr 17, 2017

@refack
Contributor

I have a faint memory that all the logic was in parse and the others just projected parts of the parse resault... Now I'm confused why is there quadruple duplication (win/posix × parse/specific)?

49 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues that request new features to be added to Node.js.pathIssues and PRs related to the path subsystem.staletest-action

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @sam-github@mscdex@refack@Trott@Fishrock123

        Issue actions

          Improve Path.parse / Path.format combo · Issue #1999 · nodejs/node