Skip to content

fix: codegen crash when parsing TS interfaces#35492

Closed
vonovak wants to merge 1 commit into
facebook:mainfrom
vonovak:patch-21
Closed

fix: codegen crash when parsing TS interfaces#35492
vonovak wants to merge 1 commit into
facebook:mainfrom
vonovak:patch-21

Conversation

@vonovak
Copy link
Copy Markdown
Contributor

@vonovak vonovak commented Nov 27, 2022

Summary

when I'm defining a turbomodule spec, I tried to extract a common parameter into an interface.

The error I get is this:

[Codegen] >>>>> Processing RNSimpleToastSpec


[Codegen] Done.
/Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/utils.js:111
        throw error;
        ^

TypeError: Cannot read properties of undefined (reading 'length')
    at isModuleInterface (/Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/modules/index.js:695:18)
    at Array.filter (<anonymous>)
    at buildModuleSchema (/Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/modules/index.js:709:44)
    at /Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/index.js:158:9
    at guard (/Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/utils.js:108:14)
    at buildSchema (/Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/index.js:157:22)
    at Object.parseFile (/Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/index.js:185:10)

After the fix I get this:

[Codegen] >>>>> Processing RNSimpleToastSpec


[Codegen] Done.
/Users/vojta/_dev/_own/simple-toast/example/node_modules/react-native-codegen/lib/parsers/typescript/utils.js:111
        throw error;
        ^

Invariant Violation: GenericTypeAnnotation 'Styles' must resolve to a TSTypeAliasDeclaration. Instead, it resolved to a 'TSInterfaceDeclaration'

Then I know that I should not be using an interface but a type

Changelog

[Internal] [Fixed] - TS codegen crash when parsing interfaces

Test Plan

tested locally

let me know if you want an automated test

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 27, 2022
@vonovak vonovak changed the title fix: TS codegen reading length of undefined fix: codegen crash when parsing TS interfaces Nov 27, 2022
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 27, 2022

Warnings
⚠️

packages/react-native-codegen/src/parsers/typescript/modules/index.js#L57 - packages/react-native-codegen/src/parsers/typescript/modules/index.js line 57 – 'UnsupportedArrayElementTypeAnnotationParserError' is assigned a value but never used. (no-unused-vars)

Generated by 🚫 dangerJS against bf4838d

@analysis-bot
Copy link
Copy Markdown

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 6e9d3bf
Branch: main

@analysis-bot
Copy link
Copy Markdown

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,103,533 +0
android hermes armeabi-v7a 6,471,857 +0
android hermes x86 7,521,168 +0
android hermes x86_64 7,380,054 +0
android jsc arm64-v8a 8,968,359 +0
android jsc armeabi-v7a 7,699,467 +0
android jsc x86 9,030,658 +0
android jsc x86_64 9,508,690 +0

Base commit: 6e9d3bf
Branch: main

@pull-bot
Copy link
Copy Markdown

PR build artifact for bf4838d is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@cipolleschi
Copy link
Copy Markdown
Contributor

Hi @vonovak, thank you for taking care of this.

For what concern testing, we have a bunch of automated tests already in place. You can run them locally with the command, from the React Native root folder:

yarn jest react-native-codegen

They are also executed in CI, which is green, so it is all good. :D

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @vonovak in 9087186.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 28, 2022
@vonovak vonovak deleted the patch-21 branch November 28, 2022 13:25
@vonovak
Copy link
Copy Markdown
Contributor Author

vonovak commented Nov 28, 2022

@cipolleschi just to let you know, the flow parser has the same code but, interestingly, does not suffer from the same bug as the TS parser - the correct error is shown there 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants