jinder/path

Is this NPM package required when NodeJS ships with a path module?

capaj opened this issue ยท 15 comments

capaj commented

Why would anyone need to install this module when it comes bundled with npm?

Presumably to use it in a browser....

This is a package I've inherited, and it's not actively maintained. I'm really not sure why so many Node-specific packages rely on it. Perhaps it should be deprecated.

capaj commented

@jinder in the browser? I can't imagine this code would work in browser. process is undefined in the browser, so https://github.com/jinder/path/blob/master/path.js#L25 would throw an error.

For example browserify has it's own path shim which has nor process references:
https://github.com/substack/path-browserify/blob/master/index.js

I think people are installing this, because they don't know this is built in. A lot of people first come to node via something like express tutorial and in it, they have to install everything.

I think this should really be removed from NPM. No need to vaste megabytes of bandwith downloading this.

Although it's not packaged (browserify-style), it does depend on the process and util NPM packages. So there's certainly going to be dependencies that are using it and packaging it themselves.

I agree with what you're saying though. But I won't remove it from NPM (hundreds of dependent packages and half a million downloads a month). Perhaps a deprecate and notice would be sufficient?

capaj commented

@jinder you can't really use this module unless you specify a require by absolute or relative path. So if there is a project, which has this as dependency and requires a path by doing: require('path') it get's the bundled native module not this one. It only sits there and does nothing. I seriously doubt anyone would be requiring path module like this:
require('./node_modules/path')
And if someone does, well, then he had this coming.

I hope NPM maintainers take the same stance.

I'm a bit apprehensive about pulling this from the registry without knowing for sure it won't break anything.

I'll deprecate it this weekend and monitor it for a while to see what to do next.

A bit of background about how I've inherited this from @coolaj86: I needed to use a package that depended on this, however it was published to the NPM registry without any licensing information. Which meant the only way I could use it, was by re-publishing it (copy + paste from node core) and updating the package.json with relevant licensing details.

capaj commented

it should be easy to try out-just find some packages which have CI setup this in package.json and remove it and make a PR. I bet you nothing breaks.

You certainly can use it without having an absolute/relative path. RequireJS lets you specify the paths in its config for named modules.

You're welcome to contact any of the dependent libraries to investigate more. For the moment, I think it's only safe to deprecate it.

You could of course add a little love to it like this:

if (process.versions && process.versions.node) {
    console.warn("[path installed via npm]: YOU'RE DOING IT WRONG! See https://github.com/jinder/path/issues/6");
}

Actually... you could do what I do with unibabel's package.json

I set "main": "node.js" and "browser": "unibabel.js".

You can then have the node.js output "you're using this module incorrectly..."

Also, a person can't really accidentally use this from npm. require('path') with always return the internal node module. require('path/'), however, would search node_modules.

I think in those instances, you'd only see the message if they're specifically forcing the use of the package under Node. Really, what we want is to somehow notify users who have it listed as a dependency in their package.json, but are actually using the native node module. i.e. it's an unnecessary dependency.

attach a message using npm deprecate?

npm deprecate path@"< 99" "This is for the BROWSER. Do NOT use it for NODE. See https://github.com/jinder/path/issues/6"

You could use the module in a browser environment together with browserify. I have tested this and it works and I am going to use this module for these purposes.

My app runs in node.js and browser environments so this is the perfect module for me.

Okay, so, latest plan of action: Update the readme to clearly indicate it's not necessary to have a dependency on this module if you're only running on Node. Any objections? :)

+1 for this

Just noticed on NPM 3 at least, the following appears when running install:

npm WARN package.json path@0.12.7 path is also the name of a node core module.