From 321efa94487e7a539b378e835137582fa38c767e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Ma=C5=82ecki?= Date: Mon, 17 Mar 2025 06:47:23 -0700 Subject: [PATCH] Add lint rule preventing "Node" imports from react and migrate "Node" types to use React namespace in react-native (#50030) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/50030 In Flow importing Node from react is equivalent importing ReactNode in Typescript. During translation Node is not translated to ReactNode but React.Node is translated to React.ReactNode. The easiest solution is to migrate all "Node" types to "React.Node" so that the translation is correct. To make sure that everyone follow the lint rule is added that checks for Node imports from react. Changelog: [Internal] - Added lint rule preventing "Node" imports from react and migrated "Node" types to use React namespace in react-native Reviewed By: javache, huntie Differential Revision: D71189533 fbshipit-source-id: baea8feb46be3dc30b6e58bcefe140655ec0530a --- .eslintrc.js | 1 + .../Components/View/ViewPropTypes.js | 7 +-- .../Libraries/Image/ImageProps.js | 5 ++- .../components/DebugInstructions.js | 4 +- .../NewAppScreen/components/Header.js | 4 +- .../NewAppScreen/components/HermesBadge.js | 4 +- .../NewAppScreen/components/LearnMoreLinks.js | 4 +- .../components/ReloadInstructions.js | 4 +- .../react-native/Libraries/Text/TextProps.js | 5 ++- .../__snapshots__/public-api-test.js.snap | 6 +-- .../__tests__/no-react-node-imports-test.js | 39 ++++++++++++++++ tools/eslint/rules/no-react-node-imports.js | 44 +++++++++++++++++++ 12 files changed, 102 insertions(+), 25 deletions(-) create mode 100644 tools/eslint/rules/__tests__/no-react-node-imports-test.js create mode 100644 tools/eslint/rules/no-react-node-imports.js diff --git a/.eslintrc.js b/.eslintrc.js index 44f80b90415..0dbe6472f4d 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -74,6 +74,7 @@ module.exports = { 'lint/no-haste-imports': 2, 'lint/no-react-native-imports': 2, 'lint/require-extends-error': 2, + 'lint/no-react-node-imports': 2, }, }, { diff --git a/packages/react-native/Libraries/Components/View/ViewPropTypes.js b/packages/react-native/Libraries/Components/View/ViewPropTypes.js index 92ebd50093e..642bf2f3cfa 100644 --- a/packages/react-native/Libraries/Components/View/ViewPropTypes.js +++ b/packages/react-native/Libraries/Components/View/ViewPropTypes.js @@ -15,17 +15,18 @@ import type {ViewStyleProp} from '../../StyleSheet/StyleSheet'; import type { BlurEvent, FocusEvent, + GestureResponderEvent, LayoutChangeEvent, LayoutRectangle, MouseEvent, PointerEvent, - GestureResponderEvent, } from '../../Types/CoreEventTypes'; import type { AccessibilityActionEvent, AccessibilityProps, } from './ViewAccessibility'; -import type {Node} from 'react'; + +import React from 'react'; export type ViewLayout = LayoutRectangle; export type ViewLayoutEvent = LayoutChangeEvent; @@ -357,7 +358,7 @@ export type ViewPropsIOS = $ReadOnly<{ }>; type ViewBaseProps = $ReadOnly<{ - children?: Node, + children?: React.Node, style?: ?ViewStyleProp, /** diff --git a/packages/react-native/Libraries/Image/ImageProps.js b/packages/react-native/Libraries/Image/ImageProps.js index a0216073a6f..c2a9ce254f3 100644 --- a/packages/react-native/Libraries/Image/ImageProps.js +++ b/packages/react-native/Libraries/Image/ImageProps.js @@ -24,7 +24,8 @@ import type { import typeof Image from './Image'; import type {ImageResizeMode} from './ImageResizeMode'; import type {ImageSource, ImageURISource} from './ImageSource'; -import type {ElementRef, Node, RefSetter} from 'react'; +import type React from 'react'; +import type {ElementRef, RefSetter} from 'react'; export type ImageSourcePropType = ImageSource; @@ -341,7 +342,7 @@ export type ImageProps = $ReadOnly<{ export type ImageBackgroundProps = $ReadOnly<{ ...ImageProps, - children?: Node, + children?: React.Node, /** * Style applied to the outer View component diff --git a/packages/react-native/Libraries/NewAppScreen/components/DebugInstructions.js b/packages/react-native/Libraries/NewAppScreen/components/DebugInstructions.js index 9963fe69081..4dcee10987c 100644 --- a/packages/react-native/Libraries/NewAppScreen/components/DebugInstructions.js +++ b/packages/react-native/Libraries/NewAppScreen/components/DebugInstructions.js @@ -8,8 +8,6 @@ * @format */ -import type {Node} from 'react'; - import StyleSheet from '../../StyleSheet/StyleSheet'; import Text from '../../Text/Text'; import Platform from '../../Utilities/Platform'; @@ -21,7 +19,7 @@ const styles = StyleSheet.create({ }, }); -const DebugInstructions: () => Node = Platform.select({ +const DebugInstructions: () => React.Node = Platform.select({ ios: () => ( Press Cmd + D in the simulator or{' '} diff --git a/packages/react-native/Libraries/NewAppScreen/components/Header.js b/packages/react-native/Libraries/NewAppScreen/components/Header.js index 5e44c81a37b..13e62b3e2e4 100644 --- a/packages/react-native/Libraries/NewAppScreen/components/Header.js +++ b/packages/react-native/Libraries/NewAppScreen/components/Header.js @@ -8,8 +8,6 @@ * @format */ -import type {Node} from 'react'; - import ImageBackground from '../../Image/ImageBackground'; import StyleSheet from '../../StyleSheet/StyleSheet'; import Text from '../../Text/Text'; @@ -18,7 +16,7 @@ import Colors from './Colors'; import HermesBadge from './HermesBadge'; import React from 'react'; -const Header = (): Node => { +const Header = (): React.Node => { const isDarkMode = useColorScheme() === 'dark'; return ( { +const HermesBadge = (): React.Node => { const isDarkMode = useColorScheme() === 'dark'; const version = global.HermesInternal?.getRuntimeProperties?.()['OSS Release Version'] ?? diff --git a/packages/react-native/Libraries/NewAppScreen/components/LearnMoreLinks.js b/packages/react-native/Libraries/NewAppScreen/components/LearnMoreLinks.js index 27195e3c4a2..39c9b56e820 100644 --- a/packages/react-native/Libraries/NewAppScreen/components/LearnMoreLinks.js +++ b/packages/react-native/Libraries/NewAppScreen/components/LearnMoreLinks.js @@ -8,8 +8,6 @@ * @format */ -import type {Node} from 'react'; - import TouchableOpacity from '../../Components/Touchable/TouchableOpacity'; import View from '../../Components/View/View'; import openURLInBrowser from '../../Core/Devtools/openURLInBrowser'; @@ -81,7 +79,7 @@ const links = [ }, ]; -const LinkList = (): Node => { +const LinkList = (): React.Node => { const isDarkMode = useColorScheme() === 'dark'; return ( diff --git a/packages/react-native/Libraries/NewAppScreen/components/ReloadInstructions.js b/packages/react-native/Libraries/NewAppScreen/components/ReloadInstructions.js index 6c1eba2fdff..145f34163c7 100644 --- a/packages/react-native/Libraries/NewAppScreen/components/ReloadInstructions.js +++ b/packages/react-native/Libraries/NewAppScreen/components/ReloadInstructions.js @@ -8,8 +8,6 @@ * @format */ -import type {Node} from 'react'; - import StyleSheet from '../../StyleSheet/StyleSheet'; import Text from '../../Text/Text'; import Platform from '../../Utilities/Platform'; @@ -21,7 +19,7 @@ const styles = StyleSheet.create({ }, }); -const ReloadInstructions: () => Node = Platform.select({ +const ReloadInstructions: () => React.Node = Platform.select({ ios: () => ( Press Cmd + R in the simulator to diff --git a/packages/react-native/Libraries/Text/TextProps.js b/packages/react-native/Libraries/Text/TextProps.js index e5ad77e77be..fc1a20a0230 100644 --- a/packages/react-native/Libraries/Text/TextProps.js +++ b/packages/react-native/Libraries/Text/TextProps.js @@ -24,7 +24,8 @@ import type { PointerEvent, TextLayoutEvent, } from '../Types/CoreEventTypes'; -import type {Node} from 'react'; + +import React from 'react'; export type PressRetentionOffset = $ReadOnly<{ top: number, @@ -169,7 +170,7 @@ type TextBaseProps = $ReadOnly<{ */ 'aria-labelledby'?: ?string, - children?: ?Node, + children?: ?React.Node, /** * When `numberOfLines` is set, this prop defines how text will be diff --git a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap index ae6d0c468e0..01d032d0f65 100644 --- a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap +++ b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap @@ -4035,7 +4035,7 @@ export type ViewPropsIOS = $ReadOnly<{ shouldRasterizeIOS?: ?boolean, }>; type ViewBaseProps = $ReadOnly<{ - children?: Node, + children?: React.Node, style?: ?ViewStyleProp, collapsable?: ?boolean, collapsableChildren?: ?boolean, @@ -4679,7 +4679,7 @@ export type ImageProps = $ReadOnly<{ }>; export type ImageBackgroundProps = $ReadOnly<{ ...ImageProps, - children?: Node, + children?: React.Node, style?: ?ViewStyleProp, imageStyle?: ?ImageStyleProp, imageRef?: RefSetter>, @@ -8136,7 +8136,7 @@ type TextBaseProps = $ReadOnly<{ \\"aria-expanded\\"?: ?boolean, \\"aria-selected\\"?: ?boolean, \\"aria-labelledby\\"?: ?string, - children?: ?Node, + children?: ?React.Node, ellipsizeMode?: ?(\\"clip\\" | \\"head\\" | \\"middle\\" | \\"tail\\"), id?: string, maxFontSizeMultiplier?: ?number, diff --git a/tools/eslint/rules/__tests__/no-react-node-imports-test.js b/tools/eslint/rules/__tests__/no-react-node-imports-test.js new file mode 100644 index 00000000000..a4e5775f2b7 --- /dev/null +++ b/tools/eslint/rules/__tests__/no-react-node-imports-test.js @@ -0,0 +1,39 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +'use strict'; + +const rule = require('../no-react-node-imports'); +const {RuleTester} = require('eslint'); + +const ruleTester = new RuleTester({ + parser: require.resolve('hermes-eslint'), + parserOptions: { + ecmaVersion: 6, + sourceType: 'module', + }, +}); + +ruleTester.run('import(...)', rule, { + valid: [ + { + code: `import React from "react";`, + }, + { + code: `import { Foo } from "react";`, + }, + ], + invalid: [ + { + code: `import { Node } from "react";`, + errors: [{messageId: 'nodeImport'}], + output: null, + }, + ], +}); diff --git a/tools/eslint/rules/no-react-node-imports.js b/tools/eslint/rules/no-react-node-imports.js new file mode 100644 index 00000000000..c96c9381824 --- /dev/null +++ b/tools/eslint/rules/no-react-node-imports.js @@ -0,0 +1,44 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +'use strict'; + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Disallow named Node imports from react', + }, + messages: { + nodeImport: + 'React.Node is React.ReactNode in TypeScript. To help us replace these correctly during type translation, prefer `import * as React` and using `React.Node`.', + }, + schema: [], + }, + + create(context) { + return { + ImportDeclaration(node) { + if (node.source.value === 'react') { + node.specifiers.forEach(specifier => { + if ( + specifier.type === 'ImportSpecifier' && + specifier.imported.name === 'Node' + ) { + context.report({ + node: specifier, + messageId: 'nodeImport', + }); + } + }); + } + }, + }; + }, +};