Skip to content

Commit

Permalink
Use hermes-parser for Flow codegen specs (facebook#39036)
Browse files Browse the repository at this point in the history
Summary:

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
  • Loading branch information
motiz88 authored and facebook-github-bot committed Nov 8, 2023
1 parent 2548521 commit 66c88ff
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 86 deletions.
1 change: 0 additions & 1 deletion packages/eslint-plugin-specs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions packages/react-native-codegen/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ describe('buildSchema', () => {

expect(getConfigTypeSpy).toHaveBeenCalledTimes(1);
expect(getConfigTypeSpy).toHaveBeenCalledWith(
parser.getAst(contents),
parser.getAst(contents, 'fileName'),
Visitor,
);
expect(schema).toEqual({
Expand Down Expand Up @@ -770,7 +770,7 @@ describe('buildSchema', () => {

expect(getConfigTypeSpy).toHaveBeenCalledTimes(1);
expect(getConfigTypeSpy).toHaveBeenCalledWith(
parser.getAst(contents),
parser.getAst(contents, 'fileName'),
Visitor,
);
expect(schema).toEqual({
Expand Down Expand Up @@ -824,7 +824,7 @@ describe('buildSchema', () => {

expect(getConfigTypeSpy).toHaveBeenCalledTimes(1);
expect(getConfigTypeSpy).toHaveBeenCalledWith(
parser.getAst(contents),
parser.getAst(contents, 'fileName'),
Visitor,
);
expect(schema).toEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."`;
Expand All @@ -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<Spec1>('SampleTurboModule1');
> 19 | export default TurboModuleRegistry.getEnforcing<Spec2>('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`] = `
"{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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==
Expand Down

0 comments on commit 66c88ff

Please sign in to comment.