Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch to work around a crash in RN internals #4426

Merged
merged 3 commits into from
Jun 8, 2024
Merged

Patch to work around a crash in RN internals #4426

merged 3 commits into from
Jun 8, 2024

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Jun 8, 2024

Work around facebook/react-native#44842.

I don't have a way to verify whether this works. It would at least be nice to trigger this codepath somehow and verify it runs like before in the common case. That said, the change itself is a trivial change and maybe we can just get it in and try it.

Copy link

render bot commented Jun 8, 2024

@gaearon gaearon requested a review from haileyok June 8, 2024 02:38
Copy link

github-actions bot commented Jun 8, 2024

Old size New size Diff
7.39 MB 7.39 MB 0 B (0.00%)

Copy link
Contributor

@haileyok haileyok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, lets try it as long as CI runs

@haileyok haileyok merged commit 01b7a94 into main Jun 8, 2024
6 checks passed
@cortinico
Copy link

QQ @gaearon: are you folks building React Native Android from source? (see https://reactnative.dev/contributing/how-to-build-from-source).

If not, this change will be ignored as the touched .java file is precompiled so the changes to it inside the NPM package will be ignored.

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 10, 2024

Hmm! We're just using Expo. But I thought in the past that these overrides worked... (cc @haileyok to confirm)

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 10, 2024

Is just the Android code precompiled?

@haileyok
Copy link
Contributor

hmm. we've definitely been able to patch for iOS (we have two load bearing patches for ios right now), don't know if we have patched for android before though! would be fairly easy to test if we want by throwing somewhere and seeing if things break.

@cortinico
Copy link

Is just the Android code precompiled?

Yes the Android code is precompiled, the iOS one is not. So unless you turn on build from source on Android, the patches are ignored.

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 10, 2024

Fascinating — thanks for the heads-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants