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.

Sign up for GitHub

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

Relaxed types for shouldForwardProp #1643

Merged
merged 4 commits into from
Nov 19, 2019
Merged

Relaxed types for shouldForwardProp #1643

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
657b901
Relaxed types for shouldForwardProp
Nov 18, 2019
3f30b4f
Update .changeset/dull-carrots-enjoy.md
JakeGinnivan Nov 19, 2019
c264c4b
Update .changeset/dull-carrots-enjoy.md
JakeGinnivan Nov 19, 2019
e229804
Update .changeset/cool-candles-lie.md
JakeGinnivan Nov 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cool-candles-lie.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@emotion/is-prop-valid': minor
---

[TypeScript] Allow isPropValid to take any PropertyKey as an argument (instead of just string)
JakeGinnivan marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 7 additions & 0 deletions .changeset/dull-carrots-enjoy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@emotion/styled': patch
---

Relaxed types for shouldForwardProp
JakeGinnivan marked this conversation as resolved.
Show resolved Hide resolved

This function needs to be able to filter a props for a generic argument of the resulting function.
JakeGinnivan marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion packages/is-prop-valid/types/index.d.ts
View file
Open in desktop
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: string): boolean
declare function isPropValid(string: PropertyKey): boolean
export default isPropValid
2 changes: 0 additions & 2 deletions packages/is-prop-valid/types/tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ isPropValid('ref')
// $ExpectError
isPropValid()
// $ExpectError
isPropValid(5)
// $ExpectError
isPropValid({})
// $ExpectError
isPropValid('ref', 'def')
4 changes: 2 additions & 2 deletions packages/styled/types/base.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ export interface FilteringStyledOptions<
ForwardedProps extends keyof Props = keyof Props
> {
label?: string
shouldForwardProp?(propName: keyof Props): propName is ForwardedProps
shouldForwardProp?(propName: PropertyKey): propName is ForwardedProps
Copy link
Member

Choose a reason for hiding this comment

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

you are not extracting anything based on this, couldnt we just use a simple string here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A type predicate's type must be assignable to its parameter's type.
  Type 'ForwardedProps' is not assignable to type 'string'.
    Type 'keyof Props' is not assignable to type 'string'.
      Type 'string | number | symbol' is not assignable to type 'string'.
        Type 'number' is not assignable to type 'string'.ts(2677)

is the error I get

Copy link
Member

Choose a reason for hiding this comment

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

Ah, so it has to match to ForwardedProps extends keyof Props, interesting. Gonna give it a thought later, but probably we'll keep PropertyKey if that's the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can replace keyof Props with Extract<keyof Props, string> to only extract strings which works. It just makes everything look more complex.

target?: string
}

export interface StyledOptions<Props> {
label?: string
shouldForwardProp?(propName: keyof Props): boolean
shouldForwardProp?(propName: PropertyKey): boolean
target?: string
}

Expand Down
13 changes: 13 additions & 0 deletions packages/styled/types/tests-base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react'
import styled from '@emotion/styled/base'
import isPropValid from '@emotion/is-prop-valid'

// tslint:disable-next-line: interface-over-type-literal
type ReactClassProps0 = {
Expand Down Expand Up @@ -98,6 +99,18 @@ const Canvas1 = styled('canvas', {
;<Canvas0 id="id-should-be-filtered" />
;<Canvas1 />

const styledWithForwardedExtraProp = styled('div', {
shouldForwardProp: prop => prop !== 'priority' && isPropValid(prop)
})

type Priority = 'info' | 'warning' | 'error'

const Alert = styledWithForwardedExtraProp<{
priority?: Priority
}>(({ priority, theme }) => ({
backgroundColor: theme.colors[priority || 'info']
}))

interface PrimaryProps {
readonly primary: string
}
Expand Down