Handle properly negative values in TS parser #34800
Closed
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: D39847784 |
Base commit: 277a64f |
Base commit: 277a64f |
|
This pull request was exported from Phabricator. Differential Revision: D39847784 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Sep 29, 2022
Summary: Pull Request resolved: facebook#34800 The TypeScript parser was not handling negative default values properly. The reason why is because the AST for those values is structurally different and wraps them in a `UnaryExpression` with the `-` operator. This Diff adds the support for those default values and it also add some tests in both Flow and TS. ## Changelog [General][Fixed] - Properly parse negative values Differential Revision: D39847784 fbshipit-source-id: 22e75b91ba3660ab2551478afeb9a60994263555
cipolleschi
force-pushed
the
export-D39847784
branch
from
e9c8278
to
c76e8ce
Compare
Summary: Pull Request resolved: facebook#34800 The TypeScript parser was not handling negative default values properly. The reason why is because the AST for those values is structurally different and wraps them in a `UnaryExpression` with the `-` operator. This Diff adds the support for those default values and it also add some tests in both Flow and TS. ## Changelog [General][Fixed] - Properly parse negative values Differential Revision: D39847784 fbshipit-source-id: 6885cb8eda0659e98aa208a43d027047082a2506
|
This pull request was exported from Phabricator. Differential Revision: D39847784 |
cipolleschi
force-pushed
the
export-D39847784
branch
from
c76e8ce
to
c57456a
Compare
|
This pull request was successfully merged by @cipolleschi in f3c98c5. When will my fix make it into a release? | Upcoming Releases |
4 tasks
kkafar
added a commit
to software-mansion/react-native-screens
that referenced
this pull request
Jan 18, 2023
## Description Issues * [x] ~~`activityState` is set to `0.0` by default in codegened files despite being explicitly set to `WithDefault<Float, -1.0>` in config (it should be `-1.0`). Tested it multiple times it looks like you can't set number < 0 for default when using TypeScript - tested with both `Int32` and `Float` (with Flow it worked fine).~~ * [x] ~~`yarn lint` fails~~ * [x] ~#1631 There are no other differences between codegened files -- I checked it file by file with diff tool. Update: It was confirmed by React Native developer that this indeed is a bug and will be fixed with 0.71. Update: * facebook/react-native#34800 was merged and is available is `0.71` rc branches of React Native. ### ***Important note*** Merging this PR will be breaking change for Fabric support for `react-native` versions lower than `0.71.0` (`activityState` & `sheetCornerRadius` will be broken). ## Changes * Migrated codegen spec files from Flow to TS. * Also bumped `@types/react-native` to `0.69.6` [(see explanation)](d2ed6ec) * Had to do some minor type-fixing in our types * Had to patch `react-navigation-stack` types. This dependency is required for v4. <!-- ## Screenshots / GIFs Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. ### Before ### After --> ## Test code and steps to reproduce I created a backup of codegened code from before this PR. After applying these changes I ran: `./gradlew :react-native-screens:generateCodegenArtifactsFromSchema` (or just build application) and then I manually compared codegend files using diff checker - the output code is the same. ## Checklist - [x] Ensured that CI passes (there is a known issue with e2e on iOS, it will be handled separately)
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
Summary: Pull Request resolved: facebook#34800 The TypeScript parser was not handling negative default values properly. The reason why is because the AST for those values is structurally different and wraps them in a `UnaryExpression` with the `-` operator. This Diff adds the support for those default values and it also add some tests in both Flow and TS. ## Changelog [General][Fixed] - Properly parse negative values Reviewed By: cortinico Differential Revision: D39847784 fbshipit-source-id: 95fc5768987477c540a54a7c4e4ff785d7a1e5d7
mccoyplayer
pushed a commit
to mccoyplayer/reactScreen
that referenced
this pull request
Feb 9, 2024
## Description Issues * [x] ~~`activityState` is set to `0.0` by default in codegened files despite being explicitly set to `WithDefault<Float, -1.0>` in config (it should be `-1.0`). Tested it multiple times it looks like you can't set number < 0 for default when using TypeScript - tested with both `Int32` and `Float` (with Flow it worked fine).~~ * [x] ~~`yarn lint` fails~~ * [x] ~software-mansion/react-native-screens#1631 There are no other differences between codegened files -- I checked it file by file with diff tool. Update: It was confirmed by React Native developer that this indeed is a bug and will be fixed with 0.71. Update: * facebook/react-native#34800 was merged and is available is `0.71` rc branches of React Native. ### ***Important note*** Merging this PR will be breaking change for Fabric support for `react-native` versions lower than `0.71.0` (`activityState` & `sheetCornerRadius` will be broken). ## Changes * Migrated codegen spec files from Flow to TS. * Also bumped `@types/react-native` to `0.69.6` [(see explanation)](software-mansion/react-native-screens@d2ed6ec) * Had to do some minor type-fixing in our types * Had to patch `react-navigation-stack` types. This dependency is required for v4. <!-- ## Screenshots / GIFs Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. ### Before ### After --> ## Test code and steps to reproduce I created a backup of codegened code from before this PR. After applying these changes I ran: `./gradlew :react-native-screens:generateCodegenArtifactsFromSchema` (or just build application) and then I manually compared codegend files using diff checker - the output code is the same. ## Checklist - [x] Ensured that CI passes (there is a known issue with e2e on iOS, it will be handled separately)
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:
The TypeScript parser was not handling negative default values properly. The reason why is because the AST for those values is structurally different and wraps them in a
UnaryExpressionwith the-operator.This Diff adds the support for those default values and it also add some tests in both Flow and TS.
Changelog
[General][Fixed] - Properly parse negative values
Differential Revision: D39847784