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.

Sign up for GitHub

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

Use React hoistable styles #3295

Draft
wants to merge 2 commits into
base: next
Choose a base branch
from
Draft

Use React hoistable styles #3295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d49f20a
Use React hoistable styles
Andarist Dec 15, 2024
0439d5a
remove debugger
Andarist Dec 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/cache/__tests__/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('throws correct error with invalid key', () => {
}).toThrowErrorMatchingSnapshot()
})

test('should accept insertionPoint option', () => {
test.skip('should accept insertionPoint option', () => {
const head = safeQuerySelector('head')

head.innerHTML = `
Expand All @@ -34,7 +34,7 @@ test('should accept insertionPoint option', () => {
expect(document.head).toMatchSnapshot()
})

test('should accept container option', () => {
test.skip('should accept container option', () => {
const body = safeQuerySelector('body')

body.innerHTML = `
Expand Down
6 changes: 4 additions & 2 deletions packages/cache/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ let createCache = (options: Options): EmotionCache => {
let insert: (
selector: string,
serialized: SerializedStyles,
sheet: StyleSheet,
sheet: Pick<StyleSheet, 'insert'>,
shouldCache: boolean
) => string | void
) => string | undefined
const omnipresentPlugins = [compat, removeLabel]

if (isDevelopment) {
Expand Down Expand Up @@ -180,6 +180,8 @@ let createCache = (options: Options): EmotionCache => {
if (shouldCache) {
cache.inserted[serialized.name] = true
}

return undefined
}
} else {
const finalizingPlugins = [stringify]
Expand Down
19 changes: 16 additions & 3 deletions packages/jest/src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,28 @@ export function getStyleElements() /*: Array<HTMLStyleElement> */ {
'jest-emotion requires jsdom. See https://jestjs.io/docs/en/configuration#testenvironment-string for more information.'
)
}
const elements = Array.from(document.querySelectorAll('style[data-emotion]'))
return elements
const elements1 = Array.from(document.querySelectorAll('style[data-emotion]'))
const elements2 = Array.from(
document.querySelectorAll('style[data-precedence^=emotion-]')
)
return elements1.concat(elements2)
}

const unique = arr => Array.from(new Set(arr))

export function getKeys(elements /*: Array<HTMLStyleElement> */) {
const keys = unique(
elements.map(element => element.getAttribute('data-emotion'))
elements.map(element => {
const dataEmotion = element.getAttribute('data-emotion')
if (dataEmotion) {
return dataEmotion.split(' ')[0]
}
const dataPrecedence = element.getAttribute('data-precedence')
if (dataPrecedence?.startsWith('emotion-')) {
return dataPrecedence.replace('emotion-', '')
}
return null
})
).filter(Boolean)
return keys
}
Expand Down
2 changes: 1 addition & 1 deletion packages/primitives-core/src/styled.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from 'react'
import React from 'react'
import { interleave } from './utils'
import { ThemeContext } from '@emotion/react'
import { createCss } from './css'
Expand Down
2 changes: 1 addition & 1 deletion packages/primitives/src/styled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from 'react'
import React from 'react'
import { StyleSheet, View, Text, Image } from 'react-primitives'
import { createStyled } from '@emotion/primitives-core'
import {
Expand Down
22 changes: 10 additions & 12 deletions packages/react/__tests__/__snapshots__/globals-are-the-worst.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,17 @@ exports[`specificity with globals 1`] = `
<html>
<head>
<style
data-emotion="css-global"
data-s=""
data-href="1ndtvd6"
data-precedence="emotion-css"
>
.some-class{color:green;}
.css-1ndtvd6{color:hotpink;}
</style>
<style
data-emotion="css"
data-emotion="css-global"
data-s=""
>
.css-1ndtvd6{color:hotpink;}
.some-class{color:green;}
</style>
</head>
<body>
Expand All @@ -39,18 +38,17 @@ exports[`specificity with globals 2`] = `
<html>
<head>
<style
data-emotion="css-global"
data-s=""
data-href="1ndtvd6"
data-precedence="emotion-css"
>
.some-class{color:yellow;}
.css-1ndtvd6{color:hotpink;}
</style>
<style
data-emotion="css"
data-emotion="css-global"
data-s=""
>
.css-1ndtvd6{color:hotpink;}
.some-class{color:yellow;}
</style>
</head>
<body>
Expand Down
2 changes: 1 addition & 1 deletion packages/react/__tests__/babel/__snapshots__/source-map-server.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`basic 1`] = `"<style data-emotion="css 15xr5n4-html">.css-15xr5n4-html{color:hotpink;}.css-15xr5n4-html:hover{color:green;}/*# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbInNvdXJjZS1tYXAtc2VydmVyLmpzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQVVNIiwiZmlsZSI6InNvdXJjZS1tYXAtc2VydmVyLmpzIiwic291cmNlc0NvbnRlbnQiOlsiLyoqIEBqc3gganN4XG4gKiBAamVzdC1lbnZpcm9ubWVudCBub2RlXG4gKi9cblxuaW1wb3J0IHsganN4IH0gZnJvbSAnQGVtb3Rpb24vcmVhY3QnXG5pbXBvcnQgeyByZW5kZXJUb1N0cmluZyB9IGZyb20gJ3JlYWN0LWRvbS9zZXJ2ZXInXG5cbnRlc3QoJ2Jhc2ljJywgKCkgPT4ge1xuICBsZXQgaHRtbCA9IHJlbmRlclRvU3RyaW5nKFxuICAgIDxkaXZcbiAgICAgIGNzcz17e1xuICAgICAgICBjb2xvcjogJ2hvdHBpbmsnLFxuICAgICAgICAnOmhvdmVyJzoge1xuICAgICAgICAgIGNvbG9yOiAnZ3JlZW4nXG4gICAgICAgIH1cbiAgICAgIH19XG4gICAgPlxuICAgICAgc29tZSBob3RwaW5rIHRleHRcbiAgICA8L2Rpdj5cbiAgKVxuICBleHBlY3QoaHRtbCkudG9NYXRjaFNuYXBzaG90KClcbn0pXG4iXX0= */</style><div class="css-15xr5n4-html">some hotpink text</div>"`;
exports[`basic 1`] = `"<style data-precedence="emotion-css" data-href="15xr5n4-html">.css-15xr5n4-html{color:hotpink;}.css-15xr5n4-html:hover{color:green;}/*# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbInNvdXJjZS1tYXAtc2VydmVyLmpzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQVVNIiwiZmlsZSI6InNvdXJjZS1tYXAtc2VydmVyLmpzIiwic291cmNlc0NvbnRlbnQiOlsiLyoqIEBqc3gganN4XG4gKiBAamVzdC1lbnZpcm9ubWVudCBub2RlXG4gKi9cblxuaW1wb3J0IHsganN4IH0gZnJvbSAnQGVtb3Rpb24vcmVhY3QnXG5pbXBvcnQgeyByZW5kZXJUb1N0cmluZyB9IGZyb20gJ3JlYWN0LWRvbS9zZXJ2ZXInXG5cbnRlc3QoJ2Jhc2ljJywgKCkgPT4ge1xuICBsZXQgaHRtbCA9IHJlbmRlclRvU3RyaW5nKFxuICAgIDxkaXZcbiAgICAgIGNzcz17e1xuICAgICAgICBjb2xvcjogJ2hvdHBpbmsnLFxuICAgICAgICAnOmhvdmVyJzoge1xuICAgICAgICAgIGNvbG9yOiAnZ3JlZW4nXG4gICAgICAgIH1cbiAgICAgIH19XG4gICAgPlxuICAgICAgc29tZSBob3RwaW5rIHRleHRcbiAgICA8L2Rpdj5cbiAgKVxuICBleHBlY3QoaHRtbCkudG9NYXRjaFNuYXBzaG90KClcbn0pXG4iXX0= */</style><div class="css-15xr5n4-html">some hotpink text</div>"`;
4 changes: 4 additions & 0 deletions packages/react/__tests__/compat/__snapshots__/browser.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
exports[`composition works from old emotion css calls 1`] = `
<html>
<head>
<style
data-href="ulmpz9"
data-precedence="emotion-css"
/>
<style
data-emotion="css"
data-s=""
Expand Down
6 changes: 3 additions & 3 deletions packages/react/__tests__/compat/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import createCache from '@emotion/cache'
import { CacheProvider } from '@emotion/react'
import { renderToString } from 'react-dom/server'

test('it works', () => {
test.skip('it works', () => {
let cache = createCache({ key: 'ssr' })
let { extractCritical } = createEmotionServer(cache)
let ele = (
Expand All @@ -18,7 +18,7 @@ test('it works', () => {
expect(extractCritical(renderToString(ele))).toMatchSnapshot()
})

test('Global component extracts the styles rather than inlines it', () => {
test.skip('Global component extracts the styles rather than inlines it', () => {
let cache = createCache({ key: 'ssr' })
let { extractCritical } = createEmotionServer(cache)
let ele = (
Expand All @@ -30,7 +30,7 @@ test('Global component extracts the styles rather than inlines it', () => {
expect(extractCritical(renderToString(ele))).toMatchSnapshot()
})

test('extracted rules have correct keys when dealing with multiple caches', () => {
test.skip('extracted rules have correct keys when dealing with multiple caches', () => {
let cache1 = createCache({ key: 'ssr-first-key' })
let { extractCritical: extractCritical1 } = createEmotionServer(cache1)
let ele1 = (
Expand Down
2 changes: 1 addition & 1 deletion packages/react/__tests__/css-cache-hash.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const utils = require('@emotion/utils')

const spy = jest.spyOn(utils, 'insertStyles')

test('does not rehash if value is css call return value', () => {
test.skip('does not rehash if value is css call return value', () => {
const val = css`
color: hotpink;
`
Expand Down
7 changes: 1 addition & 6 deletions packages/react/__tests__/css.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ afterEach(() => {

const SomeComponent = (props /*: { lol: true } */) => (props.lol ? 'yes' : 'no')

// test to make sure flow prop errors work.
// should probably try to make it so that components that require className props
// and have the css prop passed to them don't have type errors
;<SomeComponent /> // eslint-disable-line no-unused-expressions

test('thing', () => {
const { container } = render(
<div>
Expand Down Expand Up @@ -157,7 +152,7 @@ test('nested at rule', () => {
expect(container.firstChild).toMatchSnapshot()
})

test('can set speedy via custom cache', () => {
test.skip('can set speedy via custom cache', () => {
let cache = createCache({ key: 'speedy-test', speedy: true })

render(
Expand Down
33 changes: 16 additions & 17 deletions packages/react/__tests__/custom-cache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,20 @@ test('Global should "inherit" sheet class from the cache', () => {
)

expect(safeQuerySelector('head')).toMatchInlineSnapshot(`
<head>
<style
data-emotion="test-global"
data-s=""
>
/** test-global */body{width:0;}
</style>
<style
data-emotion="test"
data-s=""
>
/** test */.test-1lrxbo5{color:hotpink;}
</style>
</head>
`)
<head>
<style
data-href="1lrxbo5"
data-precedence="emotion-test"
>
.test-1lrxbo5{color:hotpink;}
</style>
<style
data-emotion="test-global"
data-s=""
>
/** test-global */body{width:0;}
</style>
Comment on lines +69 to +81
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

their order flipped, we can fix this by using precedence with -global on global styles, that will allow people to control how they are grouped - they could render dummy styles in head with those precedence set and then React will insert new styles there

the problem is that with those hoistable styles... we lose the ability to unmount global styles, are we fine with that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we lose the ability to unmount global styles, are we fine with that?

imo, yes, it's fine. I've never been a big fan of the unmounting styles is Global, it's kind of started out as "i guess we can do this so why not" but imo it's not really that valuable since people should just scope styles (whether that's literally with the css prop/etc. or with global styles but all the styles are scoped to some selector) and conditionally apply the selector rather than have conditional style insertion.

</head>
`)
})
20 changes: 10 additions & 10 deletions packages/react/__tests__/rehydration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ beforeEach(() => {
safeQuerySelector('body').innerHTML = ''
})

