Use `@emotion/server` for server-side security prompts #142662

cee-chen · 2022-10-04T19:30:34Z

Summary

In #134919, EUI updated Security's prompt_page template to correctly support the CSS of Emotion-converted components. At the time, styling was working and Emotion was outputting its CSS styles into <body> tags. At some point between then (June) and now, Emotion styles stopped rendering correctly / as expected.

The solution is to this is to utilize @emotion/server for server-side rendering. In addition to fixing the now-broken/missing Emotion styles, this also has the added benefits of:

Ensuring Emotion styles are in the <head> and not the <body>, reducing potential CSS specificity issues
Gets rid of server-side warnings/logs complaining about :first-child/:nth-child selectors
Only loads critical CSS necessary for that page, which is a nice microperf bonus

This PR also incidentally updates all Kibana's various @emotion dependencies to the latest minors.

Before

After

cee-chen · 2022-10-04T19:35:40Z

x-pack/plugins/security/server/prompt_page.tsx

+        {/* eslint-disable-next-line react/no-danger */}
+        <style dangerouslySetInnerHTML={{ __html: `</style>${emotionStyles}` }} />


A quick explanation on why this creates a blank <style></style> tag:

Since Emotion is returning its styles as one giant HTML string (including specific classes/attributes on its <style> tags that we likely should not ignore), we need to use dangerouslySetInnerHTML

The number of children allowed within the <head> tag is limited, e.g. no <div>s or otherwise handy 'grouping' type tag. If you try to use a tag that normally does not have children, e.g. the <meta> tag, Kibana/React will error with [ERROR][http.server.Kibana] Error: meta is a void element tag and must neither have `children` nor use `dangerouslySetInnerHTML`.

style was therefore the tag that made the most sense, but causes weird/unnecessary nesting behavior, hence the prepended </style>. See this StackOverflow answer for how this works. This ends up creating a blank <style></style> tag, but that really isn't the worst thing in the world, and is an unfortunate necessary evil until React supports dangerouslySetInnerHTML on fragments.

cee-chen · 2022-10-04T19:53:04Z

@kc13greiner Does this need to be backported to 8.4? Not sure if Ops would have any issue with that considering the extra dependency/dependency upgrades, but in general they shouldn't be risky.

kc13greiner

LGTM!

legrego · 2022-10-04T20:04:06Z

Does this need to be backported to 8.4?

No, 8.5 is sufficient here. The regression was introduced via #140323, which wasn't backported to 8.4 as far as I can tell.

Thanks for working on this fix for us, @constancecchen!

kibana-ci · 2022-10-04T22:15:17Z

💚 Build Succeeded

Buildkite Build
Commit: 4e46258

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	385	390	+5
`dataViewManagement`	176	181	+5
`discover`	529	534	+5
`expressionGauge`	81	86	+5
`expressionHeatmap`	113	118	+5
`expressionPartitionVis`	95	100	+5
`expressionXY`	145	150	+5
`kibanaOverview`	101	106	+5
`lens`	922	927	+5
`maps`	996	1001	+5
`visualizations`	352	357	+5
total			+55

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`dashboard`	418.3KB	418.7KB	+406.0B
`dataViewManagement`	148.3KB	148.6KB	+391.0B
`discover`	478.2KB	478.2KB	+19.0B
`expressionGauge`	18.0KB	18.3KB	+376.0B
`expressionHeatmap`	90.7KB	91.1KB	+376.0B
`expressionPartitionVis`	50.6KB	51.0KB	+376.0B
`expressionXY`	118.0KB	118.4KB	+376.0B
`kibanaOverview`	46.3KB	46.7KB	+396.0B
`lens`	1.3MB	1.3MB	+411.0B
`maps`	2.6MB	2.6MB	+406.0B
`visualizations`	239.4KB	239.8KB	+406.0B
total			+3.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`charts`	45.3KB	45.3KB	+3.0B
`discover`	26.7KB	27.0KB	+376.0B
`kbnUiSharedDeps-npmDll`	5.0MB	5.0MB	+1.1KB
total			+1.5KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`security`	20	21	+1

