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

CSS code splitting and loading performance #546

Open
i-like-robots opened this issue Aug 8, 2019 · 11 comments
Open

CSS code splitting and loading performance #546

i-like-robots opened this issue Aug 8, 2019 · 11 comments
Assignees
Labels
Build tools Client-side integration Help wanted Extra attention is needed Performance Making the sites built with Page Kit faster

Comments

@i-like-robots
Copy link
Contributor

What's the problem?

We are currently generating one big stylesheet for each application using Page Kit. There are two issues with this:

  1. Styles which could be reused across applications are not so our users must download and process the same things over and over.

  2. Styles which are not required by the current page will always be downloaded anyway.

I believe solving the first issue could have a large benefit on performance as my initial investigations seemed to show that cached styles are the fastest way to get content on screen.

The second issue may only noticeably affect applications which deliver multiple variations of pages and a range of components between them but over time our stylesheets for these apps kept growing which impacts performance.

What we currently do

Currently n-ui implements a tool for splitting CSS output based on code comments, this was originally based upon a Webpack plugin and later a PostCSS plugin. Most applications use this tool to generate "critical CSS" but some also use it to split individual components or page styles.

Using code comments was seen as a viable solution because we use a range of tools to generate and optimise our CSS and splitting on code comments meant operating on the final output. However, it is not totally reliable because output order != source order (depending on Sass mixin usage, Origami silient mode, and optimisation tools) and in the case of critical CSS, completely unmaintainable as this set of styles needed to absorb more and more in order to avoid creating jank and avoid bugs created by changes to the source-order.

We tend to use a single CSS entry point because we use Sass which must store information in a single global state.

What could we do?

At a minimum I think we should try and share as many of the shared UI styles between applications.

But how we do this I do not know. A solution based upon code comments could be resurrected but if there are more robust alternatives I would love to hear about them.

@i-like-robots i-like-robots added Help wanted Extra attention is needed Client-side integration Build tools Performance Making the sites built with Page Kit faster labels Aug 8, 2019
@i-like-robots i-like-robots added this to the Release v0.4.0 milestone Aug 8, 2019
@i-like-robots
Copy link
Contributor Author

i-like-robots commented Aug 8, 2019

The simplest solution I can think of for splitting out shared styles would be to create separate entry points for the header and footer. However, we'd need to be careful about not creating duplicate output in the applications CSS bundle... which can be hard with n-ui-foundations and o- components.

@sjparkinson
Copy link

If we are able to split out styles for the header, footer and navigation then we'd be in a pretty good place. Stretch would be to also split out x-teaser and myFT components.

Lean on Renovate more (through auto-merging) to ensure we're serving the same version of components across the site (to replace the way n-ui rebuilds all apps at the moment).

Could we make use of the SplitChunksPlugin webpack plugin for this?

There is also media-query-splitting-plugin which may provide some inspiration.

@i-like-robots
Copy link
Contributor Author

i-like-robots commented Aug 8, 2019

Could we make use of the SplitChunksPlugin webpack plugin for this?

This is actually an important point I should have added to the original issue... the answer is no, sadly. This is because Webpack is not responsible for assembling the Sass asset graph. The responsibility is delegated entirely to the Sass runtime, so Webpack only sees the resulting CSS*

* There a more technically correct answer which can be found my delving into the sass-loader package but it for our purposes it doesn't matter.

@i-like-robots
Copy link
Contributor Author

i-like-robots commented Aug 8, 2019

If we are able to split out styles for the header, footer and navigation then we'd be in a pretty good place.

