From 5abaf388cde499747e81ecb73aa01fa819dbbd47 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Wed, 8 Nov 2023 01:33:42 -0800 Subject: [PATCH] Use hermes-parser for Flow codegen specs (#39036) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39036 Changelog: [General][Changed] Use `hermes-parser` instead of `flow-parser` to parse Flow Codegen specs. `hermes-parser` is a WASM build of the Hermes parser (plus supporting code), maintained by the Flow and Hermes teams. It is the recommended way of parsing Flow code in Node and its benefits (compared to `flow-parser`) include better performance and improved type safety. Here we update `react-native/codegen` to use `hermes-parser` instead of `flow-parser`. Both parsers produce ASTs that conform to the ESTree spec so this is mostly a drop-in replacement. In future work we should be able to use the improved AST types available in `hermes-estree` to improve type safety within `react-native/codegen` itself. Reviewed By: huntie Differential Revision: D48384078 fbshipit-source-id: 310ad150ec62671ba395b0e2f6415ccae97ac04d --- packages/eslint-plugin-specs/package.json | 1 - packages/react-native-codegen/package.json | 4 +- .../parsers/__tests__/parsers-commons-test.js | 6 +- .../module-parser-snapshot-test.js.snap | 28 ++----- .../parsers/flow/parseFlowAndThrowErrors.js | 75 +++++-------------- yarn.lock | 2 +- 6 files changed, 30 insertions(+), 86 deletions(-) diff --git a/packages/eslint-plugin-specs/package.json b/packages/eslint-plugin-specs/package.json index cf8a89fce1f047..339739794dc032 100644 --- a/packages/eslint-plugin-specs/package.json +++ b/packages/eslint-plugin-specs/package.json @@ -32,7 +32,6 @@ "@babel/plugin-transform-flow-strip-types": "^7.20.0", "@babel/preset-flow": "^7.20.0", "@react-native/codegen": "*", - "flow-parser": "^0.206.0", "make-dir": "^2.1.0", "pirates": "^4.0.1", "source-map-support": "0.5.0" diff --git a/packages/react-native-codegen/package.json b/packages/react-native-codegen/package.json index 69ccde2ba2d3df..193583d5dbef77 100644 --- a/packages/react-native-codegen/package.json +++ b/packages/react-native-codegen/package.json @@ -30,8 +30,7 @@ ], "dependencies": { "@babel/parser": "^7.20.0", - "@babel/code-frame": "^7.0.0", - "flow-parser": "^0.206.0", + "hermes-parser": "0.17.1", "jscodeshift": "^0.14.0", "nullthrows": "^1.1.1" }, @@ -48,6 +47,7 @@ "@babel/preset-env": "^7.20.0", "chalk": "^4.0.0", "glob": "^7.1.1", + "hermes-estree": "0.17.1", "invariant": "^2.2.4", "micromatch": "^4.0.4", "mkdirp": "^0.5.1", diff --git a/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js b/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js index d2870f40d13c4e..66ab3a1369025b 100644 --- a/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js +++ b/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js @@ -714,7 +714,7 @@ describe('buildSchema', () => { expect(getConfigTypeSpy).toHaveBeenCalledTimes(1); expect(getConfigTypeSpy).toHaveBeenCalledWith( - parser.getAst(contents), + parser.getAst(contents, 'fileName'), Visitor, ); expect(schema).toEqual({ @@ -770,7 +770,7 @@ describe('buildSchema', () => { expect(getConfigTypeSpy).toHaveBeenCalledTimes(1); expect(getConfigTypeSpy).toHaveBeenCalledWith( - parser.getAst(contents), + parser.getAst(contents, 'fileName'), Visitor, ); expect(schema).toEqual({ @@ -824,7 +824,7 @@ describe('buildSchema', () => { expect(getConfigTypeSpy).toHaveBeenCalledTimes(1); expect(getConfigTypeSpy).toHaveBeenCalledWith( - parser.getAst(contents), + parser.getAst(contents, 'fileName'), Visitor, ); expect(schema).toEqual({ diff --git a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap index 6ba4c7709483b7..43210001dc7759 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap +++ b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap @@ -3,16 +3,12 @@ exports[`RN Codegen Flow Parser Fails with error message EMPTY_ENUM_NATIVE_MODULE 1`] = `"Module NativeSampleTurboModule: Failed parsing the enum SomeEnum in NativeSampleTurboModule with the error: Enums should have at least one member and member values can not be mixed- they all must be either blank, number, or string values."`; exports[`RN Codegen Flow Parser Fails with error message MIXED_VALUES_ENUM_NATIVE_MODULE 1`] = ` -"Syntax error in path/NativeSampleTurboModule.js: Enum \`SomeEnum\` has inconsistent member initializers. Either use no initializers, or consistently use literals (either booleans, numbers, or strings) for all member initializers. - 15 | import * as TurboModuleRegistry from '../TurboModuleRegistry'; - 16 | -> 17 | export enum SomeEnum { - | ^^^^^^^^ - 18 | NUM = 1, - 19 | STR = 'str', - 20 | } - -and 1 other error in the same file." +"Syntax error in path/NativeSampleTurboModule.js: cannot use string initializer in number enum (19:2) + STR = 'str', + ^~~~~~~~~~~ +note: start of enum body (17:21) +export enum SomeEnum { + ^" `; exports[`RN Codegen Flow Parser Fails with error message NATIVE_MODULES_WITH_ARRAY_WITH_NO_TYPE_FOR_CONTENT 1`] = `"Module NativeSampleTurboModule: Generic 'Array' must have type parameters."`; @@ -29,17 +25,7 @@ exports[`RN Codegen Flow Parser Fails with error message NATIVE_MODULES_WITH_UNN exports[`RN Codegen Flow Parser Fails with error message TWO_NATIVE_EXTENDING_TURBO_MODULE 1`] = `"Module NativeSampleTurboModule: Every NativeModule spec file must declare exactly one NativeModule Flow interface. This file declares 2: 'Spec', and 'Spec2'. Please remove the extraneous Flow interface declarations."`; -exports[`RN Codegen Flow Parser Fails with error message TWO_NATIVE_MODULES_EXPORTED_WITH_DEFAULT 1`] = ` -"Syntax error in path/NativeSampleTurboModule.js: Duplicate export for \`default\` - 17 | - 18 | export default TurboModuleRegistry.getEnforcing('SampleTurboModule1'); -> 19 | export default TurboModuleRegistry.getEnforcing('SampleTurboModule2'); - | ^^^^^^^ - 20 | - 21 | - -and 1 other error in the same file." -`; +exports[`RN Codegen Flow Parser Fails with error message TWO_NATIVE_MODULES_EXPORTED_WITH_DEFAULT 1`] = `"Module NativeSampleTurboModule: No Flow interfaces extending TurboModule were detected in this NativeModule spec."`; exports[`RN Codegen Flow Parser can generate fixture ANDROID_ONLY_NATIVE_MODULE 1`] = ` "{ diff --git a/packages/react-native-codegen/src/parsers/flow/parseFlowAndThrowErrors.js b/packages/react-native-codegen/src/parsers/flow/parseFlowAndThrowErrors.js index ab1685c6195aa1..8ffb03422e4fc0 100644 --- a/packages/react-native-codegen/src/parsers/flow/parseFlowAndThrowErrors.js +++ b/packages/react-native-codegen/src/parsers/flow/parseFlowAndThrowErrors.js @@ -10,69 +10,28 @@ 'use strict'; -// $FlowFixMe[untyped-import] -const {codeFrameColumns} = require('@babel/code-frame'); -// $FlowFixMe[untyped-import] there's no flowtype flow-parser -const flowParser = require('flow-parser'); +import type {Program as ESTreeProgram} from 'hermes-estree'; -class FlowParserSyntaxError extends Error { - constructor( - code: string, - filename: ?string, - errors: $ReadOnlyArray<{ - loc: $ReadOnly<{ - start: $ReadOnly<{line: number, column: number}>, - end: $ReadOnly<{line: number, column: number}>, - }>, - message: string, - }>, - ) { - const firstError = errors[0]; - const codeFrame = codeFrameColumns( - code, - { - start: { - line: firstError.loc.start.line, - // flow-parser returns 0-indexed columns but Babel expects 1-indexed - column: firstError.loc.start.column + 1, - }, - end: { - line: firstError.loc.end.line, - // flow-parser returns 0-indexed columns but Babel expects 1-indexed - column: firstError.loc.end.column + 1, - }, - }, - { - forceColor: false, - }, - ); - const additionalErrorsMessage = errors.length - ? '\n\nand ' + - errors.length + - ' other error' + - (errors.length > 1 ? 's' : '') + - ' in the same file.' - : ''; - - super( - (filename != null ? `Syntax error in ${filename}: ` : 'Syntax error: ') + - firstError.message + - '\n' + - codeFrame + - additionalErrorsMessage, - ); - } -} +const hermesParser = require('hermes-parser'); function parseFlowAndThrowErrors( code: string, options: $ReadOnly<{filename?: ?string}> = {}, -): $FlowFixMe { - const ast = flowParser.parse(code, { - enums: true, - }); - if (ast.errors && ast.errors.length) { - throw new FlowParserSyntaxError(code, options.filename, ast.errors); +): ESTreeProgram { + let ast; + try { + ast = hermesParser.parse(code, { + // Produce an ESTree-compliant AST + babel: false, + // Parse Flow without a pragma + flow: 'all', + ...(options.filename != null ? {sourceFilename: options.filename} : {}), + }); + } catch (e) { + if (options.filename != null) { + e.message = `Syntax error in ${options.filename}: ${e.message}`; + } + throw e; } return ast; } diff --git a/yarn.lock b/yarn.lock index dfbb950f5c2c49..616deca17b7c95 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5149,7 +5149,7 @@ flow-enums-runtime@^0.0.6: resolved "https://registry.yarnpkg.com/flow-enums-runtime/-/flow-enums-runtime-0.0.6.tgz#5bb0cd1b0a3e471330f4d109039b7eba5cb3e787" integrity sha512-3PYnM29RFXwvAN6Pc/scUfkI7RwhQ/xqyLUyPNlXUp9S40zI8nup9tUSrTLSVnWGBN38FNiGWbwZOB6uR4OGdw== -flow-parser@0.*, flow-parser@^0.206.0: +flow-parser@0.*: version "0.206.0" resolved "https://registry.yarnpkg.com/flow-parser/-/flow-parser-0.206.0.tgz#f4f794f8026535278393308e01ea72f31000bfef" integrity sha512-HVzoK3r6Vsg+lKvlIZzaWNBVai+FXTX1wdYhz/wVlH13tb/gOdLXmlTqy6odmTBhT5UoWUbq0k8263Qhr9d88w==