Total ESLint disabled count

id	before	after	diff
`security`	22	23	+1

History

💔 Build #78094 failed 98172fa

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

kibanamachine · 2022-10-04T22:19:51Z

💔 All backports failed

Status	Branch	Result
❌	8.5	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 142662

Questions ?

Please refer to the Backport tool documentation

cee-chen · 2022-10-04T23:40:11Z

@legrego @kc13greiner I think I lost track of my FF releases 🙈 Is 8.5 the one that just went into FF and 8.6 is latest main? If so, do we need to backport this manually to 8.5?

edit: backported #142689

…2689) * Update all `@emotion` dependencies to latest * Install `@emotion/server` * Use Emotion server-side rendering for security prompt pages * snapshots (cherry picked from commit 899081a)

legrego · 2022-10-05T11:10:56Z

I think I lost track of my FF releases 🙈 Is 8.5 the one that just went into FF and 8.6 is latest main? If so, do we need to backport this manually to 8.5?

It looks like you figured this out already, but to close the loop: Yes, 8.5 just went into FF, so main represents 8.6.

* Update all `@emotion` dependencies to latest * Install `@emotion/server` * Use Emotion server-side rendering for security prompt pages * snapshots

cee-chen added 3 commits October 4, 2022 11:54

Update all @emotion dependencies to latest

72fac53

Install @emotion/server

d1b7b92

Use Emotion server-side rendering for security prompt pages

98172fa

cee-chen added release_note:skip Skip the PR/issue when compiling release notes v8.5.0 labels Oct 4, 2022

cee-chen requested review from thompsongl and kc13greiner October 4, 2022 19:30

cee-chen requested a review from a team as a code owner October 4, 2022 19:30

cee-chen commented Oct 4, 2022

View reviewed changes

kc13greiner approved these changes Oct 4, 2022

View reviewed changes

snapshots

4e46258

cee-chen enabled auto-merge (squash) October 4, 2022 21:28

cee-chen merged commit 899081a into elastic:main Oct 4, 2022

cee-chen deleted the eui-emotion-ssr branch October 4, 2022 22:15

kibanamachine added the v8.6.0 label Oct 4, 2022

cee-chen mentioned this pull request Oct 5, 2022

[8.5] Use @emotion/server for server-side security prompts #142689

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `@emotion/server` for server-side security prompts #142662

Use `@emotion/server` for server-side security prompts #142662

cee-chen commented Oct 4, 2022 •

edited by kibanamachine

Loading

cee-chen Oct 4, 2022 •

edited

Loading

cee-chen commented Oct 4, 2022

kc13greiner left a comment

legrego commented Oct 4, 2022

kibana-ci commented Oct 4, 2022

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Oct 4, 2022

cee-chen commented Oct 4, 2022 •

edited

Loading

legrego commented Oct 5, 2022

		{/* eslint-disable-next-line react/no-danger */}
		<style dangerouslySetInnerHTML={{ __html: `</style>${emotionStyles}` }} />

Use @emotion/server for server-side security prompts #142662

Use @emotion/server for server-side security prompts #142662

Conversation

cee-chen commented Oct 4, 2022 • edited by kibanamachine Loading

Summary

Before

After

cee-chen Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

cee-chen commented Oct 4, 2022

kc13greiner left a comment

Choose a reason for hiding this comment

legrego commented Oct 4, 2022

kibana-ci commented Oct 4, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Oct 4, 2022

💔 All backports failed

Manual backport

Questions ?

cee-chen commented Oct 4, 2022 • edited Loading

legrego commented Oct 5, 2022

Use `@emotion/server` for server-side security prompts #142662

Use `@emotion/server` for server-side security prompts #142662

cee-chen commented Oct 4, 2022 •

edited by kibanamachine

Loading

cee-chen Oct 4, 2022 •

edited

Loading

cee-chen commented Oct 4, 2022 •

edited

Loading