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

[discuss] New Platform style support for plugins #42185

Closed
tylersmalley opened this issue Jul 29, 2019 · 9 comments
Closed

[discuss] New Platform style support for plugins #42185

tylersmalley opened this issue Jul 29, 2019 · 9 comments
Labels
blocker discuss Feature:New Platform Team:Operations Team label for Operations Team Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins.

Comments

@tylersmalley
Copy link
Contributor

Before the initial SCSS support, all styles were loaded with JS as with most modern web applications. Given the ever-increasing optimization times in produce, we chose to remove styles from Webpack and ship them at CSS in the build. When loading the styles, we went the simplest route which was to load all plugin CSS files on Kibana load. This was fine when a few plugins were using it, but that has not scaled.

Given that we need to add styles support to the new platform, we need to give some thought as to how we would like to see them handled. Here are a few options:

A route which concatenates all plugin styles into a single request

Pro:

  • Alleviates the issue of requests to the domain being blocked due to the number of concurrent requests to plugin styles
  • Creates a deterministic loading order of all styles. This should eliminate any issue where the order of styles being applied can create a bug. This is common when using JS to dynamically apply styles when a component is used. If we are being too liberal with our selectors we can create a scenario where a bug would arise when a specific order of styles was loaded.
  • Global support could be added for hot-reloading of styles

Cons:

  • Over time this plugins stylesheet endpoint would continue to get grow. Loading of stylesheets has a cost, not just in the download time but also the time which the browser spends parsing and applying the styles.
  • There can be a bad actor in a 3rd party plugin which includes a massive amount of styles which would affect the performance of Kibana as a whole.

Leave it to the plugin to asynchronously load styles

Pro:

  • Would keep the styles loaded in the browser to a minimum

Con:

  • More complexity in that the plugin would need to dynamically inject the styles into the head. However, we would provide a helper in Core to assist the plugin developer with this.
  • As mentioned previously, the order is not deterministic here. For this not to be an issue, we would need to adopt a style guide of having very explicit selectors.
  • A user would be more likely to see an "unstyled" plugin as we wait for the styles to load.

I prefer the latter approach. It would keep the request size and style parsing to a minimum and would best scale as more plugins are added. However, I would like to hear some other folks opinion before moving forward.

@tylersmalley tylersmalley added discuss Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team labels Jul 29, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-design

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@snide
Copy link
Contributor

snide commented Jul 30, 2019

As mentioned previously, the order is not deterministic here. For this not to be an issue, we would need to adopt a style guide of having very explicit selectors.

In practice, we only need the following from a load order perspective

  1. EUI loaded first
  2. Kibana "global" loaded second (or more likely, EUI can be imported into this file directly so it's just one file).
  3. Any other plugins can come in here, in any order. Most of Kibana's sass is already written with enough rigor that there shouldn't be any conflict.

Generally we'd prefer the latter approach you mention, and that Kibana ONLY loads the css files for the plugins on the current page. There have been cases where we've been very worried about bleedy CSS.

I'd highly suggest talking with @thompsongl. We're in the early stages of moving EUI (and Kibana) to a more modularized system. He can give you the full overview. The above will be fine in the short term, but likely we can work together on something more flexible for you long term.

elastic/eui#1656

@joshdover
Copy link
Contributor

We also need to consider the building aspect of SCSS. Currently, this is baked into uiExports and the legacy server code (src/legacy/server/sass). This needs to be moved out of the legacy platform and into the new platform so that new platform plugins can register SCSS entry points to be compiled for their plugins.

Should this be coupled to the server process at all? Seems like moving this into the optimizer / build process would make more sense so that we don't need to ship sass with our distributable.

In terms of the plugin API, it may make sense to go with a convention-over-configuration approach here, similar to how the webpack bundling works by directory structure.

For example, instead of specifying a JavaScript API to define SCSS entrypoints, we could require all plugins to simply have a single public/index.scss file that the optimizer could use to build the plugin's stylesheet.

The Platform could then handle loading the compiled CSS on the page. Our first pass could be very dumb (just load everything) and we could make this more sophisticated as we go (load and unload CSS based on the app on the page).

One caveat here is that plugins can have more than one application, so there's not exactly a one-to-one mapping from a single CSS output to a single application. The other caveat is that plugins may add CSS that is actually used by UI that doesn't live inside an application (eg. embeddables, chrome nav controls, global toasts or modal UI).


One thing I want to make clear is that what the Platform Team needs from the Operations Team is mostly the migration of the build-side of SCSS files, not the browser loading aspect.

@tylersmalley
Copy link
Contributor Author

@joshdover, knowing most plugin authors desire to move back to CSS in JS for multiple reasons, including CSS module support. I am thinking there might be a quick near-term solution without being blocked on the new build system.

If we can remove the new platform plugins from being included in the optimizer build in production, we can then re-introduce the CSS loader into Webpack. This would have the added benefit of immediately reducing the number of CSS files in production, as well as beginning to slowly trim down the optimizer as plugins are migrated.

Thoughts?

@joshdover
Copy link
Contributor

I'm a big fan of that option. Another benefit is that we can defer loading CSS that isn't used yet via async imports that we already have for application bundles.

@tylersmalley
Copy link
Contributor Author

@mistic lets discuss this next week.

@joshdover joshdover added Team:Operations Team label for Operations Team Feature:New Platform and removed Team:Operations Team label for Operations Team Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Dec 16, 2019
@tylersmalley
Copy link
Contributor Author

This issue is now being tracked as part of #53532

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker discuss Feature:New Platform Team:Operations Team label for Operations Team Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins.
Projects
None yet
Development

No branches or pull requests

4 participants