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

RJS-2888: Fixing React scheduler's call to RN CallInvoker's invokeAsync and upgrading our RNTA test app to React Native v0.75.1 #6851

Merged
merged 8 commits into from
Aug 21, 2024

Conversation

kraenhansen
Copy link
Member

@kraenhansen kraenhansen commented Aug 19, 2024

What, How & Why?

This closes #6845 and #6846 by updating RN specific scheduler to use a backwards compatible call signature for CallInvoker's invokeAsync.

A bit more context on the proposed changes

React Native's CallInvoker API changed with facebook/react-native#43375, such that the func callback argument now takes a jsi::Runtime. This was first released as react-native@0.75.0.

Care was made in that PR to provide signatures for backward compatibility, but these were only added for overload that doesn't take a SchedulerPriority as first argument (which we depend on). As I see it, we have a few ways forward:

  1. Upstream a fix to RN adding the missing backward compatibility overloads.
  2. Fix the callback we pass to take a jsi::Runtime argument.
  3. Call the backwards compatibility overload that doesn't take a SchedulerPriority (without a jsi::Runtime in the func).

Option 1 will leave realm broken for 0.75.0 and 0.75.1 forever and depends on the grace and a hotfix from the React Native team to be immediately effective. Option 2 will make the coming release of realm backwards incompatible with react-native prior to 0.75.0 which would be very unfortunate and could potentially require a major version bump on our side. I believe our best option now is 3, as that will alleviate the immediate issue fast and good compatibility, although we'll be providing less context when calling into the call invoker.

☑️ ToDos

  • 📝 Changelog entry
  • 📝 Compatibility label is updated or copied from previous entry
  • 📝 Update COMPATIBILITY.md
  • 🚦 Tests
  • 📦 Updated internal package version in consuming package.jsons (if updating internal packages)
  • 📱 Check the React Native/other sample apps work if necessary
  • 💥 Breaking label has been applied or is not necessary
  • 🔔 Mention @realm/devdocs if documentation changes are needed

@kraenhansen kraenhansen self-assigned this Aug 19, 2024
@cla-bot cla-bot bot added the cla: yes label Aug 19, 2024
@kraenhansen kraenhansen changed the title Fixing React scheduler's call to RN CallInvoker's invokeAsync and upgrading our RNTA test app to React Native v0.75.1 RJS-2888: Fixing React scheduler's call to RN CallInvoker's invokeAsync and upgrading our RNTA test app to React Native v0.75.1 Aug 19, 2024
@kraenhansen
Copy link
Member Author

This PR is currently blocked by microsoft/react-native-test-app#2189.

@kraenhansen kraenhansen merged commit b798a7b into main Aug 21, 2024
34 checks passed
@kraenhansen kraenhansen deleted the kh/react-native-0.75.1 branch August 21, 2024 10:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

> Task :realm:buildCMakeDebug[arm64-v8a][realm-js-android-binding] FAILED . React native - 0.75.1
1 participant