zspecza / common-tags

🔖 Useful template literal tags for dealing with strings in ES2015+
Other
2k stars 61 forks source link

Why the NPM >=3.0.0 requirement? #43

Closed ntwb closed 8 years ago

ntwb commented 8 years ago

I just noticed in a stylelint/stylelint Travis CI job the warning:

npm WARN engine common-tags@1.3.0: wanted: {"node":">=4.0.0","npm":">=3.0.0"} (current: {"node":"4.4.7","npm":"2.15.8"})

In your readme requirements: https://github.com/declandewet/common-tags#requirements 👍

"NPM (v3.0.0+ highly recommended) (this comes with Node.js)"

When NodeJS 4.x is installed with NVM as Travis CI does it does not install the latest NPM 3.x.x branch, the 2.x.x branch is used, you can also see this in your own 4.x Travis job: • https://travis-ci.org/declandewet/common-tags/jobs/139596870#L123


2.28s$ nvm install 4
Downloading https://nodejs.org/dist/v4.4.5/node-v4.4.5-linux-x64.tar.xz...
######################################################################## 100.0%
Now using node v4.4.5 (npm v2.15.5)
...
$ node --version
v4.4.5
$ npm --version
2.15.5
$ nvm --version
0.31.0
install
8.15s$ npm install 

The NodeJS 5.x branch doesn't have this issue as it uses NPM 3.x for example: • https://travis-ci.org/declandewet/common-tags/jobs/139596869#L123

NodeJS 4.x.x is the LTS (Long Term Support) NodeJS release: https://github.com/nodejs/LTS/blob/master/README.md

NPM 2.x.x is the LTS NPM release: https://github.com/npm/npm/wiki/Roadmap#tactical-roadmap

So, does common-tags really require NPM 3.x.x? Would you consider removing the 3.x.x requirement? It appears to work fine with common-tags and stylelint using NPM 2.x.x

zspecza commented 8 years ago

Woops! Seems I'd forgotten to remove that - the NPM requirement was moreso supposed to just be a guideline as there are some in-general edge-case bugs that occur with Babel 6 and npm < v3 - but it doesn't make sense to have such a restriction here, since Babel is jsut one of many transpilers. I will push a fix for this today if I have time, otherwise you can expect it to be handled tomorrow morning

Thanks for the report @ntwb!