test("cache created in render doesn't cause a hydration mismatch", () => {
test.skip("cache created in render doesn't cause a hydration mismatch", () => {
safeQuerySelector('body').innerHTML = [
'<div id="root">',
'<style data-emotion="stl 168r6j">.stl-1pdkrhd {color: hotpink;}</style>',
Expand Down Expand Up @@ -88,7 +88,7 @@ test("cache created in render doesn't cause a hydration mismatch", () => {
expect(console.warn.mock.calls).toMatchInlineSnapshot(`[]`)
})

test('initializing another Emotion instance should not move already moved styles elements', () => {
test.skip('initializing another Emotion instance should not move already moved styles elements', () => {
safeQuerySelector('head').innerHTML = '<div id="style-container"></div>'
safeQuerySelector('body').innerHTML = [
'<div id="root">',
Expand Down Expand Up @@ -140,15 +140,15 @@ test('initializing another Emotion instance should not move already moved styles
data-emotion="stl"
data-s=""
>
.stl-168r6j{color:hotpink;}
</style>
</div>
</head>
`)
})

test('initializing another Emotion instance should not move already moved styles elements', () => {
test.skip('initializing another Emotion instance should not move already moved styles elements', () => {
safeQuerySelector('head').innerHTML = '<div id="style-container"></div>'
safeQuerySelector('body').innerHTML = '<div id="root"></div>'

Expand Down Expand Up @@ -188,15 +188,15 @@ test('initializing another Emotion instance should not move already moved styles
data-emotion="stl"
data-s=""
>
.stl-168r6j{color:hotpink;}
</style>
</div>
</head>
`)
})

test('global styles can be removed individually after rehydrating HTML SSRed with extractCriticalToChunks', async () => {
test.skip('global styles can be removed individually after rehydrating HTML SSRed with extractCriticalToChunks', async () => {
const { app, styles } = await disableBrowserEnvTemporarily(() => {
resetAllModules()

Expand Down Expand Up @@ -326,7 +326,7 @@ test('global styles can be removed individually after rehydrating HTML SSRed wit
`)
})

test('duplicated global styles can be removed safely after rehydrating HTML SSRed with extractCriticalToChunks', async () => {
test.skip('duplicated global styles can be removed safely after rehydrating HTML SSRed with extractCriticalToChunks', async () => {
const { app, styles } = await disableBrowserEnvTemporarily(() => {
resetAllModules()

Expand Down Expand Up @@ -405,7 +405,7 @@ test('duplicated global styles can be removed safely after rehydrating HTML SSRe
data-emotion="muii-global"
data-s=""
>
</style>
<style
data-emotion="muii 1lrxbo5"
Expand Down Expand Up @@ -466,7 +466,7 @@ test('duplicated global styles can be removed safely after rehydrating HTML SSRe
`)
})

test('duplicated global styles can be removed safely after rehydrating HTML SSRed with zero config approach', async () => {
test.skip('duplicated global styles can be removed safely after rehydrating HTML SSRed with zero config approach', async () => {
const { app } = await disableBrowserEnvTemporarily(() => {
resetAllModules()

Expand Down Expand Up @@ -605,7 +605,7 @@ test('duplicated global styles can be removed safely after rehydrating HTML SSRe
`)
})

describe('useId', () => {
describe.skip('useId', () => {
test('no hydration mismatch for styled when using useId', async () => {
const finalHTML = await disableBrowserEnvTemporarily(() => {
resetAllModules()
Expand Down
1 change: 1 addition & 0 deletions packages/react/__tests__/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ let cases = {
}
},
'styled with keyframes': {
skip: true,
render: () => {
const SomeComponent = styled.div({
animation: `${keyframes({
Expand Down
Loading
Loading