Added npm task for each grunt task. Updated developing documentation. #488
Conversation
|
This PR is to get the ball rolling on converting the grunt scripts. Basically for each grunt script I created a npm script with the same name and attached the current grunt command to it. This way we can start using |
- Added .npmrc file to silent npm ERR messages on fail. - Added postcss-cli to run postcss plugins via npm. - Converted stylelint, stylefmt, autoprefixer to use npm run
ghost
assigned 
| @@ -0,0 +1,3 @@ | |||
| { | |||
| loglevel=silent | |||
There was a problem hiding this comment.
Sometimes npm errors can be useful. Possible loglevel values are "silent", "error", "warn", "notice", "http", "timing", "info", "verbose", "silly". I think we should leave the loglevel at warn or higher to avoid suppressing useful information about problems. A user can always run an npm script with the -s or --silent flags to hide errors, and if there's a particular script that we want to suppress warnings for, we could do something like "test":"npm run lint -s && npm run e2e -s".
There was a problem hiding this comment.
There were issues with using the silent flag which I can't remember now. .npmrc is the most consistent way to control logging. I will need to revisit this and see what we can do because when running linters, it was really hard to see what the issue was because of all the npm error messages.
There was a problem hiding this comment.
Which version of npm are you using? Updating to node v8.2.1 and npm v5.4.2 makes the default error messages much shorter.
There's a discussion about this on the npm repo with a lot of potential workarounds: npm/npm#8821
The final solution was an update to npm which makes the log messages shorter as of npm v5.4.2: npm/npm@65b9943
We should add an npm key with 5.4.2 in the engines field in package.json, and then update version-check.js to also check the npm version.
There was a problem hiding this comment.
Here's an example after the update;
There was a problem hiding this comment.
I am using npm 3.8.10 as this is the default that ships with the node.js long term stable installer. Yep, I read through that discussion as well. Those messages look better in npm 5. But, I think we should stick with supporting what is included in the default nodejs lts install.
There was a problem hiding this comment.
Worldview barely uses Node, since it's all client side code, so we shouldn't worry too much about Node updates breaking things anyway, since it would mostly affect developers and not users.
There was a problem hiding this comment.
I personally wouldn't mind upgrading to 5. I wanted to use it to link the semver of the options repo but that wasn't a very good reason to upgrade since I can link it to the tagged version with 3. I just worry about upgrading npm and making it harder for developers because of the fact that the default lts has 3 and most people will install that first and probably never upgrade.
There was a problem hiding this comment.
Keep in mind upgrading node and npm on Windows is not as straight forward as Mac. (Though it's not rocket science either).
See: https://stackoverflow.com/questions/18412129/how-do-i-update-node-and-npm-on-windows
I personally use nvm for windows: https://github.com/coreybutler/nvm-windows
But that's also not as straight forward as using nvm on Mac.
There was a problem hiding this comment.
Our engines field and version-check.js are just recommendations, and not a hard requirement. I don't see any reason why our app isn't backwards compatible for developers who have an older version of Node installed. But, just in case, we use the version check to warn developers on older versions that they may have to update Node if they run into any problems. Since our Node dependencies are minimal, I doubt anyone would run into problems. But asking a developer to update Node if they do run into a problem doesn't seem like a big deal to me.
There was a problem hiding this comment.
The only real risk (and I'd argue that it's still very small) is that us even using a newer version of Node while developing (even without making explicit changes to the engines/version check) could end up with us writing code that depends on newer Node features without realizing it which then don't work for developers who have an older version of Node installed. The best way to make sure that we don't run into that is to make sure that the engines fields have the version numbers that we're actually using in development.
The engines fields and version checks are basically saying "Hey, these are the versions we're using, so if you have any problems, try using these too."
| }, | ||
| "devDependencies": { | ||
| "autoprefixer": "7.1.1", | ||
| "babel-polyfill": "6.16.0", | ||
| "bluebird": "3.4.6", | ||
| "buster": "0.7.12", | ||
| "chromedriver": "2.25.1", | ||
| "cssnano": "^4.0.0-rc.2", |
There was a problem hiding this comment.
No where yet. It is intended to replace grunt-contrib-cssmin
Connects to #439
Added npm task for each grunt task.
Updated developing documentation.
Added .npmrc file to supress npm ERR! on fail
Converted
eslint,stylelint,stylefmt,autoprefixgrunt commands into pure npm run commands.