Type changes around StyledOptions/FilteringStyledOptions and `sho…

…uldForwardProp` (#2333) * chore(style): let StyledOptions generic argument be optional * chore(native): let StyledOptions generic argument be optional * test(types): add tests for StyledOptions iface * feat(types): add keys assertion for StyledOptions generic * fix(styled): add StyledOptions type to CreateStyledComponent call signature fixes #2333 (comment) * test(styled): check type StyledOpts for composite component * test(styled): revert erroneous changes * test(styled): clarify StyledOptsBroken test case * test: refactor `styledOptions` generics tests * test: add more tests for StyledOptions * test: linting * test: tweak ups * feat: expose FilteringStyledOptions * fix: expost `FilteringStyledOptions` from native, tweak up some tests, add some comments * chore: use consistent default props type for both StyledOptions and FilteringStyledOptions * chore: snap #3 * styled.shouldForwardProp: require prop to be a string * Add PropertyKey->string changesets * Remove redundant `compilerOptions` * make `shouldForwardProp` always contravariant * Type fixes * Tweak `FilteringStyledOptions` and `StyledOptions` interfaces * Change `Props` constraint * Tweak type-level tests around `shouldForwardProp` * add changesets Co-authored-by: Sam Magura <[email protected]> Co-authored-by: Mateusz Burzyński <[email protected]>
emotion-js · Jun 12, 2022 · 3055efd · 3055efd
1 parent f5c80c8
commit 3055efd
Show file tree

Hide file tree

Showing 11 changed files with 166 additions and 19 deletions.
diff --git a/.changeset/good-cars-roll.md b/.changeset/good-cars-roll.md
@@ -0,0 +1,10 @@
+---
+'@emotion/native': patch
+'@emotion/styled': patch
+---
+
+pr: #2759
+author: @srmagura
+author: @Andarist
+
+Change the argument of the `shouldForwardProp` option of `styled` from `PropertyKey` to `string` in the TypeScript definitions.
diff --git a/.changeset/purple-ladybugs-think.md b/.changeset/purple-ladybugs-think.md
@@ -0,0 +1,8 @@
+---
+'@emotion/is-prop-valid': patch
+---
+
+pr: #2759
+author: @srmagura
+
+Change the type of the argument to `isPropValid` from `PropertyKey` to `string` in the TypeScript definitions.
diff --git a/.changeset/tall-flies-smoke.md b/.changeset/tall-flies-smoke.md
@@ -0,0 +1,8 @@
+---
+'@emotion/native': patch
+'@emotion/styled': patch
+---
+
+author: @Andarist
+
+`shouldForwardProp` has been changed from being a bivariant method to a contravariant function - it improves the type-safety for those that type this option.
diff --git a/.changeset/thirty-boats-hang.md b/.changeset/thirty-boats-hang.md
@@ -0,0 +1,6 @@
+---
+'@emotion/native': patch
+'@emotion/styled': patch
+---
+
+`FilteringStyledOptions` and `StyledOptions` types no longer require a type argument for the `Props` generic.
diff --git a/packages/is-prop-valid/types/index.d.ts b/packages/is-prop-valid/types/index.d.ts
@@ -1,5 +1,5 @@
 // Definitions by: Junyoung Clare Jang <https://github.com/Ailrun>
 // TypeScript Version: 2.1
 
-declare function isPropValid(string: PropertyKey): boolean
+declare function isPropValid(prop: string): boolean
 export default isPropValid
diff --git a/packages/native/types/base.d.ts b/packages/native/types/base.d.ts
@@ -60,14 +60,14 @@ export type Interpolation<
 
 /** Same as StyledOptions but shouldForwardProp must be a type guard */
 export interface FilteringStyledOptions<
-  Props,
-  ForwardedProps extends keyof Props = keyof Props
+  Props = Record<string, any>,
+  ForwardedProps extends keyof Props & string = keyof Props & string
 > {
-  shouldForwardProp?(propName: PropertyKey): propName is ForwardedProps
+  shouldForwardProp?: (propName: string) => propName is ForwardedProps
 }
 
-export interface StyledOptions<Props> {
-  shouldForwardProp?(propName: PropertyKey): boolean
+export interface StyledOptions<Props = Record<string, any>> {
+  shouldForwardProp?: (propName: string) => boolean
 }
 
 /**
@@ -146,7 +146,8 @@ export interface CreateStyledComponent<
 export interface CreateStyled {
   <
     C extends React.ComponentClass<React.ComponentProps<C>>,
-    ForwardedProps extends keyof React.ComponentProps<C> = keyof React.ComponentProps<C>
+    ForwardedProps extends keyof React.ComponentProps<C> &
+      string = keyof React.ComponentProps<C> & string
   >(
     component: C,
     options: FilteringStyledOptions<React.ComponentProps<C>, ForwardedProps>
@@ -175,7 +176,8 @@ export interface CreateStyled {
 
   <
     C extends React.ComponentType<React.ComponentProps<C>>,
-    ForwardedProps extends keyof React.ComponentProps<C> = keyof React.ComponentProps<C>
+    ForwardedProps extends keyof React.ComponentProps<C> &
+      string = keyof React.ComponentProps<C> & string
   >(
     component: C,
     options: FilteringStyledOptions<React.ComponentProps<C>, ForwardedProps>

diff --git a/packages/native/types/index.d.ts b/packages/native/types/index.d.ts
@@ -18,6 +18,7 @@ export {
   ArrayInterpolation,
   CreateStyledComponent,
   CSSInterpolation,
+  FilteringStyledOptions,
   FunctionInterpolation,
   Interpolation,
   InterpolationPrimitive,

diff --git a/packages/native/types/tests.tsx b/packages/native/types/tests.tsx
@@ -7,7 +7,12 @@ import {
   TextStyle,
   View
 } from 'react-native'
-import styled, { css, ReactNativeStyle } from '@emotion/native'
+import styled, {
+  css,
+  ReactNativeStyle,
+  StyledOptions,
+  FilteringStyledOptions
+} from '@emotion/native'
 
 declare module '@emotion/react' {
   // tslint:disable-next-line: strict-export-declare-modifiers
@@ -166,3 +171,22 @@ export const ImageFullWidthContained = styled.Image`
   ;<Container1 ref={containerRef1} />
   ;<Container2 ref={containerRef2} />
 }
+
+{
+  // Props forwarding through StyledOptions and FilteringStyledOptions
+
+  styled(View, { shouldForwardProp: (prop: string) => true })({})
+  // $ExpectError
+  styled(View, { shouldForwardProp: (prop: 'testID') => true })({})
+
+  styled(View, {
+    shouldForwardProp: (prop: string): prop is 'testID' => true
+  })({})
+  styled(View, {
+    // $ExpectError
+    shouldForwardProp: (prop: 'testID'): prop is 'testID' => true
+  })({})
+
+  // $ExpectError
+  styled(View, { shouldForwardProp: (prop: 'foo') => true })({})
+}
diff --git a/packages/styled/types/base.d.ts b/packages/styled/types/base.d.ts
@@ -3,7 +3,7 @@
 
 import * as React from 'react'
 import { ComponentSelector, Interpolation } from '@emotion/serialize'
-import { PropsOf, DistributiveOmit, Theme } from '@emotion/react'
+import { PropsOf, Theme } from '@emotion/react'
 
 export {
   ArrayInterpolation,
@@ -15,17 +15,17 @@ export { ComponentSelector, Interpolation }
 
 /** Same as StyledOptions but shouldForwardProp must be a type guard */
 export interface FilteringStyledOptions<
-  Props,
-  ForwardedProps extends keyof Props = keyof Props
+  Props = Record<string, any>,
+  ForwardedProps extends keyof Props & string = keyof Props & string
 > {
   label?: string
-  shouldForwardProp?(propName: PropertyKey): propName is ForwardedProps
+  shouldForwardProp?: (propName: string) => propName is ForwardedProps
   target?: string
 }
 
-export interface StyledOptions<Props> {
+export interface StyledOptions<Props = Record<string, any>> {
   label?: string
-  shouldForwardProp?(propName: PropertyKey): boolean
+  shouldForwardProp?: (propName: string) => boolean
   target?: string
 }
 
@@ -118,7 +118,8 @@ export interface CreateStyledComponent<
 export interface CreateStyled {
   <
     C extends React.ComponentClass<React.ComponentProps<C>>,
-    ForwardedProps extends keyof React.ComponentProps<C> = keyof React.ComponentProps<C>
+    ForwardedProps extends keyof React.ComponentProps<C> &
+      string = keyof React.ComponentProps<C> & string
   >(
     component: C,
     options: FilteringStyledOptions<React.ComponentProps<C>, ForwardedProps>
@@ -147,7 +148,8 @@ export interface CreateStyled {
 
   <
     C extends React.ComponentType<React.ComponentProps<C>>,
-    ForwardedProps extends keyof React.ComponentProps<C> = keyof React.ComponentProps<C>
+    ForwardedProps extends keyof React.ComponentProps<C> &
+      string = keyof React.ComponentProps<C> & string
   >(
     component: C,
     options: FilteringStyledOptions<React.ComponentProps<C>, ForwardedProps>
@@ -168,7 +170,8 @@ export interface CreateStyled {
 
   <
     Tag extends keyof JSX.IntrinsicElements,
-    ForwardedProps extends keyof JSX.IntrinsicElements[Tag] = keyof JSX.IntrinsicElements[Tag]
+    ForwardedProps extends keyof JSX.IntrinsicElements[Tag] &
+      string = keyof JSX.IntrinsicElements[Tag] & string
   >(
     tag: Tag,
     options: FilteringStyledOptions<JSX.IntrinsicElements[Tag], ForwardedProps>

diff --git a/packages/styled/types/index.d.ts b/packages/styled/types/index.d.ts
@@ -12,6 +12,7 @@ export {
   Interpolation,
   StyledComponent,
   StyledOptions,
+  FilteringStyledOptions,
   CreateStyledComponent
 } from './base'
 

diff --git a/packages/styled/types/tests.tsx b/packages/styled/types/tests.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react'
-import styled from '@emotion/styled'
+import styled, { StyledOptions, FilteringStyledOptions } from '@emotion/styled'
 
 // This file uses the same Theme declaration from tests-base.tsx
 
@@ -217,3 +217,87 @@ const Input5 = styled.input`
   // $ExpectError
   ;<StyledCompWithoutAs as={Section} />
 }
+
+{
+  // Props forwarding through StyledOptions and FilteringStyledOptions
+
+  const fc: React.FC<{ foo: string }> = () => null
+
+  // we can't accept a "known" prop here because we need to include `AdditionalProps` and those aren't available yet
+  // `Props` represent the actual props of a component while `AdditionalProps` represent props used only for styling purposes
+  // $ExpectError
+  styled(fc, { shouldForwardProp: (prop: 'foo') => true })({})
+
+  styled(fc, { shouldForwardProp: (prop: string) => true })({})
+
+  // $ExpectError
+  styled(fc, { shouldForwardProp: (prop: 'bar') => true })({})
+  // $ExpectError
+  styled(fc, { shouldForwardProp: (prop: 'foo') => true })({})
+
+  // $ExpectError
+  const shouldForwardProp1: StyledOptions['shouldForwardProp'] = (
+    prop: 'unknown'
+  ) => true
+  styled(fc, { shouldForwardProp: shouldForwardProp1 })({})
+
+  // $ExpectError
+  styled(fc, { shouldForwardProp: (prop: 'unknown') => true })({})
+
+  // $ExpectError
+  const shouldForwardProp2: StyledOptions<{
+    foo: string
+  }>['shouldForwardProp'] = (prop: 'unknown') => true
+
+  styled(fc, { shouldForwardProp: (prop: string): prop is 'foo' => true })({})
+  // $ExpectError
+  styled(fc, { shouldForwardProp: (prop: 'foo'): prop is 'foo' => true })({})
+
+  const shouldForwardProp3: FilteringStyledOptions['shouldForwardProp'] = (
+    prop: string
+  ): prop is 'foo' => true
+
+  // $ExpectError
+  const shouldForwardProp4: FilteringStyledOptions['shouldForwardProp'] = (
+    prop: 'foo'
+  ): prop is 'foo' => true
+
+  const shouldForwardProp5: FilteringStyledOptions<{
+    foo: string
+  }>['shouldForwardProp'] = (prop: string): prop is 'foo' => true
+  // $ExpectError
+  const shouldForwardProp6: FilteringStyledOptions<{
+    foo: string
+  }>['shouldForwardProp'] = (prop: 'foo'): prop is 'foo' => true
+
+  // $ExpectError
+  const shouldForwardProp7: FilteringStyledOptions<{
+    foo: string
+  }>['shouldForwardProp'] = (prop: 'unknown'): prop is 'unknown' => true
+
+  const shouldForwardProp8: FilteringStyledOptions<
+    { foo: string; bar: string },
+    'foo'
+  >['shouldForwardProp'] = (prop: string): prop is 'foo' => true
+
+  // $ExpectError
+  const shouldForwardProp9: FilteringStyledOptions<
+    { foo: string; bar: string },
+    'foo'
+  >['shouldForwardProp'] = (prop: 'foo' | 'bar'): prop is 'bar' => true
+
+  styled('div', {
+    shouldForwardProp: (prop: string) => true
+  })({})
+
+  // $ExpectError
+  styled('div', { shouldForwardProp: (prop: 'color') => true })({})
+
+  styled('div', {
+    // $ExpectError
+    shouldForwardProp: (prop: 'color'): prop is 'color' => true
+  })({})
+
+  // $ExpectError
+  styled('div', { shouldForwardProp: (prop: 'foo') => true })({})
+}