chore(*): switch from bower to npm for frontend dependencies
#389
Conversation
This is an alternative to angular#385. Closes angular#385
| // Copy lib files | ||
| shx.rm('-rf', baseDstDir); | ||
|
|
||
| files.forEach(relPath => { |
There was a problem hiding this comment.
I'd use for-of here but it doesn't really matter. :)
| const shx = require('shelljs'); | ||
|
|
||
| // Copy lib files | ||
| shx.rm('-rf', baseDstDir); |
There was a problem hiding this comment.
If all you need is a cross-platform rm -rf then rimraf.
There was a problem hiding this comment.
I also need a cross-platform mkdir -p 😃
There was a problem hiding this comment.
https://www.npmjs.com/package/mkdirp ;)
But I see your point now.
There was a problem hiding this comment.
I suspect it would be easier to ditch this file and use https://www.npmjs.com/package/cpx or something similar.
There was a problem hiding this comment.
Wouldn't we need another pretty long npm script for that?
There was a problem hiding this comment.
We could save some space in the part that iterates over files but the biggest part would probably be listing the files so it seems you're right; we wouldn't save that much.
|
I have some suggestions but LGTM with or without them. |
| // Copy lib files | ||
| shx.rm('-rf', baseDstDir); | ||
|
|
||
| for (let relPath of files) { |
| @@ -42,27 +42,27 @@ The `depth=1` tells git to only pull down one commit worth of historical data. | |||
| We have two kinds of dependencies in this project: tools and Angular framework code. The tools help | |||
| us manage and test the application. | |||
|
|
|||
| * We get the tools we depend upon via `npm`, the [Node package manager][npm]. | |||
| * We get the Angular code via `bower`, a [client-side code package manager][bower]. | |||
| * We get the tools we depend upon and the Angular code via `npm`, the [Node package manager][npm]. | |||
There was a problem hiding this comment.
Hey, I know! Why don't we switch this to yarn :-)
There was a problem hiding this comment.
npm is not "Node Package Manager", it's just "npm". The name should be changed.
| "angular-loader": "^1.5.9", | ||
| "angular-route": "^1.5.9", | ||
| "html5-boilerplate": "0.0.1", | ||
| "shelljs": "^0.7.5" |
There was a problem hiding this comment.
shelljs is strictly a devDependency
There was a problem hiding this comment.
Currently it is necessary in production, because we are not committing app/lib/. So it is needed to copy the files to app/lib/ in production as well.
There was a problem hiding this comment.
We need grunt in the angular 1 project to generate the production distribution but that doesn't stop grunt from being a devDependency. Unless we ship shelljs as part of the application I don't think I would call is a dependency.
There was a problem hiding this comment.
Actually, since we copy every frontend asset to app/lib/, nothing here should be a dependency. Were we to publish an npm package from this project it would have included the built app/lib/ and no deps.
There was a problem hiding this comment.
Actually, not true (afaict). Because app/lib/ is ignored (in .gitignore). In the angular.js repo, it is different, because we have publish script that does all sorts of stuff.
Also, using git for deployment (especially for small to medium apps) is a viable approach (and one I've used several times). It is handy to be able to git pull && npm install --production.
Anyway, I don't feel too strongly about it (although I do think it is better to have them as dependencies).
| const shx = require('shelljs'); | ||
|
|
||
| // Copy lib files | ||
| shx.rm('-rf', baseDstDir); |
There was a problem hiding this comment.
I suspect it would be easier to ditch this file and use https://www.npmjs.com/package/cpx or something similar.
|
Superceded by #390. |
This is an alternative to #385.