Skip to content

Commit

Permalink
Fixed an issue with css prop type not being added to all components…
Browse files Browse the repository at this point in the history
… that accept a string `className` correctly (#2140)

* Invert `extends` in WithConditionalCSSProp

* Add tests to WithConditionalCSSProp type

* Update jsx-namespace.d.ts

* Use Public API in test

* Remember that components can have more than one prop 🤦

* Work around Prettier betrayal

* Add changeset

* Clean up prettier-ignore

* Add dtslint $ExpectType comments

* Update .changeset/perfect-spoons-whisper.md

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
  • Loading branch information
hasparus and Andarist committed Dec 1, 2020
1 parent f43aadf commit e584353
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/perfect-spoons-whisper.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@emotion/react': patch
---

Fixed an issue with `css` prop type not being added to all components that accept a string `className` correctly.
4 changes: 3 additions & 1 deletion packages/react/types/jsx-namespace.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { Interpolation } from '@emotion/serialize'
import { Theme } from './index'

type WithConditionalCSSProp<P> = 'className' extends keyof P
? (P extends { className?: string } ? P & { css?: Interpolation<Theme> } : P)
? string extends P['className' & keyof P]
? P & { css?: Interpolation<Theme> }
: P
: P

// unpack all here to avoid infinite self-referencing when defining our own JSX namespace
Expand Down
55 changes: 51 additions & 4 deletions packages/react/types/tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
keyframes,
withEmotionCache
} from '@emotion/react'
import { JSX as EmotionJSX } from '@emotion/react/jsx-runtime'

declare module '@emotion/react' {
// tslint:disable-next-line: strict-export-declare-modifiers
Expand Down Expand Up @@ -159,14 +160,14 @@ const anim1 = keyframes`

// TS@next reports an error on a different line, so this has to be in a single line so `test:typescript` can validate this on all TS versions correctly
// $ExpectError
;<CompWithoutClassNameSupport prop1="test" css={{ backgroundColor: 'hotpink' }} />
;<CompWithoutClassNameSupport prop1="test" css={{ color: 'hotpink' }} />

const MemoedCompWithoutClassNameSupport = React.memo(
CompWithoutClassNameSupport
)
// TS@next reports an error on a different line, so this has to be in a single line so `test:typescript` can validate this on all TS versions correctly
// $ExpectError
;<MemoedCompWithoutClassNameSupport prop1="test" css={{ backgroundColor: 'hotpink' }} />
;<MemoedCompWithoutClassNameSupport prop1="test" css={{ color: 'hotpink' }} />
}

{
Expand All @@ -181,6 +182,52 @@ const anim1 = keyframes`
// https://github.com/DefinitelyTyped/DefinitelyTyped/issues/40993
// this is really problematic behaviour by @types/react IMO
// but it's what @types/react does so let's not break it.
const CompWithImplicitChildren: React.FC = () => null;
;<CompWithImplicitChildren>content<div/></CompWithImplicitChildren>
const CompWithImplicitChildren: React.FC = () => null
;<CompWithImplicitChildren>
content
<div />
</CompWithImplicitChildren>
}

// Tests for WithConditionalCSSProp
{
// $ExpectType Interpolation<Theme>
type _HasCssPropAsIntended3 = EmotionJSX.LibraryManagedAttributes<
{},
{
className?: string
}
>['css']

// $ExpectType Interpolation<Theme>
type _HasCssPropAsIntended4 = EmotionJSX.LibraryManagedAttributes<
{},
{
className: string
}
>['css']

// $ExpectType Interpolation<Theme>
type _HasCssPropAsIntended5 = EmotionJSX.LibraryManagedAttributes<
{},
{
className?: unknown
}
>['css']

// $ExpectType Interpolation<Theme>
type _HasCssPropAsIntended6 = EmotionJSX.LibraryManagedAttributes<
{},
{
className?: string | Array<string>
}
>['css']

// $ExpectType false
type _NoCssPropAsIntended1 = 'css' extends keyof EmotionJSX.LibraryManagedAttributes<
{},
{ className?: undefined }
>
? true
: false
}

0 comments on commit e584353

Please sign in to comment.