chore(*): switch from `bower` to `npm` for frontend dependencies by petebacondarwin · Pull Request #390 · angular/angular-seed

petebacondarwin · 2016-12-07T21:58:50Z

A tweak to #389

googlebot · 2016-12-07T21:58:53Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

gkalpak

This copies a little more than necessary, but this was the case with bower as well.
LGTM (with a couple of comments addressed)

gkalpak · 2016-12-07T22:01:51Z

    "karma-junit-reporter": "^0.4.1",
-    "protractor": "^4.0.9"
+    "protractor": "^4.0.9",
+    "shelljs": "^0.7.5"


Do we need shelljs any more?

no - you're right :-)

gkalpak · 2016-12-07T22:03:31Z

  "devDependencies": {
    "angular-mocks": "^1.5.9",
    "bower": "^1.7.7",
+    "cpx": "^1.5.0",


I still consider this a non-dev dependency. We rely on it to copy the files to app/lib/ even in "production". Alternatively, we could add app/lib/ to the repository (but I am not a big fan of that 😃).

I guess it comes down to what you mean by "production". In my view, the production stuff is what will be deployed to the web server. To be honest, since we are not using npm to do the deployment, it is irrelevant. I would be happy to move everything into either dependencies or devDependencies but I feel that putting the angular libraries (and the html5-boilerplate) into the dependencies, it is signalling to the developer what will eventually end up as part of the actually deployed application.

petebacondarwin · 2016-12-07T22:08:03Z

    "update-deps": "npm update",
-    "postupdate-deps": "bower update",
-
+    "postupdate-deps": "npm run copy-libs",


I wondered if we really need update-deps...
Can we not just have a postupdate hook that copies the libs?

npm does not run postupdate after npm update. From reading this, I would expect it run postinstall (after it runs npm install under the hood), but it doesn't.

Another idea is to recommend using npm install even for updating in README.md.

I think postinstall only runs when a module is being installed as a dependency, rather than when one is installing the dependencies of the current module.

What I mean is, it does run when I execute npm install, but it doesn't when I execute npm update (which is supposed to run npm install <outdated deps> under the hood) from the exact same project state.

alexandernst · 2016-12-16T10:32:40Z

Is this ready to be merged?

gkalpak · 2016-12-16T11:00:10Z

Not yet (but it's close) 😃

alexandernst · 2017-05-29T12:31:12Z

How about now? Is it ready? What is missing?

Natanagar · 2017-05-29T12:52:51Z

I use your code to create templates in my project, I hope you allow?

gkalpak · 2017-10-13T08:51:09Z

+  },
  "devDependencies": {
+    "angular-mocks": "^1.5.9",
    "bower": "^1.7.7",


This could go now 😃

petebacondarwin · 2017-10-13T10:37:31Z

I guess we should switch it to yarn not npm?

gkalpak · 2017-10-13T10:57:26Z

I don't know. This adds an extra dependency (because npm typically comes "bundled" with node). Since this is a seed project, it might be better to be less opinionated.
So, I slightly lean towards npm, but don't feel too strongly about it. (Obviously, either is better than using bower 😁)

petebacondarwin · 2017-10-13T12:17:47Z

I have tweaked and squashed. PTAL

jaythomas · 2017-10-13T15:10:25Z

-    "postupdate-deps": "bower update",
-
+    "postupdate-deps": "npm run copy-libs",
+    "copy-libs": "cpx \"node_modules/{angular,angular-*,html5-boilerplate}/**/*\"  app/lib -C",


This seems less than ideal to copy dependencies manually, having to update and maintain this inline script whenever a dependency is added, like bootstrap for instance. Perhaps it would be simpler to just go the route of using a build tool like webpack or brunch?

I think that in any significant project you would end up with some tool like that, but for angular-seed we are trying to keep the complexity and number of tools to the minimum.

gkalpak

A couple of minor comments. LGTM otherwise 👍

gkalpak · 2017-10-13T19:42:49Z


-Behind the scenes this will also call `bower install`. After that, you should find out that you have
-two new folders in your project.
+Behind the scenes this will also call `npm run copy-libs`, which runs a simple JS script that copies


Maybe remove the simple JS script part (this is no longer the case).

gkalpak · 2017-10-13T19:43:02Z

-Behind the scenes this will also call `bower install`. After that, you should find out that you have
-two new folders in your project.
+Behind the scenes this will also call `npm run copy-libs`, which runs a simple JS script that copies
+the AngularJS files and other fronted dependencies. After that, you should find out that you have two


fronted --> frontend

gkalpak · 2017-10-13T19:44:48Z

  baseUrl: 'http://localhost:8000/',

-  framework: 'jasmine',
+  framework: 'jasmine2',


I don't think this is necessary in latest version, but not sure about the version of Protractor we are using here.

you're right. Since Protractor v3 it is the same thing.

Closes angular#385 Closes angular#389

binki · 2018-08-31T19:24:46Z

As asked earlier, is this ready now?

petebacondarwin · 2018-08-31T21:17:00Z

I don't think this is going to land now that we are in LTS mode.

gkalpak · 2018-10-26T15:43:25Z

Rolled into #444.

gkalpak suggested changes Dec 7, 2016

View reviewed changes

petebacondarwin commented Dec 7, 2016

View reviewed changes

gkalpak mentioned this pull request Jan 30, 2017

Bower Install Fails #392

Closed

gkalpak mentioned this pull request May 26, 2017

avoid publishing new releases to deprecated bower angular/angular.js#16013

Closed

3 tasks

gkalpak reviewed Oct 13, 2017

View reviewed changes

gkalpak mentioned this pull request Oct 13, 2017

chore(bower): remove bower #425

Closed

petebacondarwin force-pushed the pr-389 branch from cfe5b4e to 41d089c Compare October 13, 2017 12:17

jaythomas reviewed Oct 13, 2017

View reviewed changes

gkalpak approved these changes Oct 13, 2017

View reviewed changes

gkalpak and others added 2 commits October 13, 2017 21:32

chore(*): switch from bower to npm for frontend dependencies

52022ee

Closes angular#385 Closes angular#389

docs(*): use the name AngularJS rather than Angular

a41e802

petebacondarwin force-pushed the pr-389 branch from 41d089c to a41e802 Compare October 13, 2017 20:33

gkalpak mentioned this pull request Oct 26, 2018

chore(*): switch from bower to npm for frontend dependencies #389

Closed

gkalpak closed this Oct 26, 2018

gkalpak mentioned this pull request Oct 26, 2018

chore(*): refactor, improve, upgrade #444

Closed

Conversation

petebacondarwin commented Dec 7, 2016

Uh oh!

googlebot commented Dec 7, 2016

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkalpak Dec 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexandernst commented Dec 16, 2016

Uh oh!

gkalpak commented Dec 16, 2016

Uh oh!

alexandernst commented May 29, 2017

Uh oh!

Natanagar commented May 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Oct 13, 2017

Uh oh!

gkalpak commented Oct 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petebacondarwin commented Oct 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

binki commented Aug 31, 2018

Uh oh!

petebacondarwin commented Aug 31, 2018

Uh oh!

gkalpak commented Oct 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

gkalpak Dec 8, 2016 •

edited

Loading

gkalpak commented Oct 13, 2017 •

edited

Loading