[kbn/optimizer] only build specified themes #70389

spalger · 2020-07-01T00:22:54Z

This PR enables devs to customize which themes the @kbn/optimizer will build. By default we continue to build the v7 light and dark themes while removing some sass compilation options (sassOptions.importer) to reduce the build time about 20% on my machine.

For the subset of devs who don't normally need to switch between themes, the KBN_OPTIMIZER_THEMES environment variable can be used to build only a single theme, such as KBN_OPTIMIZER_THEMES=v7light, which reduces the build time an additional 20% on my machine.

For devs who are working on the new v8 theme support, KBN_OPTIMIZER_THEMES=v8light,v8dark can be used to swap for those themes instead, or KBN_OPTIMIZER_THEMES=* can be used to build all four themes, at a the cost of additional build time.

When Kibana is loaded with uiSettings requesting a different theme than the ones which were built an error is written to the console for each style file imported describing how an alternate theme was loaded and how to fix the issue by setting the KBN_OPTIMIZER_THEMES environment variable.

Additional docs for theme support in the @kbn/optimizer can be found in the @kbn/optimizer readme

I would prefer to show a toast notification when this occurs, but the styles are loaded in a context where communicating with the core services would require some serious hacks, and this is only a feature for development, so I've settled on spamming the console and hoping that people notice it.

@cchaos is going to help make sure that folks working on the design side of things know about this requirement.

Removing the sass importer config, which @restrry discovered was a huge performance issue for us, was done by splitting up the styling_constants.scss file so that there is now a specific styling_constants file for each theme. We now prepend the right file as necessary when we build each stylesheet for each theme (including legacy style files which only support the v7 theme).

…-optimizer-theme-config

…lger/kibana into implement/kbn-optimizer-theme-config

- Removed the normal `_styling_constants.scss` file where possible - Still some legacy imports I don’t know what to do with

…lger/kibana into implement/kbn-optimizer-theme-config

…-optimizer-theme-config

…ach theme

…-optimizer-theme-config

…se v7)

…-optimizer-theme-config

…s are readable

…-optimizer-theme-config

spalger · 2020-07-02T00:22:45Z

Note: The rise in modules across bundles is expected as we're now building twice as many style modules for each production bundle. Most of these modules should be asynchronously loaded by applications and shouldn't cause load issues.

elasticmachine · 2020-07-02T00:22:58Z

Pinging @elastic/kibana-operations (Team:Operations)

clintandrewhall

Good for Canvas! We'll watch for the console spam.

mistic

LGTM

…-optimizer-theme-config

cjcenizal

Code LGTM, didn't test locally. Thanks for the detailed PR description!

… the total styles failed

…-optimizer-theme-config

cchaos

🎉 🙇 Thank you!

