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

Remove React.forwardRef #3287

Merged
merged 4 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading