[Codegen] extract assertGenericTypeAnnotationHasExactlyOneTypeParameter to parsers-commons #34933
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
the
CLA Signed
…meter to parsers-commons
AntoineDoubovetzky
force-pushed
the
refactor/extract-assertGenericTypeAnnotationHasExactlyOneTypeParameter-to-parsers-commons
branch
from
fa21a90
to
4eeeb57
Compare
Base commit: 11f4743 |
Base commit: 11f4743 |
rshest
approved these changes
Oct 11, 2022
|
@rshest has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
cipolleschi
approved these changes
Oct 11, 2022
Comment on lines
103
to
136
| it("throws an error if typeAnnotation.typeParameters.type doesn't have the correct value depending on language", () => { | ||
| const moduleName = 'testModuleName'; | ||
| const flowTypeAnnotation = { | ||
| typeParameters: { | ||
| type: 'TypeParameterInstantiation', | ||
| }, | ||
| id: { | ||
| name: 'typeAnnotationName', | ||
| }, | ||
| }; | ||
| expect(() => | ||
| assertGenericTypeAnnotationHasExactlyOneTypeParameter( | ||
| moduleName, | ||
| flowTypeAnnotation, | ||
| 'Flow', | ||
| ), | ||
| ).toThrow(Error); | ||
|
|
||
| const typeScriptTypeAnnotation = { | ||
| typeParameters: { | ||
| type: 'TypeParameterInstantiation', | ||
| }, | ||
| typeName: { | ||
| name: 'typeAnnotationName', | ||
| }, | ||
| }; | ||
| expect(() => | ||
| assertGenericTypeAnnotationHasExactlyOneTypeParameter( | ||
| moduleName, | ||
| typeScriptTypeAnnotation, | ||
| 'TypeScript', | ||
| ), | ||
| ).toThrow(Error); | ||
| }); |
There was a problem hiding this comment.
nit: this could be split in two separate tests. In this way, if it fails, it is immediately clear what languages is making it fail.
There was a problem hiding this comment.
You're right, I corrrected
| @@ -65,7 +68,6 @@ const { | |||
| IncorrectModuleRegistryCallArgumentTypeParserError, | |||
| } = require('../../errors.js'); | |||
|
|
|||
| const invariant = require('invariant'); | |||
AntoineDoubovetzky
force-pushed
the
refactor/extract-assertGenericTypeAnnotationHasExactlyOneTypeParameter-to-parsers-commons
branch
from
b1d3b34
to
920e115
Compare
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 11, 2022
…#34935) Summary: This PR is a task from #34872: > Extract the content of the case 'Promise' ([Flow](https://github.com/facebook/react-native/blob/b444f0e44e0d8670139acea5f14c2de32c5e2ddc/packages/react-native-codegen/src/parsers/flow/modules/index.js#L90-L97), [TypeScript](https://github.com/facebook/react-native/blob/00b795642a6562fb52d6df12e367b84674994623/packages/react-native-codegen/src/parsers/typescript/modules/index.js#L197-L205)) into a single emitPromise function in the parsers-primitives.js file. Use the new function in the parsers. Note that this PR should be merged after #34933 ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - Extract contents of the case 'Promise' into a single emitPromise function inside parsers-primitives Pull Request resolved: #34935 Test Plan: I tested using jest and flow commands. Reviewed By: cipolleschi Differential Revision: D40257033 Pulled By: cipolleschi fbshipit-source-id: 0246f43c6b688629e2de1259e7f535c2cf6dd0a4
|
This pull request was successfully merged by @AntoineDoubovetzky in 29e5655. When will my fix make it into a release? | Upcoming Releases |
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 19, 2022
…34942) Summary: #34933 has been merged just after I pushed a new commit to the branch to improve tests of `assertGenericTypeAnnotationHasExactlyOneTypeParameter` function, but the last commit was not imported to the internal repository. The `assertGenericTypeAnnotationHasExactlyOneTypeParameter` can throw different types of Error, and I believe that `.toThrow(Error)` is not specific enough. So I replaced it with `toThrowErrorMatchingInlineSnapshot()`. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - Improve assertGenericTypeAnnotationHasExactlyOneTypeParameter tests in parsers-commons Pull Request resolved: #34942 Test Plan: Some test cases were ok because the assertGenericTypeAnnotationHasExactlyOneTypeParameter function threw an Error, but for the wrong reason. I've made sure that the error displayed in the inline snapshot is the one we expect. Reviewed By: cortinico Differential Revision: D40384993 Pulled By: cipolleschi fbshipit-source-id: aaa943be1e808af2c5131f7d06baf24bc3bffa31
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
…ers-commons (facebook#34933) Summary: This PR is a task from facebook#34872: > Extract the function assertGenericTypeAnnotationHasExactlyOneTypeParameter ([Flow](https://github.com/facebook/react-native/blob/b444f0e44e0d8670139acea5f14c2de32c5e2ddc/packages/react-native-codegen/src/parsers/flow/modules/index.js#L441) [TypeScript](https://github.com/facebook/react-native/blob/00b795642a6562fb52d6df12e367b84674994623/packages/react-native-codegen/src/parsers/typescript/modules/index.js#L476)) into a single function in the parsers-common.js file and replace its invocation with this new function. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - Extract assertGenericTypeAnnotationHasExactlyOneTypeParameter from the flow and typescript folders to parsers-commons Pull Request resolved: facebook#34933 Test Plan: I tested using jest and flow commands. Reviewed By: cipolleschi Differential Revision: D40257022 Pulled By: cipolleschi fbshipit-source-id: 609972914deade9afdd1c0cf1e17be1c08c1d37b
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
…facebook#34935) Summary: This PR is a task from facebook#34872: > Extract the content of the case 'Promise' ([Flow](https://github.com/facebook/react-native/blob/b444f0e44e0d8670139acea5f14c2de32c5e2ddc/packages/react-native-codegen/src/parsers/flow/modules/index.js#L90-L97), [TypeScript](https://github.com/facebook/react-native/blob/00b795642a6562fb52d6df12e367b84674994623/packages/react-native-codegen/src/parsers/typescript/modules/index.js#L197-L205)) into a single emitPromise function in the parsers-primitives.js file. Use the new function in the parsers. Note that this PR should be merged after facebook#34933 ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - Extract contents of the case 'Promise' into a single emitPromise function inside parsers-primitives Pull Request resolved: facebook#34935 Test Plan: I tested using jest and flow commands. Reviewed By: cipolleschi Differential Revision: D40257033 Pulled By: cipolleschi fbshipit-source-id: 0246f43c6b688629e2de1259e7f535c2cf6dd0a4
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
…acebook#34942) Summary: facebook#34933 has been merged just after I pushed a new commit to the branch to improve tests of `assertGenericTypeAnnotationHasExactlyOneTypeParameter` function, but the last commit was not imported to the internal repository. The `assertGenericTypeAnnotationHasExactlyOneTypeParameter` can throw different types of Error, and I believe that `.toThrow(Error)` is not specific enough. So I replaced it with `toThrowErrorMatchingInlineSnapshot()`. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - Improve assertGenericTypeAnnotationHasExactlyOneTypeParameter tests in parsers-commons Pull Request resolved: facebook#34942 Test Plan: Some test cases were ok because the assertGenericTypeAnnotationHasExactlyOneTypeParameter function threw an Error, but for the wrong reason. I've made sure that the error displayed in the inline snapshot is the one we expect. Reviewed By: cortinico Differential Revision: D40384993 Pulled By: cipolleschi fbshipit-source-id: aaa943be1e808af2c5131f7d06baf24bc3bffa31
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
This PR is a task from #34872:
Changelog
[Internal] [Changed] - Extract assertGenericTypeAnnotationHasExactlyOneTypeParameter from the flow and typescript folders to parsers-commons
Test Plan
I tested using jest and flow commands.