Order of merged-stylesheets.css in <head> #24

SachaG · 2017-06-07T02:02:15Z

SachaG · 2017-06-07T02:02:32Z

A pretty common use case if you're using a third-party UI framework such as Bootstrap is loading it from a CDN, for example using Helmet to inject link tags. .

The problem is that currently, Meteor's own stylesheet (merged-stylesheets.css) gets injected right at the top of the , meaning that Bootstrap's CSS will come afterwards and override your own user-defined styles.

So I would suggest modifying the boilerplate-generator package so that Meteor's own stylesheet always comes last. Are there any downsides to doing this?

aogaili · 2017-08-26T17:21:07Z

Any update on this?

rcurrier666 · 2017-08-27T06:00:28Z

@aliogalli, it appears the decision was made to not address it in the initial Boilerplate refactor (#8820 above) but instead to come back to it as a separate issue.

@abernix, please don't forget to come back around to the CSS load issue. My code is littered with class="myapp" additions to Bootstrap classes just so I can override the default Bootstrap formatting. Clutters up and confuses the code. Re the previous discussions, I'm happy with whatever solution is decided upon, though I would lean toward adding a replacement field, suggested by hwilson, to allow me to put the merged code wherever I want (not for any particular use case, just seems the most flexible).
<link rel="stylesheet" href="METEOR_BUNDLED_CSS_URL" />

aogaili · 2017-08-29T22:41:34Z

I really like the proposed solution of using the <link rel="stylesheet" href="meteor_merged_css" /> placeholder.

The placement of the merged CSS has performance implications as well. Using SSR, bundle visualizer, Meteor 1.6 and dynamic imports I was able to drop the initial load time from 5 seconds to 1.4 seconds (thanks @benjamn!). The merged CSS is in the rendering critical path and it's delaying the app by the 0.5 seconds. It's also impacting the lighthouse performance results ( right now it's 52% but it could be 92% if I remove the non critical css and inline what's needed.)

So @abernix I think this change is crucial for developing a highly performing Meteor apps in addition to having more control over the style sheets priority.

aogaili · 2017-08-30T01:12:48Z

Here is a screenshot of where I currently stand. According to lighthouse, meteor merged css is delaying the initial paint by 590ms. I think if we close this issue, I can hit 100% on performance in lighthouse and this is a fairly complex app.

hwillson · 2017-09-05T16:47:21Z

Thanks for sharing your findings @aliogaili. Now that the boilerplate refactor has been completed, making the change outlined in meteor/meteor#8651 (comment) is quite straightforward (actually, I made the necessary code changes a while back to test things out; they could be easily brought into the main repo). I'll bring this up at this week's issue triage meeting. More details shortly - thanks!

aogaili · 2017-09-05T17:17:25Z

Thanks @hwillson Just to confirm, with the proposed solution, would it be possible to place the link outside of the head block?

benjamn · 2017-09-06T15:49:41Z

Since the boilerplate generator technically allows for multiple CSS resources, I would tweak @hwillson's METEOR_BUNDLED_CSS_URL idea to use a placeholder HTML element instead:

<head>
  <title>meteor-issue-8651</title>
  <link
    rel="stylesheet"
    href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css"
    integrity="sha384-BVYiiSIFeK1dGmJRAkycuHAHRg32OmUcww7on3RYdg4Va+PmSTsz/K68vbdEjh4u"
    crossorigin="anonymous"
  />
  <meteor-bundled-css />
</head>

The <meteor-bundled-css/> element would usually be replaced by just one <link> tag, but could be replaced by more than one (or zero).

benjamn · 2017-09-06T15:51:24Z

And while we're at it, we might as well support <meteor-bundled-js/> as well!

aogaili · 2017-09-06T19:44:29Z

Perhaps we can think and name those placeholders as component (Meteor Component). The two components are <MeteorBundledCSS /> and the <MeteorBundledJS />. Those "Meteor Components" can be placed anywhere in the main.html and they can also have additional props in the future to control their rendering behaviour.

Developers are already used to the concept of components and perhaps it's useful to name and present them that way.

zimme · 2017-09-06T19:46:56Z

Developers are already used to the concept of components and perhaps it's useful to name and present them that way.

But as they aren't really components I personally don't think they should be "seen" as components which might confuse inexperienced developers.

aogaili · 2017-09-06T19:50:42Z

Fair point, I just thought I'd put it out there.

aogaili · 2017-09-06T19:53:54Z

But the concept of components has already been stretched out in the react ecosystem, think of react helmet, it's behaviour is not that different then the proposed Meteor tags. And inexperienced developers will probably not use those tags anyway, so perhaps it's debatable objection.

zimme · 2017-09-06T20:06:43Z

@aliogaili, what about angular developers using meteor or vue or some other view layer? They might be used to components looking different.

I think that this should be it's own thing which isn't something to relate to "components" because it's a part of Meteor's build system. But that's just my view of this.

aogaili · 2017-09-06T20:15:50Z

Got it @zimme I guess it's because I'm coming from react naming convention. So are you also objecting to what @benjamn proposed? because what he proposed in syntactically similar to a component (with vue naming convention) or are you just objecting on using react naming convention (which I agree that it might be confusing to some) ?

zimme · 2017-09-06T20:19:26Z

What he suggested is ok with me because that's closer to html element naming convention.

aogaili · 2017-09-06T20:28:09Z

I agree with you, I think it's better to keep closer to the html naming convention.

But I guess it's worth emphasizing, from conceptual perspective, that what's being proposed here is actually a custom html tags (a.k.a components) transpiled by Meteor build system and could potentially have additional props in future.

antoninadert · 2017-09-07T12:44:52Z

what @benjamn proposed looks great to me. I can easily imagine using it in my apps.
The js version too. Can't wait for it

antoninadert · 2017-09-08T14:14:19Z

I just have a new comment: I love Meteor, but now that I think of it twice, I wish I was able to take full control of packages on the client-side.

The problem is that Meteor client build provides an abstraction layer that was supposed to make things simpler (goold old Blaze-only times) but now is actually more complicated (because if you open front-end to multiple stacks, then you need more control over them)

I totally believe we should be able to plug any front-end stack, and it should work with server-side rendering provided by the server-render package.

To maintain backward compat, the current build system should be isolated in a package, and removing this package should let me play with the frontend stack at will (think of it like insecure package)

I am totally PRO having some solution like meteor-client-bundler integrated in Meteor core and supported by MDG.
At this stage I could just remove meteor's client-side packages carefully and it would be obvious what features will be missing by removing specific packages, by looking at the docs for example.

I would then be able to control my client-side script and css order entirely, even maybe to switch my builder to webpack for the front, and still be able to connect to any meteor backend via HTTP or DDP and get this reactivity, if I opt-in for some packages that enable that to me.

Well... Sorry if I am a dreamer

benjamn · 2017-09-08T16:54:22Z

@antoninadert I'm not sure where to begin convincing you of the value of Meteor's build system versus something like Webpack. What you're proposing no longer sounds like much of a framework at all, and I'm afraid I can't imagine that dream having more value than the current system for most Meteor developers.

Let's keep this issue focused on the JS/CSS tag order problem?

antoninadert · 2017-09-09T15:29:03Z

Sure !

derwaldgeist · 2018-02-03T01:00:28Z

I personally think that if loading order of CSS files matters, your CSS code is messed up. It is the purpose of the "cascading" part in CSS that you can override your styles as needed, your styles just have to be specific enough.

The simplest solution to achieve this is to add a style class to your <html> tag like this:

<html class="my-awesome-app">

You can now override any Bootstrap style easily by prepending it with .my-awesome-app, e.g.:

.my-awesome-app .btn-primary {
   background-color: magenta;
}

A second approach is to use the SASS version of Bootstrap which allows you to customize Bootstrap via variables.

In my own apps, I am using a combination of the two approaches and am very happy with this solution.

Another benefit of the "class-on-html-tag" approach is that it can be applied to user use-cases, too. For instance, I am defining additional classes depending on the environment the app runs in. If the app runs on iOS, I add the ios class, no Android android is added etc. This is very helpful if you want to design your Cordova apps according to the OS-specific style-guides.

SachaG · 2018-02-03T01:17:06Z

I personally think that if loading order of CSS files matters, your CSS code is messed up.

In that case you won't mind if we change it to be the other way around ;)

But seriously, I think relying on source order in CSS is pretty common, it helps avoid selector inflation. Making selectors more specific than needed is generally a bad idea anyway, and sometimes it's just not practical. For example, what if you're using a third-party bootstrap theme stylesheet and you need to ensure it always load after the main bootstrap CSS? You can't really just prepend every single class with .my-awesome-app…

rcurrier666 · 2018-02-03T07:07:59Z

To imply that CSS load order doesn't or shouldn't matter is simply incorrect. It's part of the CSS specification. See Section 6 of this W3C document entitled Order of Appearance. And, yes, you can overcome the load order by adding additional selector specificity, but that clutters up the CSS with extra selectors that could be avoided if the Meteor load order is changed. What I'm interested in most, is there a use case where changing the order of the CSS bundle from first to last will break existing code. If the developer has already worked around the issue by adding selectors, moving the bundle to the end shouldn't break anything. But is there a case where the existing order matters and would break code if the order is reversed? I'm looking for a reason to not simply implement case 1 (which involves moving 4 lines of code from the top to the bottom of the template). Otherwise, options 2 and 3 would suit your viewpoint since if you don't modify your code everything remains the same.

SachaG · 2018-02-03T09:14:03Z

I can't really think of a scenario where option 1 would break people's code, but with CSS who knows… It's worth pointing out that it's really easy to copy the package's code locally and change it manually if you need to.

derwaldgeist · 2018-02-03T09:47:19Z

There might be cases where the simple change in load order breaks existing styles, since load order matters in CSS. Imagine a developer who had set styles for an html element like p and a third party library had overwritten this. This might be inadvertently, but changing the load order would change the styling immediately.

Having said that, I think that putting the user-defined styles last is what developers would expect in the first place, since some just rely on the order of appearance and believe their own styles should always „win“.

The only problem is that Meteor decided to do it the other way around, and I am a big fan of backwards compatibility unless it isn’t avoidable. I had so many cases in the past when a simple meteor update broke my apps that I became somewhat sensitive for this.

derwaldgeist · 2018-02-03T09:52:07Z

@SachaG in principle, you’re right. That’s one of the reasons why I love Sass, since you can easily nest your styles then. However, this doesn’t always help. For instance, I had a third party library that made its own styles more specific in one of its minor releases which even broke the nested styles approach, and this would also break the styles if you had relied on loading order. In essence, there’s no good way to solve these kind of problems, since it is inherent in the way CSS works.

hwillson · 2018-02-04T18:08:12Z

Thanks for looking into this further @rcurrier666. Ideally we'd like to see @benjamn's approach mentioned in #24 (comment) and #24 (comment) implemented. With this approach a developer can control the location of Meteor's CSS bundle by putting the <meteor-bundled-css /> element wherever they want the bundle (which could be made up of several CSS resources) to appear. So top of <head />, bottom of <head />, top of <body />, etc. If <meteor-bundled-css /> isn't used, then Meteor's current approach would be used (for backwards compatibility). In addition, we'd like to do the same thing for bundled JS resources as well, using <meteor-bundled-js/> (but this could be handled in a separate PR if/as needed). Thanks!

rcurrier666 · 2018-02-04T21:22:57Z

@hwillson, glad we agree on the approach! While simply moving the bundle to the end is a trivial change, it is bound to break someone's app. A couple of questions for you;

Is there a use case for putting the bundle in the <body />? It would require either moving the body code from the closeTemplate back into the headTemplate (which is how the Cordova version is structured), or passing some sort of flag into headTemplate to let it know not to emit the <link /> tag for the bundle (presumably by looking into the body for the <meteor-bundled-css /> tag (which would mean the generator would need to know about the 'magic' tag.
Do we want the same feature in Cordova builds?
I'll do the bundled JS changes as a separate PR. Since the implementation is similar, let's get the CSS PR done and approved so I can just reproduce the changes for JS (and so one or the other can be backed out if need be.)

Look for a PR later in the week. And please bear with me, it's only my second PR, so I may need some guidance if I get it wrong.

skirunman · 2018-02-05T17:36:31Z

I'll answer

Do we want the same feature in Cordova builds?

Yes, please!

hwillson · 2018-02-05T17:39:44Z

I agree with @skirunman on #2. Regarding #1, I think if we can give the flexibility to include the bundled css anywhere someone might want to (in <head />, <body />, etc.), that would be ideal. Sounds good about #3 as well. Thanks @rcurrier666!

rcurrier666 · 2018-02-05T18:34:06Z

There is a major problem with inserting the CSS bundle in the body and with supporting moving around the JS bundle. If you've ever looked at the page source in the browser, you'll see there is NO HTML in the body, just <script> tags. The HTML is generated by the app/app.js script. It should be possible to move the 60+ <script> tags into the head and leave the app/app.js script in the body using a <meteor-bundled-js> tag in the head, if that is the desired behavior. Moving the CSS bundle into the body with <meteor-bundled-css /> doesn't seem possible since the body template can't see any HTML. We could, I suppose, use a different tag like <meteor-bundled-css-in-body> that is placed in the head and will put the CSS bundle at the top of the body before any <script> tags.

Unless someone likes the special tag to move the CSS bundle into the body, or has another idea on how to handle it, I'm going to limit the bundled CSS to the head.

And since I won't be looking at moving the bundled JS until after the CSS PR is complete, I'm going to open a new issue to discuss moving the bundled JS. That way the two features can be kept separate.

rcurrier666 · 2018-02-07T07:23:08Z

Could someone provide me with details of how to run the tests in /meteor/packages/boilerplate-generator-tests. I want to make sure I haven't broken anything plus I need to add new tests (if I can figure out how to test my changes). TIA.

abernix · 2018-02-07T08:33:30Z

@rcurrier666 You should be able to run those tests with the instructions in the Running specific tests section of DEVELOPMENT.md, found in the root of the repository.

So in your case you'd run:

./meteor test-packages boilerplate-generator-tests

rcurrier666 · 2018-02-08T00:54:21Z

@abernix, thanks. I tried that last night and it didn't work. I discovered that my Ubuntu image was missing some packages. After adding them, the tests work fine. Thanks for bearing with me, I've spent the last 7 years in a Windows + Perforce environment and so I'm dealing with a bit of a culture shock.

rcurrier666 · 2018-02-10T00:26:54Z

This feature has been submitted as PR #9657

rcurrier666 · 2018-02-13T07:37:41Z

Just a note that the changes for both browser and mobile are now in the PR in case anyone wants to take a look at them or try them out. I'm waiting to hear back on where to document this feature and then hopefully one of the powers-that-be will merge the PR (maybe we can get it in 1.6.2?)

rcurrier666 · 2018-02-13T08:06:27Z

All, I've opened a new issue to discuss implementing <meteor-bundled-js> to allow the JS scripts to be moved into the head. If you have any interest in this feature please head over here and chime in. I'm not going to implement this, and the powers-that-be won't merge the PR, unless there is actually some interest in the feature.

… CSS bundle. (#9657) Implements meteor/meteor-feature-requests#24.

antoninadert · 2018-03-13T22:36:49Z

I believe there should be a way to mark JS or CSS to be merged in specific package and then there would be 2 package position options: either in HTML < head> either in HTML end of < body>

This is a standard way to position HTML < Script> and < Style> tags

SachaG · 2018-06-10T01:27:41Z

The new meteor-bundled-css thing isn't working for me. In template-web-browser.js, head is blank and all my head content is in dynamicHead, which doesn't get parsed for <meteor-bundled-css/>. I'm not super clear what the differences are between head and dynamicHead, I'm guessing it has something to do with the SSR process?

rcurrier666 · 2018-06-10T05:47:07Z

Since I did the original work, I'll take a look at it unless one of the MDG folks wants to do so. = Ron

…

rcurrier666 · 2018-06-12T07:11:01Z

I've reproduced the problem (using juliancwirko/scotty boilerplate) and have a browser fix coded and manually tested. I still need to fix the cordova template and write some unit-tests to cover the new cases.

@SachaG, would you please open an issue for this so it can be tracked? I'll then do a PR for the fixes.

SachaG · 2018-06-12T08:39:06Z

Done: meteor/meteor#9982

skirunman · 2018-10-12T23:00:58Z

Is this not closed as this implemented in meteor/meteor#9657 and released in Meteor 1.7. Understand there is still out outstanding bug.

hwillson added the Project:Webapp label Jun 7, 2017

hwillson mentioned this issue Jun 8, 2017

Order of merged-stylesheets.css in <head> meteor/meteor#8651

Closed

This was referenced Jul 7, 2017

Boilerplate refactor meteor/meteor#8820

Merged

Cordova boilerplate does not play well with external scripts meteor/meteor#8876

Closed

hwillson added the review label Sep 5, 2017

benjamn added pull-requests-encouraged Project:Webapp and removed Project:Webapp review labels Sep 6, 2017

axelvaindal mentioned this issue Feb 11, 2018

Support <meteor-bundled-css /> pseudo-tag for controlling position of CSS bundle. (#24) meteor/meteor#9657

Merged

rcurrier666 mentioned this issue Feb 13, 2018

Order of merged JS in <head> #267

Open

benjamn pushed a commit to meteor/meteor that referenced this issue Feb 21, 2018

Support <meteor-bundled-css /> pseudo-tag for controlling position of…

282b450

… CSS bundle. (#9657) Implements meteor/meteor-feature-requests#24.

SachaG mentioned this issue Jun 12, 2018

<meteor-bundled-css/> doesn't work with dynamicHead meteor/meteor#9982

Open

benjamn closed this as completed Oct 13, 2018

benjamn added this to the Meteor 1.7 milestone Oct 13, 2018

dr-dimitru mentioned this issue Dec 15, 2018

Please move <script> tags out of the <body> tag #341

Closed

Order of merged-stylesheets.css in <head> #24

Order of merged-stylesheets.css in <head> #24

Comments

SachaG commented Jun 7, 2017

SachaG commented Jun 7, 2017

aogaili commented Aug 26, 2017

rcurrier666 commented Aug 27, 2017

aogaili commented Aug 29, 2017 • edited Loading

aogaili commented Aug 30, 2017 • edited Loading

hwillson commented Sep 5, 2017

aogaili commented Sep 5, 2017

benjamn commented Sep 6, 2017 • edited Loading

benjamn commented Sep 6, 2017

aogaili commented Sep 6, 2017 • edited Loading

zimme commented Sep 6, 2017

aogaili commented Sep 6, 2017 • edited Loading

aogaili commented Sep 6, 2017 • edited Loading

zimme commented Sep 6, 2017

aogaili commented Sep 6, 2017 • edited Loading

zimme commented Sep 6, 2017

aogaili commented Sep 6, 2017 • edited Loading

antoninadert commented Sep 7, 2017

antoninadert commented Sep 8, 2017

benjamn commented Sep 8, 2017

antoninadert commented Sep 9, 2017

derwaldgeist commented Feb 3, 2018

SachaG commented Feb 3, 2018

rcurrier666 commented Feb 3, 2018

SachaG commented Feb 3, 2018

derwaldgeist commented Feb 3, 2018 • edited Loading

derwaldgeist commented Feb 3, 2018

hwillson commented Feb 4, 2018

rcurrier666 commented Feb 4, 2018

skirunman commented Feb 5, 2018

hwillson commented Feb 5, 2018

rcurrier666 commented Feb 5, 2018

rcurrier666 commented Feb 7, 2018

abernix commented Feb 7, 2018

rcurrier666 commented Feb 8, 2018

rcurrier666 commented Feb 10, 2018

rcurrier666 commented Feb 13, 2018

rcurrier666 commented Feb 13, 2018

antoninadert commented Mar 13, 2018 • edited Loading

SachaG commented Jun 10, 2018

rcurrier666 commented Jun 10, 2018 via email

rcurrier666 commented Jun 12, 2018 • edited Loading

SachaG commented Jun 12, 2018

skirunman commented Oct 12, 2018

aogaili commented Aug 29, 2017 •

edited

Loading

aogaili commented Aug 30, 2017 •

edited

Loading

benjamn commented Sep 6, 2017 •

edited

Loading

aogaili commented Sep 6, 2017 •

edited

Loading

aogaili commented Sep 6, 2017 •

edited

Loading

aogaili commented Sep 6, 2017 •

edited

Loading

aogaili commented Sep 6, 2017 •

edited

Loading

aogaili commented Sep 6, 2017 •

edited

Loading

derwaldgeist commented Feb 3, 2018 •

edited

Loading

antoninadert commented Mar 13, 2018 •

edited

Loading

rcurrier666 commented Jun 12, 2018 •

edited

Loading