I tested locally with the defaults in all themes (errors in k8) and then with * (doesn't error in any theme) and all the correct SASS vars are passed through.

cchaos · 2020-07-02T18:40:13Z

CONTRIBUTING.md

@@ -436,7 +436,7 @@ We are still to develop a proper process to accept any contributed translations.

 When writing a new component, create a sibling SASS file of the same name and import directly into the JS/TS component file. Doing so ensures the styles are never separated or lost on import and allows for better modularization (smaller individual plugin asset footprint).

-Any JavaScript (or TypeScript) file that imports SASS (.scss) files will automatically build with the [EUI](https://elastic.github.io/eui/#/guidelines/sass) & Kibana invisibles (SASS variables, mixins, functions) from the [`styling_constants.scss` file](https://github.com/elastic/kibana/blob/master/src/legacy/ui/public/styles/_styling_constants.scss). However, any Legacy (file path includes `/legacy`) files will not.
+All SASS (.scss) files will automatically build with the [EUI](https://elastic.github.io/eui/#/guidelines/sass) & Kibana invisibles (SASS variables, mixins, functions) from the [`globals_[theme].scss` file](src/legacy/ui/public/styles/_globals_v7light.scss).


🎉 !! I like the rename too

cchaos · 2020-07-02T18:42:54Z

src/legacy/ui/public/styles/_globals_v7dark.scss

+@import '@elastic/eui/src/global_styling/functions/index';
+@import '@elastic/eui/src/global_styling/variables/index';
+@import '@elastic/eui/src/global_styling/mixins/index';


I've got a PR up in EUI to make this import a single line for all themes. Doesn't block this PR, I'll do a follow up

kibanamachine · 2020-07-02T22:04:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 3f6830f

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
advancedSettings	21	+4	17
canvas	802	+20	782
charts	158	+4	154
console	35	+4	31
core	376	+12	364
dashboard	114	+4	110
data	203	+4	199
devTools	15	+4	11
discover	111	+28	83
embeddable	37	+4	33
expressions	100	+4	96
graph	96	+4	92
home	162	+4	158
indexManagement	146	+4	142
indexPatternManagement	46	+12	34
infra	603	+4	599
ingestManager	357	+4	353
ingestPipelines	257	+12	245
inputControlVis	14	+4	10
inspector	37	+4	33
kibanaLegacy	26	+4	22
kibanaReact	245	+12	233
lens	116	+28	88
licenseManagement	95	+4	91
management	23	+8	15
maps	459	+4	455
mapsLegacy	123	+4	119
ml	388	+32	356
monitoring	200	+76	124
navigation	17	+4	13
painlessLab	16	+4	12
regionMap	157	+4	153
remoteClusters	70	+4	66
rollup	135	+4	131
savedObjects	21	+4	17
searchprofiler	30	+4	26
security	264	+44	220
share	27	+4	23
snapshotRestore	31	+4	27
spaces	116	+44	72
tileMap	159	+4	155
transform	157	+4	153
triggers_actions_ui	141	+40	101
uiActionsEnhanced	28	+4	24
upgradeAssistant	24	+4	20
visTypeMarkdown	165	+8	157
visTypeMetric	165	+8	157
visTypeTable	170	+8	162
visTypeTagcloud	167	+8	159
visTypeTimelion	175	+8	167
visTypeTimeseries	323	+4	319
visTypeVega	187	+8	179
visTypeVislib	238	+8	230
visualizations	26	+4	22
visualize	200	+16	184
watcher	47	+4	43
total	-	+580	-

History

💔 Build #58386 failed d0f1d15
💔 Build #58364 failed 80cf271
💚 Build #58297 succeeded 5d508c9
💚 Build #58084 succeeded 4b5bd8c
💔 Build #58035 failed e498718

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Co-authored-by: spalger <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: cchaos <[email protected]> # Conflicts: # packages/kbn-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap

Co-authored-by: spalger <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: cchaos <[email protected]>

* master: (32 commits) [Ingest Pipelines] Load from json (elastic#70297) [Rum Dashbaord] Rum selected service view (elastic#70579) [Uptime] Prevent duplicate requests on load for index status (elastic#70585) [ML] Changing shared module setup function parameters (elastic#70589) [Ingest Manager] Add ability to sort to agent configs and package configs (elastic#70676) [Alerting] document requirements for developing new action types (elastic#69164) Fixed adding an extra space character on selecting alert variable in action text fields (elastic#70028) [Maps] show vector tile labels on top (elastic#69444) chore(NA): upgrade to lodash@4 (elastic#69868) Add Snapshot Restore README with quick-testing steps. (elastic#70494) [EPM] Use higher priority than default templates (elastic#70640) [Maps] Fix cannot select Solid fill-color when removing fields (elastic#70621) [kbn/optimizer] only build specified themes (elastic#70389) Fix saved query modal overlay (elastic#68826) Update component templates list to render empty prompt inside of content container. Show detail panel when deep-linked, even if there are no component templates. (elastic#70633) [Security Solution] Renames the `Investigate in Resolver` Timeline action (elastic#70634) fix 400 error on initial signals search (elastic#70618) [Maps] fix unable to edit heatmap metric (elastic#70606) Update network idle timeout (elastic#70629) [APM] Disable flaky useFetcher test (elastic#70638) ...

[kbn/optimizer] only build specified themes

c453894

spalger added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jul 1, 2020

spalger mentioned this pull request Jul 1, 2020

[kbn/optimizer] style performance improvements and expanded v8 theme support #70353

Closed

4 tasks

elasticmachine and others added 24 commits June 30, 2020 20:17

Merge branch 'master' into implement/kbn-optimizer-theme-config

36ecb1b

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

1fa1000

…-optimizer-theme-config

build all themes when building the dist

6a09eac

fix outdated types and remove @ts-ignore usage

5690ce6

Merge branch 'implement/kbn-optimizer-theme-config' of github.com:spa…

44b84ab

…lger/kibana into implement/kbn-optimizer-theme-config

[SASS] 4 _styling_constants_[theme].scss files

6fa8873

- Removed the normal `_styling_constants.scss` file where possible - Still some legacy imports I don’t know what to do with

add tests

5c541ce

Merge branch 'implement/kbn-optimizer-theme-config' of github.com:spa…

3924b25

…lger/kibana into implement/kbn-optimizer-theme-config

integrate new styling_constants* files

2f32a66

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

8f339bb

…-optimizer-theme-config

remove sass importer, now that we have specific constants files for e…

0e6e023

…ach theme

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

1d32ef2

…-optimizer-theme-config

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

5891b40

…-optimizer-theme-config

unify automatic insertion of globals with legacy styles (which only u…

a8bfe97

…se v7)

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

a30f343

…-optimizer-theme-config

revert usage of global_styling/index as it creates duplicate selectors

645b4b0

rename globals to _globals_[theme].scss

7d2cc0b

use relative path for globals link

8c9ff5f

expand docs and opitions for new themes param

e498718

log a warning when only a subset of themes are being built

3ddeed1

use plural KBN_OPTIMIZER_THEMES env var

5899072

fix v8 import paths and include unthemed imports

54e67cd

parse actual lines from worker stdio so that sass deprecation message…

552c35c

…s are readable

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

3874e64

…-optimizer-theme-config

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

4b5bd8c

…-optimizer-theme-config

spalger marked this pull request as ready for review July 2, 2020 00:22

spalger requested review from a team as code owners July 2, 2020 00:22

clintandrewhall approved these changes Jul 2, 2020

View reviewed changes

mistic approved these changes Jul 2, 2020

View reviewed changes

elasticmachine and others added 4 commits July 2, 2020 09:17

Merge branch 'master' into implement/kbn-optimizer-theme-config

5d508c9

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

4670722

…-optimizer-theme-config

remove unnecessary globals, rely on theme tag everywhere

80cf271

add docs about using the KBN_OPTIMIZER_THEMES env var

d0f1d15

cjcenizal approved these changes Jul 2, 2020

View reviewed changes

spalger added 4 commits July 2, 2020 10:51

improve error message to make it more obvious that a specific part of…

1a4924e

… the total styles failed

build v7light and v7dark by default

addbc68

Merge branch 'master' of github.com:elastic/kibana into implement/kbn…

4bea131

…-optimizer-theme-config

handle undefined __kbnThemeTag__ globals caused by test runners

ca991fc

cchaos approved these changes Jul 2, 2020

View reviewed changes

update snapshots

3f6830f

spalger merged commit f5b2800 into elastic:master Jul 2, 2020

spalger deleted the implement/kbn-optimizer-theme-config branch July 2, 2020 22:06

This was referenced Jul 2, 2020

[kbn/optimizer] Should we build both light and dark themes by default? #70670

Open

[7.x] [kbn/optimizer] only build specified themes (#70389) #70671

Merged

spalger added a commit that referenced this pull request Jul 3, 2020

[7.x] [kbn/optimizer] only build specified themes (#70389) (#70671)

104d4ea

Co-authored-by: spalger <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: cchaos <[email protected]>

spalger mentioned this pull request Jul 10, 2020

[kbn/optimizer] meta issue #68321

Closed

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[kbn/optimizer] only build specified themes #70389

[kbn/optimizer] only build specified themes #70389

spalger commented Jul 1, 2020 •

edited

Loading

spalger commented Jul 2, 2020

elasticmachine commented Jul 2, 2020

clintandrewhall left a comment

mistic left a comment

cjcenizal left a comment

cchaos left a comment

cchaos Jul 2, 2020

cchaos Jul 2, 2020

kibanamachine commented Jul 2, 2020

[kbn/optimizer] only build specified themes #70389

[kbn/optimizer] only build specified themes #70389

Conversation

spalger commented Jul 1, 2020 • edited Loading

spalger commented Jul 2, 2020

elasticmachine commented Jul 2, 2020

clintandrewhall left a comment

Choose a reason for hiding this comment

mistic left a comment

Choose a reason for hiding this comment

cjcenizal left a comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

cchaos Jul 2, 2020

Choose a reason for hiding this comment

cchaos Jul 2, 2020

Choose a reason for hiding this comment

kibanamachine commented Jul 2, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

History

spalger commented Jul 1, 2020 •

edited

Loading