path icon indicating copy to clipboard operation
path copied to clipboard

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

Open capaj opened this issue 10 years ago • 15 comments

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

capaj avatar Dec 04 '15 10:12 capaj

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.

jinder avatar Dec 04 '15 10:12 jinder

@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.

capaj avatar Dec 04 '15 10:12 capaj

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?

jinder avatar Dec 04 '15 10:12 jinder

@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.

capaj avatar Dec 04 '15 10:12 capaj

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.

jinder avatar Dec 04 '15 11:12 jinder

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.

capaj avatar Dec 04 '15 11:12 capaj

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.

jinder avatar Dec 04 '15 11:12 jinder

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");
}

coolaj86 avatar Dec 04 '15 20:12 coolaj86

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.

coolaj86 avatar Dec 04 '15 20:12 coolaj86

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.

jinder avatar Dec 06 '15 13:12 jinder

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"

coolaj86 avatar Dec 07 '15 09:12 coolaj86

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.

astoilkov avatar Dec 08 '15 10:12 astoilkov

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? :)

jinder avatar Dec 08 '15 14:12 jinder

+1 for this

astoilkov avatar Dec 08 '15 14:12 astoilkov

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

npm WARN package.json [email protected] path is also the name of a node core module.

jinder avatar Dec 15 '15 14:12 jinder