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

chore(styled): let StyledOptions generic argument be optional #2333

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
ccc955d
chore(style): let StyledOptions generic argument be optional
antongolub Apr 6, 2021
29ca437
chore(native): let StyledOptions generic argument be optional
antongolub Apr 6, 2021
107e017
test(types): add tests for StyledOptions iface
antongolub Apr 7, 2021
d163c00
feat(types): add keys assertion for StyledOptions generic
antongolub Apr 7, 2021
4c5b17e
fix(styled): add StyledOptions type to CreateStyledComponent call sig…
antongolub Apr 9, 2021
659f6ec
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Apr 14, 2021
165a422
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Apr 16, 2021
696213b
test(styled): check type StyledOpts for composite component
antongolub Apr 18, 2021
fae2dab
test(styled): revert erroneous changes
antongolub Apr 19, 2021
2238e9e
test(styled): clarify StyledOptsBroken test case
antongolub Apr 19, 2021
a1775f8
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub May 9, 2021
e36ff17
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub May 29, 2021
009b7b0
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Jun 7, 2021
b2e0b7a
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Aug 10, 2021
b9ecb17
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Oct 23, 2021
1cc73a2
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Nov 8, 2021
34be1e2
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Nov 15, 2021
04e367b
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Nov 23, 2021
310dba8
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Dec 16, 2021
b8d3039
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Dec 25, 2021
5286a60
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Jan 7, 2022
d889b0e
test: refactor `styledOptions` generics tests
antongolub Jan 12, 2022
0fa6ee4
test: add more tests for StyledOptions
antongolub Jan 12, 2022
83fae39
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Jan 12, 2022
ebfbd9a
test: linting
antongolub Jan 12, 2022
9e78345
test: tweak ups
antongolub Jan 12, 2022
894aa3b
feat: expose FilteringStyledOptions
antongolub Jan 14, 2022
43f318b
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Jan 31, 2022
27b4f2d
fix: expost `FilteringStyledOptions` from native, tweak up some tests…
antongolub Feb 25, 2022
c2c2c27
Merge branch 'main' into let-styledopts-arg-be-optional
antongolub Feb 25, 2022
d3092bb
chore: use consistent default props type for both StyledOptions and F…
antongolub Mar 8, 2022
4104c2a
chore: snap #3
antongolub Mar 9, 2022
5595c8d
styled.shouldForwardProp: require prop to be a string
srmagura May 21, 2022
09df01d
Add PropertyKey->string changesets
srmagura May 21, 2022
5f3f5e5
Remove redundant `compilerOptions`
Andarist May 21, 2022
da3a461
make `shouldForwardProp` always contravariant
Andarist May 22, 2022
a4c2a75
Merge branch 'main' into let-styledopts-arg-be-optional
Andarist May 23, 2022
d133a83
Merge branch 'main' into let-styledopts-arg-be-optional
Andarist May 30, 2022
f360473
Type fixes
Andarist May 30, 2022
ac3f8c7
Tweak `FilteringStyledOptions` and `StyledOptions` interfaces
Andarist May 30, 2022
fce092c
Change `Props` constraint
Andarist May 30, 2022
59e8aec
Tweak type-level tests around `shouldForwardProp`
Andarist May 30, 2022
65f4444
Merge remote-tracking branch 'srmagura/styled-prop-string' into let-s…
Andarist Jun 5, 2022
682313b
add changesets
Andarist Jun 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/good-cars-roll.md
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.
8 changes: 8 additions & 0 deletions .changeset/purple-ladybugs-think.md
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.
8 changes: 8 additions & 0 deletions .changeset/tall-flies-smoke.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions .changeset/thirty-boats-hang.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@emotion/native': patch
'@emotion/styled': patch
---

`FilteringStyledOptions` and `StyledOptions` types no longer require a type argument for the `Props` generic.
2 changes: 1 addition & 1 deletion packages/is-prop-valid/types/index.d.ts
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
16 changes: 9 additions & 7 deletions packages/native/types/base.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions packages/native/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export {
ArrayInterpolation,
CreateStyledComponent,
CSSInterpolation,
FilteringStyledOptions,
FunctionInterpolation,
Interpolation,
InterpolationPrimitive,
Expand Down
26 changes: 25 additions & 1 deletion packages/native/types/tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 })({})
}
21 changes: 12 additions & 9 deletions packages/styled/types/base.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}

Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand All @@ -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>
Expand Down
1 change: 1 addition & 0 deletions packages/styled/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export {
Interpolation,
StyledComponent,
StyledOptions,
FilteringStyledOptions,
CreateStyledComponent
} from './base'

Expand Down
86 changes: 85 additions & 1 deletion packages/styled/types/tests.tsx
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

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is AdditionalProps referring to here? Maybe it is supposed to say ForwardedProps?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AdditionalProps are declared here:

<AdditionalProps extends {} = {}>(

The problem is this:

styled<GenericsThatDontHaveAccessToAdditionalProps>('div', {
  shouldForwardProp: (prop: ThisHasToBeAbleToAcceptAdditionalProps) => true
})<AdditionalProps>({
  color: (props) => props.myAwesomeAdditionalColor
})

As we might see here - AdditionalProps are provided to what is being returned from styled but yet shouldForwardProp has only access to what is given to styled.

// $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 })({})
}