Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

chore(bower): remove bower #425

Closed
wants to merge 1 commit into from
Closed

chore(bower): remove bower #425

wants to merge 1 commit into from

Conversation

jaythomas
Copy link

Bower has recently had issues that have even come up in some of the github issues on this project. More recently though, you will now get this warning installing dependencies on this project:

➜  angular-seed git:(master) yarn install
yarn install v1.0.2
info No lockfile found.                                                              
[1/4] Resolving packages...
warning [email protected]: ...psst! Your project can stop working at any moment because its dependencies can change. Prevent this by migrating to Yarn: https://bower.io/blog/2017/how-to-migrate-away-from-bower/ 

Changes:

  • Remove bower in favor of npm/yarn
  • Replace all bower_components references with node_modules
  • Update the README accordingly
  • Add symlink in app/ folder so that <script> tags can load node_modules without the need to copy files or use a build tool -- although a build tool like webpack or brunch should probably be used instead in a follow-up commit because that's what most of this type need and the seed project should set up all the most typical tools for getting the job done right.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@jaythomas
Copy link
Author

I signed it.

@googlebot
Copy link

CLAs look good, thanks!

@@ -0,0 +1 @@
../node_modules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work on Windows and has some other issues:

  • Serving all your npm dependencies is probably not the best idea.
  • This makes it harder to deploy the app, because you have to copy both app/ and node_modules/.
  • Due to the above, this increases the size of the deploy artifacts significantly.

"dependencies": {
"angular": "~1.5.0",
"angular-loader": "~1.5.0",
"angular-mocks": "~1.5.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really belongs in devDependencies.

@gkalpak
Copy link
Member

gkalpak commented Oct 13, 2017

This is essentially a duplicate of #390 (and #389). I am going to close it in favor of them, since they address some more issues. Thx taking the time to work on this anyway 👍

@petebacondarwin, should we move forward with #390?

@gkalpak gkalpak closed this Oct 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants