Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bower.json not including css file in main #16663

Closed
ezraroi opened this issue Jun 17, 2015 · 36 comments
Closed

bower.json not including css file in main #16663

ezraroi opened this issue Jun 17, 2015 · 36 comments
Labels

Comments

@ezraroi
Copy link

@ezraroi ezraroi commented Jun 17, 2015

Tools like Wiredep stopped working with bootstrap after the change in last version.

@Johann-S

This comment has been minimized.

Copy link
Member

@Johann-S Johann-S commented Jun 17, 2015

It's due to this Pull request : #16359

But you have to check the new bower specs bower/spec#43

@ezraroi

This comment has been minimized.

Copy link
Author

@ezraroi ezraroi commented Jun 17, 2015

yes I saw this.
But looks like the bootstrap bower.json file should provide a solution for the css file.
I don;t see a reason why not to include it

@kkirsche

This comment has been minimized.

Copy link
Contributor

@kkirsche kkirsche commented Jun 17, 2015

I'm not sure I fully understand, so if I am wrong I apologize. But I think we should not do anything that is not directly from / for the spec. Anything else is up to the author because they chose to depend on third party libraries. We should not, in my opinion, be held responsible for fixing third-party functionality due to a different third party changing their spec. Our only goal is to follow the spec provided by the third party. I would recommend creating an issue with wiredep to adjust their behavior to use the new spec

@mdo

This comment has been minimized.

Copy link
Member

@mdo mdo commented Jun 17, 2015

Should we say fuck the spec and do this? I didn't think about it much when I saw the PR—spec is spec—but this actually might fuck up a lot of folks. Thoughts @cvrebert?

@mlakmal

This comment has been minimized.

Copy link

@mlakmal mlakmal commented Jun 17, 2015

since .css reference is removed from main, wiredep is not able to identify if bootstrap has any css files to be injected into automated bower:css section ... this will break angular apps for lot of folks using bootstrap with yeoman scaffolding since yeoman uses this setup by default ...

i reverted bootstrap version to 3.3.1 for our project to build successfully until i identify a solution for this ...

@redysanin

This comment has been minimized.

Copy link

@redysanin redysanin commented Jun 17, 2015

Even we also face same issue in bootstrap 3.3.5 . at grunt serve it remove
"bower_components/bootstrap/dist/css/bootstrap.css"
It look like they remove "dist/css/bootstrap.css" from bower_components\bootstrap\bower.json

We fix this issue temporary by overrides our project bower.json. for us it work fine. but waiting for some solution from bootstrap.

 "overrides":{
    "bootstrap" : {
         "main": [
            "less/bootstrap.less",
            "dist/css/bootstrap.css",
            "dist/js/bootstrap.js"
          ]
    }
  }
@mlakmal

This comment has been minimized.

Copy link

@mlakmal mlakmal commented Jun 17, 2015

I was trying out that override, that seems to do the trick. as long as we can override, this shouldn't be a issue on bootstrap side ...

@kkirsche

This comment has been minimized.

Copy link
Contributor

@kkirsche kkirsche commented Jun 17, 2015

My personal preference is to follow the spec rather than to modify the bower file Bootstrap provides. My belief is that that as new bower projects are made and as other projects migrate to the latest spec this issue will crop up there as well. Us patching it while potentially solving this issue for users of Bootstrap, doesn't prevent the same issue with other repos thus making me believe the issue should be addressed by the package(s) which use this data, not by the package maintainers.

