ignore bower for now #896

Closed
wants to merge 4 commits into
from

Projects

None yet

8 participants

@RustyToms
Contributor
RustyToms commented Oct 12, 2016 edited

Summary
This PR is a possible short term patch if yarn needs to buy time to fix bower support. It's just a suggestion, and should be considered carefully before merging, if it gets merged at all. I won't be offended if it gets closed :)

This PR removes bower from the default list of registries. If the concept of this PR is acceptable, I might need some help updating tests. For now I just commented out the ones that seemed to rely on the bower code that is commented out.

Instead of deleting files I commented them out, as this is a short-term solution.

Why?
There are quite a few problems associated with bower at the moment:

  • bower.json dependencies are not installed
  • bower_components is emptied out even if it was properly installed with bower
  • bower.json files found inside a bower/components directory are all being deleted
  • bower dependencies might be added to the lockfile if there is a custom directory listed in the .bowerrc, but it seems that they are not provided with a version number, causing failures on subsequent updates
  • it looks like even if bower installs dependencies in the bower_components folder, some of the dependencies of those dependencies are put inside the node_modules folder

Test plan

  • clone https://github.com/RustyToms/yarn-issue-616, a project with one bower and one npm dependency
  • bower install
  • yarn
  • Before, the bower_components file would be emptied after yarn was run, with this PR bower_components should be untouched
RustyToms added some commits Oct 12, 2016
@RustyToms RustyToms ignore bower for now
59d6338
@RustyToms RustyToms update tests
f234665
@RustyToms RustyToms restore snapshots
78cc2e8
@RustyToms RustyToms update snapshot properly
7bbf855
@steveklabnik steveklabnik referenced this pull request in rust-lang/crates.io Oct 12, 2016
Merged

Move from npm cli -> yarn #457

@sheerun
sheerun commented Oct 12, 2016 edited

Why such move? As bower maintainer I consider endorsing Yarn as an alternative CLI, but if Yarn drops support for it, it's out of question.. How about fixing these bugs instead of removing support?

@RustyToms
Contributor

@sheerun I agree they should be fixed. I couldn't figure out how to do it and I think the project maintainers aren't sure yet either, they weren't aware it was breaking until yesterday. This is a tourniquet to stop the bleeding so it doesn't destroy bower installs.

But if someone can put a PR together to actually fix it that would be better. The idea here is "do no harm", right now someone types in "yarn" and their bower_components file gets emptied, at least every time I have done it. Since there is no documentation for bower support that I could find, I figured it might be better to roll back on that until it gets fixed. Maybe there are some configurations where it isn't happening, it seems to work at least partially for some people. I would +1 a PR that can get it working right!

@sheerun
sheerun commented Oct 12, 2016

It might be a challenge to re-introduce Bower support if Yarn drops it. I think it's better to release a patch that fixes most annoying issues (like deleting bower_components or bower.json), and then improve support in small steps.

I see the potential of this project, as it solves most of the issues Bower currently has (file locking, reliable installs, much better codebase). I'd be glad to work on improving Bower support, so maybe eventually Yarn could become Bower's "next major version". I'll talk with @kittens about it..

@RustyToms
Contributor

I was going to keep this open until there was some viable alternative, but I do agree with @sheerun about the importance of including bower support, especially if he is supporting it. So I'm going to close this, I guess a maintainer could reopen it if they really wanted to.

@RustyToms RustyToms closed this Oct 12, 2016
@thejameskyle
Member

Yeah, let's push forward one issue at a time. It probably makes sense to let the dust settle. We expected a ton of issues to be opened at launch, we just need to work through them. The problems will get smaller and smaller very quickly.

@sashashakun sashashakun referenced this pull request in code-corps/code-corps-ember Oct 12, 2016
Closed

Add yarn lock file and additional info in docs #604

@sombriks

so, if i understood correctly, bower projects just need keep a fair distance from yarn until it gets updated to ignore bower setups?

@ollisal
ollisal commented Oct 13, 2016 edited

Can I even instruct Yarn to skip messing with bower stuff via some command line or configuration option in the meantime?

With dozens of grave open issues concerning bower support, I think it would've been a good bet to disable bower stuff by default until those issues are sorted. Now it's pretty hard to figure out how/if yarn can be used safely together with bower, or to install the npm dependencies for a project that also uses bower in any way, for that matter.

@ollisal
ollisal commented Oct 13, 2016

After some inspection of the source code and such, it looks like it's definitely the best to keep my distance for now - no way to get yarn to leave bower alone.

@leizhao4

I think Yarn should eventually fix Bower support, but at this point it's broken and should be disabled by default until there is a fix. Having nothing is better than having a broken support which deletes my files.

@RustyToms
Contributor

The easiest work around, if you don't have any .bowerrc files except for the one in the root of your project, appears to be to run bower install after you run yarn. In my experience the problem of files being deleted only happens the first time you run yarn. Then bower will have to install again. And it should be fine after that.

@tallaxes

only happens the first time you run yarn

Or maybe anytime packages are installed? yarn add also appears to delete files ...

@AllenBW AllenBW added a commit to AllenBW/manageiq-ui-self_service that referenced this pull request Oct 17, 2016
@AllenBW AllenBW Adds yarn, updates travis to install using yarn
Yarn does not yet jive with bower, when it does this build step will be removed: yarnpkg/yarn#896
b4ec885
@AllenBW AllenBW added a commit to AllenBW/manageiq-ui-self_service that referenced this pull request Oct 17, 2016
@AllenBW AllenBW Adds yarn, updates travis to install using yarn
Yarn does not yet jive with bower, when it does this build step will be removed: yarnpkg/yarn#896
67fb5d4
@AllenBW AllenBW added a commit to AllenBW/manageiq-ui-self_service that referenced this pull request Oct 17, 2016
@AllenBW AllenBW Adds yarn, updates travis to install using yarn
Yarn does not yet jive with bower, when it does this build step will be removed: yarnpkg/yarn#896
7d5d0d3
@AllenBW AllenBW referenced this pull request in ManageIQ/manageiq-ui-service Oct 17, 2016
Merged

Adds yarn, updates travis to install using yarn #252

@eladchen
eladchen commented Oct 19, 2016 edited

How about allowing the two to coexist ?

As a temporary solution, Why not make 'yarn' move existing 'bower_components' ( if there is one )
to some random folder when an install / add process is ?

e.g: 'bower_components' > 'bower_components_temp_cf45f2fa62cc73f38220' and revert it back once yarn's install flow is done.

Thoughts ?

@RustyToms
Contributor

Bower support was removed in #1441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment