Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Clarify children for fragments and similar React APIs 
Summary:
Before this change, a `Fragment`'s children were a non-optional maybe type,
which allows `void` but does not allow omitting the children property entirely.
Instead, the children should be optional. Since `React$Node` already includes
`null`, and `void` is not actually a valid child type, we can entirely replace
the maybe type with optionality.

The fragment expression `<></>` should be an error with a non-optional children
type, since at runtime the element will not be passed a children prop at all.
This worked by accident, since `Fragment` is defined as a function type and
function type's have an `any`-typed statics object, and thus `any` typed default
props.

In a stacked diff, I change function types to use a stricter type for statics
(an empty object with Function.prototype as the __proto__), which exposed this
bug.

Note that this bug also allowed creating a fragment with any children type,
e.g., `<>{() => {}}</>`, even though functions are not members of `React.Node`.

Reviewed By: bvaughn

Differential Revision: D14447780

fbshipit-source-id: db0cf4700cd97762b5c92b8a24ff5e0221c7ff96
  • Loading branch information
@samwgoldman @facebook-github-bot
samwgoldman authored and facebook-github-bot committed Apr 22, 2019
1 parent b9e26fd commit 6dd62e0
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 22 deletions.
12 changes: 6 additions & 6 deletions lib/react.js
View file
Expand Up @@ -200,7 +200,7 @@ declare type React$Ref<ElementType: React$ElementType> =
* createContext() with a default value.
*/
declare type React$Context<T> = {
Provider: React$ComponentType<{ value: T, children?: ?React$Node }>,
Provider: React$ComponentType<{ value: T, children?: React$Node }>,
Consumer: React$ComponentType<{ children: (value: T) => ?React$Node }>,

// Optional, user-specified value for custom display label in React DevTools.
Expand Down Expand Up @@ -252,18 +252,18 @@ declare module react {
> = React$AbstractComponent<Config, Instance>;
declare export type ElementType = React$ElementType;
declare export type Element<+C> = React$Element<C>;
declare export var Fragment: ({children: ?React$Node}) => React$Node;
declare export var Fragment: ({children?: React$Node}) => React$Node;
declare export type Key = React$Key;
declare export type Ref<C> = React$Ref<C>;
declare export type Node = React$Node;
declare export type Context<T> = React$Context<T>;
declare export type Portal = React$Portal;
declare export var ConcurrentMode: ({children: ?React$Node}) => React$Node; // 16.7+
declare export var StrictMode: ({children: ?React$Node}) => React$Node;
declare export var ConcurrentMode: ({children?: React$Node}) => React$Node; // 16.7+
declare export var StrictMode: ({children?: React$Node}) => React$Node;

declare export var Suspense: React$ComponentType<{
children?: ?React$Node,
fallback?: React$Node
children?: React$Node,
fallback?: React$Node,
}>; // 16.6+

declare export type ElementProps<C> = React$ElementProps<C>;
Expand Down
25 changes: 9 additions & 16 deletions tests/type-at-pos_react/type-at-pos_react.exp
View file
Expand Up @@ -44,7 +44,7 @@ create_calss.js:31:7 = {
"end":20
}
react_component.js:3:9 = {
"type":"{|+AbstractComponent: type AbstractComponent<-Config, +Instance = mixed> = React$AbstractComponent<Config, Instance>, +Children: {+count: (children: ChildrenArray<any>) => number, +forEach: <T>(children: ChildrenArray<T>, fn: (child: T, index: number) => mixed, thisArg?: mixed) => void, +map: <T, U>(children: ChildrenArray<T>, fn: (child: $NonMaybeType<T>, index: number) => U, thisArg?: mixed) => Array<$NonMaybeType<U>>, +only: <T>(children: ChildrenArray<T>) => $NonMaybeType<T>, +toArray: <T>(children: ChildrenArray<T>) => Array<$NonMaybeType<T>>}, +ChildrenArray: type ChildrenArray<+T> = $ReadOnlyArray<ChildrenArray<T>> | T, +Component: class React$Component<Props, State = void>, +ComponentType: type ComponentType<-P> = React$ComponentType<P>, +ConcurrentMode: ({children: ?React$Node}) => React$Node, +Config: type Config<Props, DefaultProps> = React$Config<Props, DefaultProps>, +Context: type Context<T> = React$Context<T>, +DOM: any, +Element: type Element<+C> = React$Element<C>, +ElementConfig: type ElementConfig<C> = React$ElementConfig<C>, +ElementProps: type ElementProps<C> = React$ElementProps<C>, +ElementRef: type ElementRef<C> = React$ElementRef<C>, +ElementType: type ElementType = React$ElementType, +Fragment: ({children: ?React$Node}) => React$Node, +Key: type Key = React$Key, +Node: type Node = React$Node, +Portal: type Portal = React$Portal, +PropTypes: ReactPropTypes, +PureComponent: class React$PureComponent<Props, State = void>, +Ref: type Ref<C> = React$Ref<C>, +StatelessFunctionalComponent: type StatelessFunctionalComponent<P> = React$StatelessFunctionalComponent<P>, +StrictMode: ({children: ?React$Node}) => React$Node, +Suspense: React$ComponentType<{children?: ?React$Node, fallback?: React$Node}>, +checkPropTypes: <V>(propTypes: any, values: V, location: string, componentName: string, getStack: ?(() => ?string)) => void, +cloneElement: React$CloneElement, +createClass: React$CreateClass, +createContext: <T>(defaultValue: T, calculateChangedBits: ?((a: T, b: T) => number)) => React$Context<T>, +createElement: React$CreateElement, +createFactory: <ElementType: React$ElementType>(type: ElementType) => React$ElementFactory<ElementType>, +createRef: <T>() => {|current: (null | T)|}, +default: {|+Children: {+count: (children: ChildrenArray<any>) => number, +forEach: <T>(children: ChildrenArray<T>, fn: (child: T, index: number) => mixed, thisArg?: mixed) => void, +map: <T, U>(children: ChildrenArray<T>, fn: (child: $NonMaybeType<T>, index: number) => U, thisArg?: mixed) => Array<$NonMaybeType<U>>, +only: <T>(children: ChildrenArray<T>) => $NonMaybeType<T>, +toArray: <T>(children: ChildrenArray<T>) => Array<$NonMaybeType<T>>}, +Component: class React$Component<Props, State = void>, +ConcurrentMode: ({children: ?React$Node}) => React$Node, +DOM: any, +Fragment: ({children: ?React$Node}) => React$Node, +PropTypes: ReactPropTypes, +PureComponent: class React$PureComponent<Props, State = void>, +StrictMode: ({children: ?React$Node}) => React$Node, +Suspense: React$ComponentType<{children?: ?React$Node, fallback?: React$Node}>, +checkPropTypes: <V>(propTypes: any, values: V, location: string, componentName: string, getStack: ?(() => ?string)) => void, +cloneElement: React$CloneElement, +createClass: React$CreateClass, +createContext: <T>(defaultValue: T, calculateChangedBits: ?((a: T, b: T) => number)) => React$Context<T>, +createElement: React$CreateElement, +createFactory: <ElementType: React$ElementType>(type: ElementType) => React$ElementFactory<ElementType>, +createRef: <T>() => {|current: (null | T)|}, +forwardRef: <Config, Instance>(render: (props: Config, ref: ({current: (null | Instance)} | (((null | Instance)) => mixed))) => React$Node) => React$AbstractComponent<Config, Instance>, +isValidElement: (element: any) => boolean, +lazy: <P>(component: () => Promise<{default: React$ComponentType<P>}>) => React$ComponentType<P>, +memo: <P>(component: React$ComponentType<P>, equal?: (P, P) => boolean) => React$ComponentType<P>, +useCallback: <T: (...args: $ReadOnlyArray<empty>) => mixed>(callback: T, inputs: ?$ReadOnlyArray<mixed>) => T, +useContext: <T>(context: React$Context<T>, observedBits: (void | number | boolean)) => T, +useEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useImperativeHandle: <T>(ref: ?({current: (T | null)} | ((inst: (T | null)) => mixed)), create: () => T, inputs: ?$ReadOnlyArray<mixed>) => void, +useLayoutEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useMemo: <T>(create: () => T, inputs: ?$ReadOnlyArray<mixed>) => T, +useReducer: ((<S, A>(reducer: (S, A) => S, initialState: S) => [S, Dispatch<A>]) & (<S, A>(reducer: (S, A) => S, initialState: S, init: void) => [S, Dispatch<A>]) & (<S, A, I>(reducer: (S, A) => S, initialArg: I, init: (I) => S) => [S, Dispatch<A>])), +useRef: <T>(initialValue: T) => {|current: T|}, +useState: <S>(initialState: ((() => S) | S)) => [S, ((((S) => S) | S)) => void], +version: string|}, +forwardRef: <Config, Instance>(render: (props: Config, ref: ({current: (null | Instance)} | (((null | Instance)) => mixed))) => React$Node) => React$AbstractComponent<Config, Instance>, +isValidElement: (element: any) => boolean, +lazy: <P>(component: () => Promise<{default: React$ComponentType<P>}>) => React$ComponentType<P>, +memo: <P>(component: React$ComponentType<P>, equal?: (P, P) => boolean) => React$ComponentType<P>, +useCallback: <T: (...args: $ReadOnlyArray<empty>) => mixed>(callback: T, inputs: ?$ReadOnlyArray<mixed>) => T, +useContext: <T>(context: React$Context<T>, observedBits: (void | number | boolean)) => T, +useDebugValue: (value: any) => void, +useEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useImperativeHandle: <T>(ref: ?({current: (T | null)} | ((inst: (T | null)) => mixed)), create: () => T, inputs: ?$ReadOnlyArray<mixed>) => void, +useLayoutEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useMemo: <T>(create: () => T, inputs: ?$ReadOnlyArray<mixed>) => T, +useReducer: ((<S, A>(reducer: (S, A) => S, initialState: S) => [S, Dispatch<A>]) & (<S, A>(reducer: (S, A) => S, initialState: S, init: void) => [S, Dispatch<A>]) & (<S, A, I>(reducer: (S, A) => S, initialArg: I, init: (I) => S) => [S, Dispatch<A>])), +useRef: <T>(initialValue: T) => {|current: T|}, +useState: <S>(initialState: ((() => S) | S)) => [S, ((((S) => S) | S)) => void], +version: string|}",
"type":"{|+AbstractComponent: type AbstractComponent<-Config, +Instance = mixed> = React$AbstractComponent<Config, Instance>, +Children: {+count: (children: ChildrenArray<any>) => number, +forEach: <T>(children: ChildrenArray<T>, fn: (child: T, index: number) => mixed, thisArg?: mixed) => void, +map: <T, U>(children: ChildrenArray<T>, fn: (child: $NonMaybeType<T>, index: number) => U, thisArg?: mixed) => Array<$NonMaybeType<U>>, +only: <T>(children: ChildrenArray<T>) => $NonMaybeType<T>, +toArray: <T>(children: ChildrenArray<T>) => Array<$NonMaybeType<T>>}, +ChildrenArray: type ChildrenArray<+T> = $ReadOnlyArray<ChildrenArray<T>> | T, +Component: class React$Component<Props, State = void>, +ComponentType: type ComponentType<-P> = React$ComponentType<P>, +ConcurrentMode: ({children?: React$Node}) => React$Node, +Config: type Config<Props, DefaultProps> = React$Config<Props, DefaultProps>, +Context: type Context<T> = React$Context<T>, +DOM: any, +Element: type Element<+C> = React$Element<C>, +ElementConfig: type ElementConfig<C> = React$ElementConfig<C>, +ElementProps: type ElementProps<C> = React$ElementProps<C>, +ElementRef: type ElementRef<C> = React$ElementRef<C>, +ElementType: type ElementType = React$ElementType, +Fragment: ({children?: React$Node}) => React$Node, +Key: type Key = React$Key, +Node: type Node = React$Node, +Portal: type Portal = React$Portal, +PropTypes: ReactPropTypes, +PureComponent: class React$PureComponent<Props, State = void>, +Ref: type Ref<C> = React$Ref<C>, +StatelessFunctionalComponent: type StatelessFunctionalComponent<P> = React$StatelessFunctionalComponent<P>, +StrictMode: ({children?: React$Node}) => React$Node, +Suspense: React$ComponentType<{children?: React$Node, fallback?: React$Node}>, +checkPropTypes: <V>(propTypes: any, values: V, location: string, componentName: string, getStack: ?(() => ?string)) => void, +cloneElement: React$CloneElement, +createClass: React$CreateClass, +createContext: <T>(defaultValue: T, calculateChangedBits: ?((a: T, b: T) => number)) => React$Context<T>, +createElement: React$CreateElement, +createFactory: <ElementType: React$ElementType>(type: ElementType) => React$ElementFactory<ElementType>, +createRef: <T>() => {|current: (null | T)|}, +default: {|+Children: {+count: (children: ChildrenArray<any>) => number, +forEach: <T>(children: ChildrenArray<T>, fn: (child: T, index: number) => mixed, thisArg?: mixed) => void, +map: <T, U>(children: ChildrenArray<T>, fn: (child: $NonMaybeType<T>, index: number) => U, thisArg?: mixed) => Array<$NonMaybeType<U>>, +only: <T>(children: ChildrenArray<T>) => $NonMaybeType<T>, +toArray: <T>(children: ChildrenArray<T>) => Array<$NonMaybeType<T>>}, +Component: class React$Component<Props, State = void>, +ConcurrentMode: ({children?: React$Node}) => React$Node, +DOM: any, +Fragment: ({children?: React$Node}) => React$Node, +PropTypes: ReactPropTypes, +PureComponent: class React$PureComponent<Props, State = void>, +StrictMode: ({children?: React$Node}) => React$Node, +Suspense: React$ComponentType<{children?: React$Node, fallback?: React$Node}>, +checkPropTypes: <V>(propTypes: any, values: V, location: string, componentName: string, getStack: ?(() => ?string)) => void, +cloneElement: React$CloneElement, +createClass: React$CreateClass, +createContext: <T>(defaultValue: T, calculateChangedBits: ?((a: T, b: T) => number)) => React$Context<T>, +createElement: React$CreateElement, +createFactory: <ElementType: React$ElementType>(type: ElementType) => React$ElementFactory<ElementType>, +createRef: <T>() => {|current: (null | T)|}, +forwardRef: <Config, Instance>(render: (props: Config, ref: ({current: (null | Instance)} | (((null | Instance)) => mixed))) => React$Node) => React$AbstractComponent<Config, Instance>, +isValidElement: (element: any) => boolean, +lazy: <P>(component: () => Promise<{default: React$ComponentType<P>}>) => React$ComponentType<P>, +memo: <P>(component: React$ComponentType<P>, equal?: (P, P) => boolean) => React$ComponentType<P>, +useCallback: <T: (...args: $ReadOnlyArray<empty>) => mixed>(callback: T, inputs: ?$ReadOnlyArray<mixed>) => T, +useContext: <T>(context: React$Context<T>, observedBits: (void | number | boolean)) => T, +useEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useImperativeHandle: <T>(ref: ?({current: (T | null)} | ((inst: (T | null)) => mixed)), create: () => T, inputs: ?$ReadOnlyArray<mixed>) => void, +useLayoutEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useMemo: <T>(create: () => T, inputs: ?$ReadOnlyArray<mixed>) => T, +useReducer: ((<S, A>(reducer: (S, A) => S, initialState: S) => [S, Dispatch<A>]) & (<S, A>(reducer: (S, A) => S, initialState: S, init: void) => [S, Dispatch<A>]) & (<S, A, I>(reducer: (S, A) => S, initialArg: I, init: (I) => S) => [S, Dispatch<A>])), +useRef: <T>(initialValue: T) => {|current: T|}, +useState: <S>(initialState: ((() => S) | S)) => [S, ((((S) => S) | S)) => void], +version: string|}, +forwardRef: <Config, Instance>(render: (props: Config, ref: ({current: (null | Instance)} | (((null | Instance)) => mixed))) => React$Node) => React$AbstractComponent<Config, Instance>, +isValidElement: (element: any) => boolean, +lazy: <P>(component: () => Promise<{default: React$ComponentType<P>}>) => React$ComponentType<P>, +memo: <P>(component: React$ComponentType<P>, equal?: (P, P) => boolean) => React$ComponentType<P>, +useCallback: <T: (...args: $ReadOnlyArray<empty>) => mixed>(callback: T, inputs: ?$ReadOnlyArray<mixed>) => T, +useContext: <T>(context: React$Context<T>, observedBits: (void | number | boolean)) => T, +useDebugValue: (value: any) => void, +useEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useImperativeHandle: <T>(ref: ?({current: (T | null)} | ((inst: (T | null)) => mixed)), create: () => T, inputs: ?$ReadOnlyArray<mixed>) => void, +useLayoutEffect: (create: () => MaybeCleanUpFn, inputs: ?$ReadOnlyArray<mixed>) => void, +useMemo: <T>(create: () => T, inputs: ?$ReadOnlyArray<mixed>) => T, +useReducer: ((<S, A>(reducer: (S, A) => S, initialState: S) => [S, Dispatch<A>]) & (<S, A>(reducer: (S, A) => S, initialState: S, init: void) => [S, Dispatch<A>]) & (<S, A, I>(reducer: (S, A) => S, initialArg: I, init: (I) => S) => [S, Dispatch<A>])), +useRef: <T>(initialValue: T) => {|current: T|}, +useState: <S>(initialState: ((() => S) | S)) => [S, ((((S) => S) | S)) => void], +version: string|}",
"reasons":[],
"loc":{
"source":"react_component.js",
Expand Down Expand Up @@ -106,22 +106,15 @@ react_component.js:31:7 = {
"prop":{
"kind":"field",
"type":{
"kind":"Union",
"types":[
{"kind":"Void"},
{"kind":"Null"},
{
"kind":"Generic",
"type":{
"provenance":{"kind":"Library","loc":"[LIB] react.js:14:5,20:25"},
"name":"React$Node"
},
"kind":"type alias"
}
]
"kind":"Generic",
"type":{
"provenance":{"kind":"Library","loc":"[LIB] react.js:14:5,20:25"},
"name":"React$Node"
},
"kind":"type alias"
},
"polarity":"Neutral",
"optional":false
"optional":true
}
}
}
Expand All @@ -139,7 +132,7 @@ react_component.js:31:7 = {
"kind":"type alias"
}
},
"type":"({children: ?React$Node}) => React$Node",
"type":"({children?: React$Node}) => React$Node",
"reasons":[],
"loc":{
"source":"react_component.js",
Expand Down

0 comments on commit 6dd62e0

Please sign in to comment.