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

@emotion/core typings are leaking #20071

Closed
ling1726 opened this issue Oct 1, 2021 · 2 comments
Closed

@emotion/core typings are leaking #20071

ling1726 opened this issue Oct 1, 2021 · 2 comments
Assignees

Comments

@ling1726
Copy link
Member

ling1726 commented Oct 1, 2021

Components can't be used in simple create-react-app projects with the following error:

image

What

To make PopoverSurface work the code needs to be written this way:

<PopoverSurface css={{}}>Great workaround \o/</PopoverSurface>

otherwise TS will continuously error

Why ?

All components were refactored to use type from interface following #19850 they are always inlined during build by typescript. The addition of IntrinsicShorthandProps in #19642 is another major factor.

Let's look at the example of PopoverSurface

export const PopoverSurface = React.forwardRef<HTMLDivElement, PopoverSurfaceProps>((props, ref) => {

The resulting props for the root slot includes a property css (as seen in the above error). Here is the type declaration in @emotion/core:

declare module 'react' {
  interface DOMAttributes<T> {
    🚨🚨🚨🚨 This leaks into the props of `PopoverSurface`
    css?: InterpolationWithTheme<any>
  }
}

declare global {
  namespace JSX {
    /**
     * Do we need to modify `LibraryManagedAttributes` too,
     * to make `className` props optional when `css` props is specified?
     */

    interface IntrinsicAttributes {
      🚨🚨🚨🚨 This leaks into the props of `PopoverSurface`
      css?: InterpolationWithTheme<any>
    }
  }
}

The css property leaks into the expanded type. There is no way for typescript to exclude type roots in the build process and a way to do so is an open issue microsoft/TypeScript#37053.

Solutions

Explicitly typing the return of forwardRef which is already proposed in #19923

⚠️⚠️ This does mean that types will not leak in the future thanks to type inlining, read article in the next section

export const PopoverSurface: React.ForwardRefExoticComponent<PopoverSurfaceProps> = React.forwardRef<HTMLDivElement, PopoverSurfaceProps>((props, ref) => {

Use interfaces over types

interface are not inlined during the typescript build process and is why Bloomberg has advocated not using type altogether: https://www.techatbloomberg.com/blog/10-insights-adopting-typescript-at-scale/

@ling1726 ling1726 self-assigned this Oct 1, 2021
@bsunderhus
Copy link
Contributor

bsunderhus commented Oct 1, 2021

Let's just merge #19923

@layershifter
Copy link
Member

The progress is tracked in #20079.

@microsoft microsoft locked as resolved and limited conversation to collaborators Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants