Throw Flow syntax errors instead of continuing to process the AST #39035
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
Base commit: 5a7ae61 |
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
motiz88
force-pushed
the
export-D48385786
branch
from
293e161
to
0811ab1
Compare
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Aug 23, 2023
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: c9a55add5adc050e54503274774c003fb36ef2c4
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Aug 24, 2023
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: 240ac5b8dce6cfd496bdc42d2e959e540384451d
motiz88
force-pushed
the
export-D48385786
branch
from
0811ab1
to
7713614
Compare
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Aug 24, 2023
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: bf53743d879bbfe7b3a126c263ee3336ba258110
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Aug 25, 2023
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: 8d7306bfb71df1786426a46b028e11d9eaeb449a
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Aug 25, 2023
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: 6b230d27281959d4800edd16482e669dd57c49a7
motiz88
force-pushed
the
export-D48385786
branch
from
7713614
to
8884b3f
Compare
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Aug 25, 2023
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: e7669b5daac1bf8f08a9d8ef5cee3040e7febc67
motiz88
force-pushed
the
export-D48385786
branch
from
8884b3f
to
2c9035e
Compare
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Aug 25, 2023
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: 6beff46b409b265f48b8c3458c7b4ad63b7081fa
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Nov 8, 2023
…cebook#39035) Summary: Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Nov 8, 2023
…cebook#39035) Summary: Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786
motiz88
force-pushed
the
export-D48385786
branch
from
2c9035e
to
f6f9a7c
Compare
motiz88
added a commit
to motiz88/react-native
that referenced
this pull request
Nov 8, 2023
…cebook#39035) Summary: Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786
motiz88
force-pushed
the
export-D48385786
branch
from
f6f9a7c
to
9f817f8
Compare
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
…cebook#39035) Summary: Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786
motiz88
force-pushed
the
export-D48385786
branch
from
9f817f8
to
a1c0119
Compare
|
This pull request was exported from Phabricator. Differential Revision: D48385786 |
|
This pull request was successfully merged by @motiz88 in a74765e. When will my fix make it into a release? | Upcoming Releases |
|
This pull request has been merged in a74765e. |
Othinn
pushed a commit
to Othinn/react-native
that referenced
this pull request
Jan 9, 2024
…cebook#39035) Summary: Pull Request resolved: facebook#39035 Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored. Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame). **This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained. Reviewed By: huntie Differential Revision: D48385786 fbshipit-source-id: c7e1f5fb64a61fb0eb9e9f8f7501b43264c9626c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored.
Instead of throwing errors like most parsers, the
flow-parserpackage returns errors as part of the AST (along with a best-effort parse). It turns out thatreact-native/codegenignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame).This change is theoretically breaking for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained.
Differential Revision: D48385786