(Note: @redysanin use a backtick (`) for inline code, three backticks with a language name or 4 space indentation to begin it be a code block. Documentation on code blocks)

@ezraroi

This comment has been minimized.

Copy link
Author

@ezraroi ezraroi commented Jun 17, 2015

From what I understand from the spec, it is one file per type, so I can have one js file and one css file?

@ezraroi

This comment has been minimized.

Copy link
Author

@ezraroi ezraroi commented Jun 17, 2015

Issue from the bower project:
bower/spec#43 (comment)

@karthilxg

This comment has been minimized.

Copy link

@karthilxg karthilxg commented Jun 17, 2015

Reverting to bootstrap version 3.3.2 also works fine..

The bootstrap bower file differences between the version,

bootstrap 3.3.5

"main": [
    "less/bootstrap.less",
    "dist/js/bootstrap.js"
  ] 

bootstrap 3.3.2

"main": [
    "less/bootstrap.less",
    "dist/css/bootstrap.css",
    "dist/js/bootstrap.js",
    "dist/fonts/glyphicons-halflings-regular.eot",
    "dist/fonts/glyphicons-halflings-regular.svg",
    "dist/fonts/glyphicons-halflings-regular.ttf",
    "dist/fonts/glyphicons-halflings-regular.woff"
  ]
@ezraroi

This comment has been minimized.

Copy link
Author

@ezraroi ezraroi commented Jun 17, 2015

Yes, but we want to be able to use the last versions.

@karthilxg

This comment has been minimized.

Copy link

@karthilxg karthilxg commented Jun 17, 2015

@ezraroi Try overriding option suggested by @redysanin as a temp fix.

   "overrides":{
      "bootstrap":{
         "main":[
            "less/bootstrap.less",
            "dist/css/bootstrap.css",
            "dist/js/bootstrap.js"
         ]
      }
   }
@cvrebert cvrebert added the meta label Jun 17, 2015
@cvrebert

This comment has been minimized.

Copy link
Member

@cvrebert cvrebert commented Jun 17, 2015

From what I understand from the spec, it is one file per type, so I can have one js file and one css file?

@ezraroi The issue isn't JavaScript and CSS. The issue is CSS and Less/Sass. Per the spec:

Use source files with module exports and imports over pre-built distribution files.

In other words, in the case of stylesheets specifically: main should include the Less/Sass source code, NOT the prebuilt CSS. I specifically sought clarity about this from the Bower devs in bower/spec#43 , and the example in the spec backs this up by including sass/motion.scss and NOT movement.css in its main.

@cvrebert

This comment has been minimized.

Copy link
Member

@cvrebert cvrebert commented Jun 18, 2015

@mdo Please god no. After over a year and a half of discussion upstream, and months of prodding the Bower devs into taking action, we finally have a proper spec; let's stick to it now that it exists. The very concept of specs is pointless otherwise. Down the path of deliberately disobeying the written spec in favor of ad-hoc shit folks just make up lies chaos and sadness.
Several other major projects have agreed to go along with the main spec change (foundation/foundation-sites#6525, zurb/foundation-apps#607, Semantic-Org/Semantic-UI#2244, videojs/video.js#2145, select2/select2#3350), and the tooling projects were consulted in bower/spec#43 and didn't raise an objection.
So it's reasonable to expect the tools to change, and there is/will-be pressure on the tools to change since we have major projects onboard, it will just take some time. There will be some pain during the transition period; I personally believe this is worth enduring in order to help bring sanity to Bower long-term.

Is the situation perfect? Of course not. Ideally, Bower ought to offer sufficient metadata to allow its users to use precompiled CSS (or JS for that matter) when packages like Bootstrap offer it. Or else it should explicitly narrow its scope to "Bower assumes that its users will use build tools (including Less/Sass/Stylus/PostCSS/etc. compilers)". But that's squarely a problem for Bower's developers and community to work out; I don't think it's Bootstrap's job to try to tackle that ourselves, other than to comply with future updates to the bower.json spec. We have quite enough work on our plate as it is.
It's high time for wiredep et al. and the Bower project to sit down and hash out how fonts, images, and precompiled CSS ought to work in bower.json now that they're definitively excluded from main. The Bower devs acknowledged the likely need for such additional fields when discussing the new main spec.

To folks interested in improving the situation, I suggest lobbying/discussing/pull-requesting the Bower project for something like bower/spec#21 (as a starting point).

(P.S. I apologize for the vitriol, but Bower's shittiness and lack of progress in addressing its shortcomings on a reasonable timescale has caused me much angst.)

ebersold added a commit to dfki-asr/compass that referenced this issue Jun 18, 2015
…5 work again

see this thread for more details: twbs/bootstrap#16663
@alber70g

This comment has been minimized.

Copy link

@alber70g alber70g commented Jun 18, 2015

I'd like to add that here is mentioned that you can also add the css. But that's before the spec changed.: bower/spec#36 (comment)

@mdo +1 fuck the spec

@cvrebert and others:
I believe the bower.json's main should provide the entry point for using it as library, not as source. If you want the source (less/scss/ts/etc), you should git clone the repository and do it all yourself.

As an example: I'm not referencing a .dll or nuget package to get the source of it in my project, and slowing down the compilation and with that the development process. I reference the library to use it, not to compile it or do some implicit charity for it

@mdo

This comment has been minimized.

Copy link
Member

@mdo mdo commented Jun 18, 2015

It sucks that the Bower spec is just now getting some definition, but until the dust settles, I'm good keeping us within what little definition we have thus far. We'll stick with the spec and not add the compiled CSS dist files, nor font files.

Sorry if this fucks up your day as a Bootstrap user, but I suggest taking any feedback to the Bower folks so they can appropriately act on it for the entire Bower community.

@mdo mdo closed this Jun 18, 2015
@danielstankiewicz

This comment has been minimized.

Copy link

@danielstankiewicz danielstankiewicz commented Jun 19, 2015

I agree that we should be able to use it as a library, not as source. My app just got broken because of 3.3.5 version. Falling back to 3.3.4. And I personally think that last number version change shouldn't come with changes that impact users so much.

@kkirsche

This comment has been minimized.

Copy link
Contributor

@kkirsche kkirsche commented Jun 19, 2015

The change itself is a package manager change and subject to their choices not bootstrap. Use and dependence on any package manager comes with risks. The core library has not had any breaking changes in the last release, my recommendation would be to raise your concern with bower as we are simply following their spec.

@s12chung

This comment has been minimized.

Copy link

@s12chung s12chung commented Jun 19, 2015

any complaints about the spec should go up to bower---web standards are incongruent enough already.

at the same time, could you upgrade the version to a major upgrade? following http://semver.org.

and maybe make an upgrade note. (i still haven't figured out how to upgrade yet)

@ThisGuyCodes

This comment has been minimized.

Copy link

@ThisGuyCodes ThisGuyCodes commented Jun 19, 2015

I understand deciding to conform to the spec right away, but this should still be considered a breaking change, this shouldn't have been a point release, but 4.0 instead.

@kkirsche

This comment has been minimized.

Copy link
Contributor

@kkirsche kkirsche commented Jun 19, 2015

That's fair, and I can understand it, though I don't agree with it. I appreciate the input, and believe conversations like this are the only way to ensure the community as a whole is the best it can be and serves the most people. Simply on this subject I happen to disagree with many others — simply put, I look at this at this point as an agree to disagree situation. I wanted to ensure though that it was and is clear that at this time I speak only for myself and if I was representing the project as a team member I probably would not be vocal on this subject as I wouldn't feel I have the authority to address this issue in the way it would need to be addressed. I feel the owners of the project would need to make the final call, not a general member or contributor due to the controversial nature of this change.

Hope that makes sense and we can agree to disagree with no hard feelings. 😃

@cvrebert

This comment has been minimized.

Copy link
Member

@cvrebert cvrebert commented Jun 19, 2015

@s12chung You make a good point; I'll see about adding the bower.json change to the 3.3.5 blog post summary and try to add some upgrade guidance for those unfortunate wiredep users.

@s12chung

This comment has been minimized.

Copy link

@s12chung s12chung commented Jun 20, 2015

thanks @cvrebert! it would be very helpful :)

cvrebert referenced this issue Jun 20, 2015
Per bower/spec#43 :
> Use source files with module exports and imports over pre-built distribution files.

Also, the example in that PR doesn't include
/dist/movement.css or /dist/movement.min.css
in its `main`.

If you have a complaint about this, take it up with the Bower folks:
https://github.com/bower/bower.json-spec/issues
@alber70g

This comment has been minimized.

Copy link

@alber70g alber70g commented Jun 22, 2015

Yea... fuck the developers, the hundreds of thousands of developers who are dependent on generators, package managers, and other third-party tools:

#we have a spec... so we will screw the people who actually need it

Every single webapplication using automatic updates for 'minor' NON API-BREAKING releases will break now due to this 'following the spec'

alber70g added a commit to alber70g/bower.json-spec that referenced this issue Jun 22, 2015
The main should contain files that can be used as library, not as sources. They can also contain the sources in forms of `less`, `sass`, or even `d.ts` files, so that the consuming party can choose based on filetype what to use and include in the project. Arguably even `.min.js` can also be added as it's the 'definitive' version of a 'compiled' library.

See also twbs/bootstrap compliance to the spec issues: twbs/bootstrap#16663
@desandro

This comment has been minimized.

Copy link

@desandro desandro commented Jun 22, 2015

I've proposed a new bower.json field files to help address this issue: bower/spec#47. Would love to get Bootstrap team's input on what would work best for the BS projects.

@cvrebert

This comment has been minimized.

Copy link
Member

@cvrebert cvrebert commented Jun 22, 2015

@desandro Thanks! Gave my 2¢.
CC: @twbs/team: bower/spec#47

@cvrebert

This comment has been minimized.

@s12chung

This comment has been minimized.

Copy link

@s12chung s12chung commented Jun 23, 2015

thanks for the note :)

fabian added a commit to worldskills/bootstrap that referenced this issue Jun 23, 2015
hefangshi added a commit to hefangshi/fis-pure-angular-demo that referenced this issue Jun 24, 2015
fabianschneiter added a commit to ERNICommunity/myCommunity.web that referenced this issue Jun 26, 2015
bootstrap.css was not included in index.html anymore by grunt-wiredep.
See twbs/bootstrap#16663
This will most likely change again / will be fixed.
@calidion

This comment has been minimized.

Copy link

@calidion calidion commented Jun 30, 2015

confirm.
please release this fix asap, this problem is very urgent for those updated.

@cvrebert

This comment has been minimized.

Copy link
Member

@cvrebert cvrebert commented Jun 30, 2015

@twbs twbs locked and limited conversation to collaborators Jun 30, 2015
thegreatsunra pushed a commit to thegreatsunra/angular-propeller that referenced this issue Jul 1, 2015
ajkitson added a commit to ajkitson/rui-components that referenced this issue Jul 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.