Skip to content

[react] allow weaker prop declarations for nullish properties #31280

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

Merged
johnnyreilly merged 2 commits into from
Dec 12, 2018
Merged

[react] allow weaker prop declarations for nullish properties #31280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d869f78
feat(react): allow weaker prop declarations for nullish properties
ferdaber Dec 11, 2018
c049ed0
fix tests
ferdaber Dec 11, 2018
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
16 changes: 10 additions & 6 deletions types/prop-types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
// TypeScript Version: 2.8

export type ReactComponentLike =
| string
| ((props: any, context?: any) => any)
| (new (props: any, context?: any) => any);

export interface ReactElementLike {
type: string | ((...args: any[]) => ReactElementLike);
type: ReactComponentLike;
Copy link
Contributor Author

@ferdaber ferdaber Dec 11, 2018

Choose a reason for hiding this comment

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

The original type property for this was not exactly the same as the one in the React type definitions, causing issues with the PropTypes.element validator, which this fixes.

props: any;
key: string | number | null;
children?: ReactNodeLike;
}

export interface ReactNodeArray extends Array<ReactNodeLike> {}
Expand All @@ -28,9 +32,9 @@ export const nominalTypeHack: unique symbol;

export type IsOptional<T> = undefined | null extends T ? true : undefined extends T ? true : null extends T ? true : false;

export type RequiredKeys<V> = { [K in keyof V]: V[K] extends Validator<infer T> ? IsOptional<T> extends true ? never : K : never }[keyof V];
export type RequiredKeys<V> = { [K in keyof V]-?: Exclude<V[K], undefined> extends Validator<infer T> ? IsOptional<T> extends true ? never : K : never }[keyof V];
export type OptionalKeys<V> = Exclude<keyof V, RequiredKeys<V>>;
export type InferPropsInner<V> = { [K in keyof V]: InferType<V[K]>; };
export type InferPropsInner<V> = { [K in keyof V]-?: InferType<V[K]>; };

