Skip to content

Add missing C++ include for prop conversion of complex array type - #35984

Closed
rshest wants to merge 1 commit into
react:mainfrom
rshest:export-D42781128
Closed

Add missing C++ include for prop conversion of complex array type#35984
rshest wants to merge 1 commit into
react:mainfrom
rshest:export-D42781128

Conversation

@rshest

Copy link
Copy Markdown
Contributor

Summary:
[Changelog][Internal]

Codegen for props parsing was failing to add a required include for the case when the type is an array of objects, which in turn use non-trivial types.

Something like:

export type NativeProps = $ReadOnly<{
...ViewProps,
bounds: $ReadOnlyArray<
$ReadOnly<{
height?: Float,
left?: Float,
top?: Float,
width?: Float,
}>,
>,
}>;

would cause compilation errors on C++ side, since the required header for the Float conversion wasn't included.

Reviewed By: cipolleschi

Differential Revision: D42781128

Summary:
[Changelog][Internal]
Codegen for props parsing was failing to add a required include for the case when the type is an array of objects, which in turn use non-trivial types.
Something like:
```
export type NativeProps = $ReadOnly<{
...ViewProps,
bounds: $ReadOnlyArray<
$ReadOnly<{
height?: Float,
left?: Float,
top?: Float,
width?: Float,
}>,
>,
}>;
```
would cause compilation errors on C++ side, since the required header for the `Float` conversion wasn't included.
Reviewed By: cipolleschi
Differential Revision: D42781128
fbshipit-source-id: 5b0ad5c3f5740c1bcb308e56cc4c1854c8f71a6a
@facebook-github-botfacebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Jan 26, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D42781128

@github-actions

Copy link
Copy Markdown
Fails
🚫

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

DetailsCATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against 0d30513

@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,463,042+0
androidhermesarmeabi-v7a7,783,401+0
androidhermesx868,936,022+0
androidhermesx86_648,793,929+0
androidjscarm64-v8a9,648,914+0
androidjscarmeabi-v7a8,383,184+0
androidjscx869,710,884+0
androidjscx86_6410,187,735+0

Base commit: 53932d0
Branch: main

@github-actions

Copy link
Copy Markdown

This pull request was successfully merged by @rshest in a00cea4.

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

@github-actionsgithub-actionsBot added the Merged This PR has been merged. label Jan 27, 2023
cipolleschi pushed a commit that referenced this pull request Feb 13, 2023
…5984)
Summary:
Pull Request resolved: #35984
[Changelog][Internal]
Codegen for props parsing was failing to add a required include for the case when the type is an array of objects, which in turn use non-trivial types.
Something like:
```
export type NativeProps = $ReadOnly<{
...ViewProps,
bounds: $ReadOnlyArray<
$ReadOnly<{
height?: Float,
left?: Float,
top?: Float,
width?: Float,
}>,
>,
}>;
```
would cause compilation errors on C++ side, since the required header for the `Float` conversion wasn't included.
Reviewed By: cipolleschi
Differential Revision: D42781128
fbshipit-source-id: d5b133b931a60e414761db0b3ed09893d3fcc9aa
@cipolleschicipolleschi mentioned this pull request Oct 11, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.fb-exportedMergedThis PR has been merged.p: FacebookPartner: FacebookPartner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@rshest@facebook-github-bot@analysis-bot