-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
chore(styled): let StyledOptions generic argument be optional #2333
Changes from 43 commits
ccc955d
29ca437
107e017
d163c00
4c5b17e
659f6ec
165a422
696213b
fae2dab
2238e9e
a1775f8
e36ff17
009b7b0
b2e0b7a
b9ecb17
1cc73a2
34be1e2
04e367b
310dba8
b8d3039
5286a60
d889b0e
0fa6ee4
83fae39
ebfbd9a
9e78345
894aa3b
43f318b
27b4f2d
c2c2c27
d3092bb
4104c2a
5595c8d
09df01d
5f3f5e5
da3a461
a4c2a75
d133a83
f360473
ac3f8c7
fce092c
59e8aec
65f4444
682313b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -13,19 +13,22 @@ export { | |
|
||
export { ComponentSelector, Interpolation } | ||
|
||
/** Same as StyledOptions but shouldForwardProp must be a type guard */ | ||
/** | ||
* Same as StyledOptions but shouldForwardProp must be a type guard (https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this link to the TS handbook is necessary, and it will eventually become outdated if they release a new version of the handbook. |
||
* Practical sense: you can define and reuse atomic `shouldForwardProp` filters that are strictly bound with some `ForwardedProps` type. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what this line is saying... maybe rewording it or including an example would be helpful. |
||
*/ | ||
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 +121,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 +151,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 +173,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> | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -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 | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
emotion/packages/styled/types/base.d.ts Line 72 in 65f4444
The problem is this: styled<GenericsThatDontHaveAccessToAdditionalProps>('div', {
shouldForwardProp: (prop: ThisHasToBeAbleToAcceptAdditionalProps) => true
})<AdditionalProps>({
color: (props) => props.myAwesomeAdditionalColor
}) As we might see here - |
||||
// $ExpectError | ||||
styled(fc, { shouldForwardProp: (prop: 'foo') => true })({}) | ||||
Andarist marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
|
||||
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 })({}) | ||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments on the version of this in
packages/styled/types/base.d.ts
.