Skip to content

Fix Macro Errors for Windows#34299

Closed
chiaramooney wants to merge 7 commits into
react:mainfrom
chiaramooney:cm-fix-macros
Closed

Fix Macro Errors for Windows#34299
chiaramooney wants to merge 7 commits into
react:mainfrom
chiaramooney:cm-fix-macros

Conversation

@chiaramooney

@chiaramooney chiaramooney commented Jul 28, 2022

Copy link
Copy Markdown
Contributor

Summary

Fix macro errors for Windows. Current syntax breaks the build of the React Common project on Windows because the ({...}) syntax is not supported; must be replaced with lambda expressions.

Resolves #34090

Changelog

[General] [Fixed] - Fix macro errors for Windows.

@lyahdav @JoshuaGross

Test Plan

Build on react-native-windows repo. Tested in RNW app.

@facebook-github-bot facebook-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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 28, 2022
@analysis-bot

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,829,069 -15
android hermes armeabi-v7a 7,220,086 -62
android hermes x86 8,141,304 -159
android hermes x86_64 8,119,892 -118
android jsc arm64-v8a 9,706,160 -23
android jsc armeabi-v7a 8,460,785 -65
android jsc x86 9,656,637 -146
android jsc x86_64 10,254,956 -130

Base commit: 1a9fb6c
Branch: main

@analysis-bot

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

Base commit: 1a9fb6c
Branch: main

@lyahdav

lyahdav commented Jul 28, 2022

Copy link
Copy Markdown
Contributor

@JoshuaGross can you review this? Also any idea about the failing CI job?

@chiaramooney

Copy link
Copy Markdown
Contributor Author

Note more changes might be needed here because we are still working through bugs on our end produced by the 7/4 build

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@NickGerleman

NickGerleman commented Jul 29, 2022

Copy link
Copy Markdown
Contributor

Could you format these files with clang format? (clang-format 12.0.0 is used during linting)

image

@chiaramooney

Copy link
Copy Markdown
Contributor Author

Ok PR is ready for review!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@NickGerleman 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 @chiaramooney in fc26dbf.

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 Aug 2, 2022
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
Summary:
Fix macro errors for Windows. Current syntax breaks the build of the React Common project on Windows because the ({...}) syntax is not supported; must be replaced with lambda expressions.

Resolves react#34090

## 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
-->

[General] [Fixed] - Fix macro errors for Windows.

lyahdav JoshuaGross

Pull Request resolved: react#34299

Test Plan: Build on react-native-windows repo. Tested in RNW app.

Reviewed By: javache

Differential Revision: D38272966

Pulled By: NickGerleman

fbshipit-source-id: e76eac11cde173ef49465d01d793c593017f2ab7
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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New props infrastructure is not portable

6 participants