export interface Validator<T> {
(props: object, propName: string, componentName: string, location: string, propFullName: string): Error | null;
Expand All @@ -45,8 +49,8 @@ export type ValidationMap<T> = { [K in keyof T]?: Validator<T[K]> };

export type InferType<V> = V extends Validator<infer T> ? T : any;
export type InferProps<V> =
& (RequiredKeys<V> extends undefined ? {} : InferPropsInner<Pick<V, RequiredKeys<V>>>)
& (OptionalKeys<V> extends undefined ? {} : Partial<InferPropsInner<Pick<V, OptionalKeys<V>>>>);
& InferPropsInner<Pick<V, RequiredKeys<V>>>
& Partial<InferPropsInner<Pick<V, OptionalKeys<V>>>>;

export const any: Requireable<any>;
export const array: Requireable<any[]>;
Expand Down
31 changes: 11 additions & 20 deletions types/prop-types/prop-types-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,35 +129,26 @@ type ExtractedPartialProps = PropTypes.InferProps<typeof partialPropTypes>;
type ExtractedPropsWithoutAnnotation = PropTypes.InferProps<typeof propTypesWithoutAnnotation>;
type ExtractedPropsFromOuterPropsWithoutAnnotation = PropTypes.InferProps<typeof outerPropTypesWithoutAnnotation>['props'];

// $ExpectType: true
// $ExpectType true
Copy link
Contributor Author

@ferdaber ferdaber Dec 11, 2018

Choose a reason for hiding this comment

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

Fixed a longstanding issue with these DTSLint assertions

type ExtractPropsMatch = ExtractedProps extends ExtractedPropsWithoutAnnotation ? true : false;
// $ExpectType: true
// $ExpectType true
type ExtractPropsMatch2 = ExtractedPropsWithoutAnnotation extends ExtractedProps ? true : false;
// $ExpectType: true
// $ExpectType true
type ExtractPropsMatch3 = ExtractedProps extends Props ? true : false;
// $ExpectType: true
// $ExpectType true
type ExtractPropsMatch4 = Props extends ExtractedPropsWithoutAnnotation ? true : false;
// $ExpectType: true
// $ExpectType true
type ExtractFromOuterPropsMatch = ExtractedPropsFromOuterProps extends ExtractedPropsFromOuterPropsWithoutAnnotation ? true : false;
// $ExpectType: true
// $ExpectType true
type ExtractFromOuterPropsMatch2 = ExtractedPropsFromOuterPropsWithoutAnnotation extends ExtractedPropsFromOuterProps ? true : false;
// $ExpectType: true
// $ExpectType true
type ExtractFromOuterPropsMatch3 = ExtractedPropsFromOuterProps extends Props ? true : false;
// $ExpectType: true
// $ExpectType true
type ExtractFromOuterPropsMatch4 = Props extends ExtractedPropsFromOuterPropsWithoutAnnotation ? true : false;

// $ExpectType: false
// $ExpectType false
type ExtractPropsMismatch = ExtractedPartialProps extends Props ? true : false;

// $ExpectType: {}
type UnmatchedPropKeys = Pick<ExtractedPropsWithoutAnnotation, Extract<{
[K in keyof ExtractedPropsWithoutAnnotation]: ExtractedPropsWithoutAnnotation[K] extends ExtractedProps[K] ? never : K
}[keyof ExtractedPropsWithoutAnnotation], keyof ExtractedPropsWithoutAnnotation>>;
// $ExpectType: {}
type UnmatchedPropKeys2 = Pick<ExtractedProps, Extract<{
[K in keyof ExtractedProps]: ExtractedProps[K] extends ExtractedPropsWithoutAnnotation[K] ? never : K
}[keyof ExtractedProps], keyof ExtractedProps>>;

PropTypes.checkPropTypes({ xs: PropTypes.array }, { xs: [] }, 'location', 'componentName');

// This would be the type that JSX sees
Expand Down Expand Up @@ -189,15 +180,15 @@ const componentDefaultProps = {
type DefaultizedProps = Defaultize<PropTypes.InferProps<typeof componentPropTypes>, typeof componentDefaultProps>;
type UndefaultizedProps = Undefaultize<PropTypes.InferProps<typeof componentPropTypes>, typeof componentDefaultProps>;

// $ExpectType: true
// $ExpectType true
type DefaultizedPropsTest = {
fi?: (...args: any[]) => any;
foo?: string | null;
bar: number;
baz?: boolean | null;
bat?: ReactNode;
} extends DefaultizedProps ? true : false;
// $ExpectType: true
// $ExpectType true
type UndefaultizedPropsTest = {
fi: (...args: any[]) => any;
foo?: string | null;
Expand Down
16 changes: 12 additions & 4 deletions types/react/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ declare namespace React {
}

interface ProviderExoticComponent<P> extends ExoticComponent<P> {
propTypes?: ValidationMap<P>;
propTypes?: WeakValidationMap<P>;
}

type ContextType<C extends Context<any>> = C extends Context<infer T> ? T : never;
Expand Down Expand Up @@ -461,23 +461,23 @@ declare namespace React {

interface FunctionComponent<P = {}> {
(props: P & { children?: ReactNode }, context?: any): ReactElement<any> | null;
propTypes?: ValidationMap<P>;
propTypes?: WeakValidationMap<P>;
contextTypes?: ValidationMap<any>;
defaultProps?: Partial<P>;
displayName?: string;
}

interface RefForwardingComponent<T, P = {}> {
(props: P & { children?: ReactNode }, ref: Ref<T> | null): ReactElement<any> | null;
propTypes?: ValidationMap<P>;
propTypes?: WeakValidationMap<P>;
contextTypes?: ValidationMap<any>;
defaultProps?: Partial<P>;
displayName?: string;
}

interface ComponentClass<P = {}, S = ComponentState> extends StaticLifecycle<P, S> {
new (props: P, context?: any): Component<P, S>;
propTypes?: ValidationMap<P>;
propTypes?: WeakValidationMap<P>;
contextType?: Context<any>;
contextTypes?: ValidationMap<any>;
childContextTypes?: ValidationMap<any>;
Expand Down Expand Up @@ -2558,6 +2558,14 @@ declare namespace React {

type ValidationMap<T> = PropTypes.ValidationMap<T>;

type WeakValidationMap<T> = {
Copy link
Contributor Author

@ferdaber ferdaber Dec 11, 2018

Choose a reason for hiding this comment

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

The weak validation map will transform any prop declaration that can be null or undefined and widen it to be both: null | undefined. This allows props authors to declare something like this:

type Props = { foo?: string }

and have it pass checks for HOCs that require it to be:

type Props = { foo?: string | null }

Most developers do not realize that propTypes allows for both so this should meet a compromise of DX and runtime accuracy.

Copy link

@chibicode chibicode Dec 13, 2018

Choose a reason for hiding this comment

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

Hi @ferdaber, after upgrading @types/react to 16.7.14 from 16.7.13, my project is getting

TS2605: JSX element type 'ReactElement<any> | null' is not a constructor function for JSX elements.
  Type 'ReactElement<any>' is not assignable to type 'Element'.

for almost every component. I've tracked down this bug into these changes. I'm wondering if anyone else is getting this error...I'll try to create a reproducible repo soon.

Copy link

@chibicode chibicode Dec 13, 2018

Choose a reason for hiding this comment

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

Actually, the problem was a different issue (cc @cuyl) - I've written the fix here:

#17161 (comment)

Copy link

@cuyl cuyl Dec 13, 2018

Choose a reason for hiding this comment

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

thx

[K in keyof T]?: null extends T[K]
? Validator<T[K] | null | undefined>
: undefined extends T[K]
? Validator<T[K] | null | undefined>
: Validator<T[K]>
};

interface ReactPropTypes {
any: typeof PropTypes.any;
array: typeof PropTypes.array;
Expand Down
25 changes: 25 additions & 0 deletions types/react/test/tsx.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,3 +345,28 @@ declare global {
}

const CustomElement2: React.ReactType = 'my-declared-element';

interface TestPropTypesProps {
foo: string;
}
interface TestPropTypesProps1 {
foo?: string;
}
interface TestPropTypesProps2 {
foo: string | null;
}
interface TestPropTypesProps3 {
foo?: string | null;
}
const testPropTypes = {
foo: PropTypes.string
};
type DeclaredPropTypes<P> = Required<Exclude<React.ComponentType<P>['propTypes'], undefined>>;
// $ExpectType false
type propTypesTest = typeof testPropTypes extends DeclaredPropTypes<TestPropTypesProps> ? true : false;
// $ExpectType true
type propTypesTest1 = typeof testPropTypes extends DeclaredPropTypes<TestPropTypesProps1> ? true : false;
// $ExpectType true
type propTypesTest2 = typeof testPropTypes extends DeclaredPropTypes<TestPropTypesProps2> ? true : false;
// $ExpectType true
type propTypesTest3 = typeof testPropTypes extends DeclaredPropTypes<TestPropTypesProps3> ? true : false;