Skip to content

Commit

Permalink
Remove React.forwardRef (#3287)
Browse files Browse the repository at this point in the history
* Remove `React.forwardRef`

* fix some things

* skip ref-based tests

* remoe redundant ref in type level tests
  • Loading branch information
Andarist authored Dec 10, 2024
1 parent 55ef071 commit 61fcc80
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 114 deletions.
7 changes: 7 additions & 0 deletions .changeset/twelve-gifts-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@emotion/primitives-core': major
'@emotion/styled': major
'@emotion/react': major
---

Refs are no longer internally forwarded using `React.forwardRef`.
6 changes: 3 additions & 3 deletions packages/jest/test/printer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('jest-emotion with dom elements', () => {
expect(output).toMatchSnapshot()
})

test('replaces class names and inserts styles into DOM element snapshots', () => {
test.skip('replaces class names and inserts styles into DOM element snapshots', () => {
const divRef = React.createRef()
render(
<div css={divStyle} ref={divRef}>
Expand Down Expand Up @@ -81,7 +81,7 @@ describe('jest-emotion with DOM elements disabled', () => {
expect(output).toMatchSnapshot()
})

test('does not replace class names or insert styles into DOM element snapshots', () => {
test.skip('does not replace class names or insert styles into DOM element snapshots', () => {
const divRef = React.createRef()
render(
<div css={divStyle} ref={divRef}>
Expand All @@ -97,7 +97,7 @@ describe('jest-emotion with DOM elements disabled', () => {
})
})

test('allows to opt-out from styles printing', () => {
test.skip('allows to opt-out from styles printing', () => {
const emotionPlugin = createSerializer({ includeStyles: false })

const divStyle = css`
Expand Down
7 changes: 2 additions & 5 deletions packages/primitives-core/src/styled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function createStyled(
}

// do we really want to use the same infra as the web since it only really uses theming?
let Styled = React.forwardRef<unknown, StyledProps>((props, ref) => {
let Styled: React.FC<StyledProps> = props => {
const finalTag =
(shouldUseAs && (props.as as React.ElementType)) || component

Expand Down Expand Up @@ -78,12 +78,9 @@ export function createStyled(
}
}
newProps.style = [css.apply(mergedProps, styles), props.style]
if (ref) {
newProps.ref = ref
}

return React.createElement(finalTag, newProps)
})
}

Styled.displayName = `emotion(${getDisplayName(component)})`

Expand Down
2 changes: 1 addition & 1 deletion packages/primitives/test/emotion-primitives.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('Emotion primitives', () => {
expect(tree).toMatchSnapshot()
})

test('ref', () => {
test.skip('ref', () => {
const StyledText = styled.Text`
color: hotpink;
`
Expand Down
2 changes: 1 addition & 1 deletion packages/react/__tests__/ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { render, cleanup } from '@testing-library/react'

afterEach(cleanup)

test('ref works', () => {
test.skip('ref works', () => {
let ref = React.createRef()
let { getByTestId } = render(
<div data-testid="test" css={{ color: 'hotpink' }} ref={ref} />
Expand Down
10 changes: 4 additions & 6 deletions packages/react/__tests__/with-theme.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,9 @@ test(`withTheme(Comp) hoists non-react static class properties`, () => {
)
})

test('should forward the ref', () => {
class SomeComponent extends React.Component {
render() {
return this.props.theme.color
}
test.skip('should forward the ref', () => {
function SomeComponent(props) {
return <div ref={props.ref}>{props.theme.color}</div>
}

const ComponentWithTheme = withTheme(SomeComponent)
Expand All @@ -48,5 +46,5 @@ test('should forward the ref', () => {
<ComponentWithTheme ref={ref} />
</ThemeProvider>
)
expect(ref.current).toBeInstanceOf(SomeComponent)
expect(ref.current).toBeInstanceOf(HTMLDivElement)
})
23 changes: 7 additions & 16 deletions packages/react/src/context.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import { useContext, forwardRef } from 'react'
import { useContext } from 'react'
import createCache, { EmotionCache } from '@emotion/cache'
import isDevelopment from '#is-development'
import isBrowser from '#is-browser'
Expand Down Expand Up @@ -27,23 +27,14 @@ export let __unsafe_useEmotionCache = function useEmotionCache() {
return useContext(EmotionCacheContext)
}

let withEmotionCache = function withEmotionCache<Props, RefType = any>(
func: (
props: React.PropsWithoutRef<Props>,
context: EmotionCache,
ref?: React.ForwardedRef<RefType>
) => React.ReactNode
):
| React.FC<React.PropsWithoutRef<Props> & React.RefAttributes<RefType>>
| React.ForwardRefExoticComponent<
React.PropsWithoutRef<Props> & React.RefAttributes<RefType>
> {
return forwardRef<RefType, Props>((props, ref) => {
let withEmotionCache = function withEmotionCache<Props>(
func: (props: Props, context: EmotionCache) => React.ReactNode
): React.FC<Props> {
return props => {
// the cache will never be null in the browser
let cache = useContext(EmotionCacheContext)!

return func(props, cache, ref)
})
return func(props, cache)
}
}

if (!isBrowser) {
Expand Down
126 changes: 59 additions & 67 deletions packages/react/src/emotion-element.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,82 +109,74 @@ const Insertion = ({
return null
}

let Emotion = /* #__PURE__ */ withEmotionCache<EmotionProps>(
(props, cache, ref) => {
let cssProp = props.css as EmotionProps['css']

// so that using `css` from `emotion` and passing the result to the css prop works
// not passing the registered cache to serializeStyles because it would
// make certain babel optimisations not possible
if (
typeof cssProp === 'string' &&
cache.registered[cssProp] !== undefined
) {
cssProp = cache.registered[cssProp]
}
let Emotion = /* #__PURE__ */ withEmotionCache<EmotionProps>((props, cache) => {
let cssProp = props.css as EmotionProps['css']

// so that using `css` from `emotion` and passing the result to the css prop works
// not passing the registered cache to serializeStyles because it would
// make certain babel optimisations not possible
if (typeof cssProp === 'string' && cache.registered[cssProp] !== undefined) {
cssProp = cache.registered[cssProp]
}

let WrappedComponent = props[
typePropName
] as EmotionProps[typeof typePropName]
let registeredStyles = [cssProp]
let className = ''

if (typeof props.className === 'string') {
className = getRegisteredStyles(
cache.registered,
registeredStyles,
props.className
)
} else if (props.className != null) {
className = `${props.className} `
}
let WrappedComponent = props[
typePropName
] as EmotionProps[typeof typePropName]
let registeredStyles = [cssProp]
let className = ''

let serialized = serializeStyles(
if (typeof props.className === 'string') {
className = getRegisteredStyles(
cache.registered,
registeredStyles,
undefined,
React.useContext(ThemeContext)
props.className
)
} else if (props.className != null) {
className = `${props.className} `
}

if (isDevelopment && serialized.name.indexOf('-') === -1) {
let labelFromStack = props[labelPropName]
if (labelFromStack) {
serialized = serializeStyles([
serialized,
'label:' + labelFromStack + ';'
])
}
}
let serialized = serializeStyles(
registeredStyles,
undefined,
React.useContext(ThemeContext)
)

className += `${cache.key}-${serialized.name}`

const newProps: Record<string, unknown> = {}
for (let key in props) {
if (
hasOwn.call(props, key) &&
key !== 'css' &&
key !== typePropName &&
(!isDevelopment || key !== labelPropName)
) {
newProps[key] = props[key]
}
}
newProps.className = className
if (ref) {
newProps.ref = ref
if (isDevelopment && serialized.name.indexOf('-') === -1) {
let labelFromStack = props[labelPropName]
if (labelFromStack) {
serialized = serializeStyles([
serialized,
'label:' + labelFromStack + ';'
])
}
}

return (
<>
<Insertion
cache={cache}
serialized={serialized}
isStringTag={typeof WrappedComponent === 'string'}
/>
<WrappedComponent {...newProps} />
</>
)
className += `${cache.key}-${serialized.name}`

const newProps: Record<string, unknown> = {}
for (let key in props) {
if (
hasOwn.call(props, key) &&
key !== 'css' &&
key !== typePropName &&
(!isDevelopment || key !== labelPropName)
) {
newProps[key] = props[key]
}
}
)
newProps.className = className

return (
<>
<Insertion
cache={cache}
serialized={serialized}
isStringTag={typeof WrappedComponent === 'string'}
/>
<WrappedComponent {...newProps} />
</>
)
})

if (isDevelopment) {
Emotion.displayName = 'EmotionCssPropInternal'
Expand Down
13 changes: 5 additions & 8 deletions packages/react/src/theming.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,16 @@ export function withTheme<
C extends React.ComponentType<React.ComponentProps<C>>
>(
Component: C
): React.ForwardRefExoticComponent<
): React.FC<
DistributiveOmit<React.ComponentProps<C>, 'theme'> & { theme?: Theme }
>
export function withTheme(
Component: React.ComponentType<any>
): React.ForwardRefExoticComponent<any> {
> {
const componentName = Component.displayName || Component.name || 'Component'

let WithTheme = React.forwardRef(function render(props, ref) {
let WithTheme: React.FC<any> = function render(props) {
let theme = React.useContext(ThemeContext)

return <Component theme={theme} ref={ref} {...props} />
})
return <Component theme={theme} {...props} />
}

WithTheme.displayName = `WithTheme(${componentName})`

Expand Down
2 changes: 1 addition & 1 deletion packages/react/types/tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const ComponentWithCache = withEmotionCache((_props: {}, cache) => {
/>
)
})
;<ComponentWithCache ref={() => {}} />
;<ComponentWithCache />
;<div css={{}}>
<h1
css={css`
Expand Down
2 changes: 1 addition & 1 deletion packages/styled/__tests__/styled-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { render, cleanup } from '@testing-library/react'

afterEach(cleanup)

test('ref', () => {
test.skip('ref', () => {
const H1 = styled.h1`
font-size: 12px;
`
Expand Down
5 changes: 1 addition & 4 deletions packages/styled/src/base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ const createStyled = (tag: ElementType, options?: StyledOptions) => {
}

const Styled: ElementType = withEmotionCache(
(props: Record<string, unknown>, cache, ref) => {
(props: Record<string, unknown>, cache) => {
const FinalTag =
(shouldUseAs && (props.as as React.ElementType)) || baseTag

Expand Down Expand Up @@ -170,9 +170,6 @@ const createStyled = (tag: ElementType, options?: StyledOptions) => {
}
}
newProps.className = className
if (ref) {
newProps.ref = ref
}

return (
<>
Expand Down
2 changes: 1 addition & 1 deletion packages/styled/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ describe('styled', () => {
expect(tree).toMatchSnapshot()
})

test('ref', () => {
test.skip('ref', () => {
const H1 = styled.h1`
font-size: 12px;
`
Expand Down

0 comments on commit 61fcc80

Please sign in to comment.