Update RecoilRootProps type for React 18 types #1718
Conversation
facebook-github-bot
added
the
CLA Signed
|
@mondaychen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
typescript/index.d.ts
Outdated
| children?: React.ReactNode | undefined, | ||
| } | { | ||
| override: false, | ||
| children?: React.ReactNode | undefined, |
There was a problem hiding this comment.
| children?: React.ReactNode | undefined, | |
| children: React.ReactNode, |
typescript/index.d.ts
Outdated
| @@ -29,7 +29,11 @@ | |||
| export type RecoilRootProps = { | |||
| initializeState?: (mutableSnapshot: MutableSnapshot) => void, | |||
| override?: true, | |||
| } | {override: false}; | |||
| children?: React.ReactNode | undefined, | |||
There was a problem hiding this comment.
Let's use this opportunity to make it required and align with the Flow type https://github.com/facebookexperimental/Recoil/blob/main/packages/recoil/core/Recoil_RecoilRoot.js#L66
| children?: React.ReactNode | undefined, | |
| children: React.ReactNode, |
(also I think undefined is included in React.ReactNode)
|
@dqn has updated the pull request. You must reimport the pull request before landing. |
|
@mondaychen Thank you for reviewing! I fixed that. |
|
@mondaychen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Can you help fix the tests? https://github.com/facebookexperimental/Recoil/blob/main/typescript/tests.ts |
|
@dqn has updated the pull request. You must reimport the pull request before landing. |
|
Fixed. |
|
@mondaychen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
typescript/tests.ts
Outdated
| @@ -34,6 +34,7 @@ | |||
| useRecoilState_TRANSITION_SUPPORT_UNSTABLE, | |||
| useRecoilValueLoadable_TRANSITION_SUPPORT_UNSTABLE, | |||
| } from 'recoil'; | |||
| import React = require('react'); | |||
There was a problem hiding this comment.
This is strange. Should this be import * as React from 'react'? Or const React = require('react')? Now it looks like a mix of the two
There was a problem hiding this comment.
We can use default import by adding the compilerOptions esModuleInterop: true, but I use namespace import because I found it used in other files.
There was a problem hiding this comment.
Note: import = require() is a valid syntax for importing modules exported with export = .
https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require
|
@dqn has updated the pull request. You must reimport the pull request before landing. |
|
@mondaychen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Fixes facebookexperimental/Recoil#1717 Since types/react v18, Implicit `children` was removed from `React.FC`. This change works for types/react v18 and earlier. DefinitelyTyped/DefinitelyTyped#56210 Pull Request resolved: facebookexperimental/Recoil#1718 Reviewed By: drarmstr Differential Revision: D35501855 Pulled By: mondaychen fbshipit-source-id: cf469b96a5220966718270e43e3f0503e144acf3
Summary: Fixes facebookexperimental/Recoil#1717 Since types/react v18, Implicit `children` was removed from `React.FC`. This change works for types/react v18 and earlier. DefinitelyTyped/DefinitelyTyped#56210 Pull Request resolved: facebookexperimental/Recoil#1718 Reviewed By: drarmstr Differential Revision: D35501855 Pulled By: mondaychen fbshipit-source-id: cf469b96a5220966718270e43e3f0503e144acf3
Summary: Fixes facebookexperimental/Recoil#1717 Since types/react v18, Implicit `children` was removed from `React.FC`. This change works for types/react v18 and earlier. DefinitelyTyped/DefinitelyTyped#56210 Pull Request resolved: facebookexperimental/Recoil#1718 Reviewed By: drarmstr Differential Revision: D35501855 Pulled By: mondaychen fbshipit-source-id: cf469b96a5220966718270e43e3f0503e144acf3
Fixes #1717
Since @types/react v18, Implicit
childrenwas removed fromReact.FC. This change works for @types/react v18 and earlier.DefinitelyTyped/DefinitelyTyped#56210