💯 that would be a big win. We could possibly even utilise progressive loading (demo here: https://css-link-tags-in-body.herokuapp.com/) which would get the header/logo on screen ASAP.

@debugwand
Copy link
Contributor

where did we get with http2? (and serving the one big file to http1 clients as a fallback)

@i-like-robots
Copy link
Contributor Author

i-like-robots commented Aug 9, 2019

We're configured to deliver traffic over HTTP2 by default.

The only browser which should receive an enhanced experience over HTTP 1.1 is IE11 on Windows 7 and 8. We currently don't provide any specific enhancements for these users but if we do something radical (e.g. we decide the performance gains of serving ES6 code is worth $$$) then we'd have to divide our browser support policy and mandate teams provide a suitable experience for them.

Our JS delivery strategy is based on splitting code into lots of pieces. The assumption is that a high level of caching and code reuse between pages will bring the biggest performance gains. Our tests indicate this is true but we're not sure yet how many pieces is too many and whether or not we'd see similar gains for CSS as we do for JS.

UPDATE: About 70% of our real-user requests are delivered over HTTP2 https://gist.github.com/i-like-robots/e4feebad20d132718767b319ad8db2f4

@notlee
Copy link
Contributor

notlee commented Aug 14, 2019

Thinking about the output order != source order issue: if we could avoid techniques like splitting with comments, we'd be able to use Sass placeholders safety and reduce our bundle size significantly (probably less so over the wire, as compression helps a lot with duplicate css).

@sjparkinson
Copy link

sjparkinson commented Aug 15, 2019

Sass has a special kind of selector known as a “placeholder”. It looks and acts a lot like a class selector, but it starts with a % and it's not included in the CSS output.

Placeholder selectors are useful when writing a Sass library where each style rule may or may not be used. As a rule of thumb, if you’re writing a stylesheet just for your own app, it’s often better to just extend a class selector if one is available.

https://sass-lang.com/documentation/style-rules/placeholder-selectors

.alert:hover, %strong-alert {
  font-weight: bold;
}

%strong-alert:hover {
  color: red;
}

@sjparkinson
Copy link

sjparkinson commented Aug 15, 2019

🆕 Splunk query..

https://financialtimes.splunkcloud.com/en-US/app/search/search?q=%7C%20tstats%20prestats%3Dt%20count%20WHERE%20index%3D%22restricted_ftdotcom%22%20source%3D%22133g5BGAc00Hv4v8t0dMry%22%20BY%20%22connection.h2.is_h2%22%2C%20_time%20span%3D1d%0A%7C%20timechart%20span%3D1d%20count%20by%20%22connection.h2.is_h2%22&display.page.search.mode=verbose&dispatch.sample_ratio=1&earliest=-30d%40d&latest=now&display.page.search.tab=visualizations&display.general.type=visualizations&sid=1573576627.6016451


We're configured to deliver traffic over HTTP2 by default.

I remember someone mentioning working with Jake to get a figure. For prosperity I ran...

SELECT
  connection.h2.is_h2 AS h2,
  COUNT(connection.h2.is_h2) AS count
FROM
  `ft-enabling-technology-group.fastly.133g5BGAc00Hv4v8t0dMry`
WHERE
  DATE(timestamp) >= "2019-08-01"
  AND cache.is_edge = TRUE
GROUP BY
  h2

And found that for all requests to www.ft.com (including requests to the Origami services), 77% were served over http2 between 2019-08-01 and 2019-08-15.

@notlee
Copy link
Contributor

notlee commented Aug 15, 2019

If we are able to split out styles for the header, footer and navigation then we'd be in a pretty good place.

💯 that would be the biggest win. We could possibly even utilise progressive loading (demo here: https://css-link-tags-in-body.herokuapp.com/) which would get the header/logo on screen ASAP.

Throwing a wild idea out there: If we included multiple requests to the build service, one for each component, with a major and carat o-example^1.0.0 we'd get strong caching both within and across projects. But how to prevent version conflicts with multiple calls to the build service...

@i-like-robots
Copy link
Contributor Author

i-like-robots commented Aug 20, 2019

I've done a few experiments and I think something along these lines may strike a reasonable balance between performance, caching, and maintainability:

  • The Page Kit layout component provides separate blocking and async Sass partials. These can be considered somewhat analogous to "header" and "footer" but the latter may also include styles for the drawer, megamenu, typeahead etc.
  • Add a new asyncStylesheets option to the Page Kit shell component which can load styles in a non-blocking manner.
  • Apps add a shared-blocking.scss and shared-async.scss Sass entrypoint. This should help to avoid some of the pitfalls outlined in https://github.com/Financial-Times/next/issues/396. Application styles should be managed separately to shared-*.scss

The code for implementing this would be ugly, but hopefully hidden from any consuming apps...

// @financial-times/dotcom-ui-layout/blocking.scss

@import "n-ui-foundations/main";

// We don't need the sub-brand or transparent header styles so disable them.
// NOTE: Don't output anything which isn't visible on page load!
@import "o-header/main";
@include oHeader($features: ('nav', 'subnav', 'search', 'anon', 'simple'), $include-base-styles: true);

@import '@financial-times/dotcom-ui-layout/styles';
// @financial-times/dotcom-ui-layout/async.scss

// This will already have been output in blocking.scss
$n-ui-foundations-applied: true;
@import "n-ui-foundations/main";

// Output anything which isn't visible on page load
@import "o-header/main";
@include oHeader($features: ('drawer', 'megamenu', 'sticky'), $include-base-styles: false);

// Megamenu z-indexes etc.
@import "@financial-times/dotcom-ui-header/styles";

@import "n-topic-search/main";
@include nTopicSearch();

$o-footer-is-silent: false;
@import "o-footer/main";

I believe this would allow the reuse of ~60kb of (minified) CSS between pages, or ~25% of the average CSS bundle size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build tools Client-side integration Help wanted Extra attention is needed Performance Making the sites built with Page Kit faster
Projects
None yet
Development

No branches or pull requests

5 participants