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

feat: Support jsi::NativeState in Shareables #5785

Merged
merged 5 commits into from
Mar 20, 2024

Conversation

mrousavy
Copy link
Contributor

Summary

Allows Shared Objects to also retain their NativeState when copied between runtimes.

Assuming this object:

jsi::Object obj;
obj.setProperty(runtime, "name", jsi::String::forUtf8(runtime, "Marc"));

std::shared_ptr<User> user = std::make_shared<User>("Marc");
obj.setNativeState(user);

..gets sent to JS, if it will be used in a Worklet runtime it will only contain the "name" property, and it's native state field will be null.

With this PR, jsi::NativeState also gets copied over to the JS object if it is shared between runtimes, which is safe to do because it is a shared_ptr already.

Test plan

Try to use an object with Native State (e.g. VisionCamera v4 tests) in a Worklet runtime.

@mrousavy
Copy link
Contributor Author

I think jsi::NativeState was introduced in RN 0.71, right? (facebook/react-native@6179233)

If so, I could also add guards to make sure this is not part of builds on RN 0.70 or lower. lmk

@mrousavy
Copy link
Contributor Author

done

@WoLewicki
Copy link
Member

I see you did it exactly the same as in my PR, nice. The other thing we would probably need is something similar to https://github.com/software-mansion/react-native-reanimated/pull/5734/files#diff-de926588e6942029aeecf8c65e8d8292808b725b95b6bcf22b421425fd10664eR150. Did you check it on 0.74 maybe?

@mrousavy
Copy link
Contributor Author

Ah lol - that was coincidentally exactly the same approach, didn't even see your PR 😂

Okay well I mean my PR unblocks me right now, if you intend to merge your PR soon then I guess we can close my PR? Otherwise we can use my PR as an intermediate solution where all jsi::Objects can have native state.

I only tried this in RN 0.73, not 0.74 - I don't think we need that check as it is always a jsi::Object that has NativeState, never a jsi::HostObject, right? I could be mistaken here, I'm mainly working on my use-cases for NativeState..

@tomekzaw
Copy link
Member

Added some more code to properly handle passing objects with jsi::NativeState between JS and UI runtime as well as in the opposite direction (18a83f8).

Here's how this PR can be tested in Reanimated Example app:

RNRuntimeDecorator.h
class MyNativeState : public jsi::NativeState {
 public:
  explicit MyNativeState(int number) : number_(number) {}
  int number_;
};
RNRuntimeDecorator.cpp
  auto setNativeState = jsi::Function::createFromHostFunction(
      rnRuntime,
      jsi::PropNameID::forAscii(rnRuntime, "setNativeState"),
      2,
      [](jsi::Runtime &rt,
         const jsi::Value &thisValue,
         const jsi::Value *arguments,
         size_t count) -> jsi::Value {
        auto state = std::make_shared<MyNativeState>(arguments[1].asNumber());
        arguments[0].asObject(rt).setNativeState(rt, state);
        return jsi::Value::undefined();
      });
  rnRuntime.global().setProperty(
      rnRuntime, "setNativeState", std::move(setNativeState));

  auto getNativeState = jsi::Function::createFromHostFunction(
      rnRuntime,
      jsi::PropNameID::forAscii(rnRuntime, "getNativeState"),
      1,
      [](jsi::Runtime &rt,
         const jsi::Value &thisValue,
         const jsi::Value *arguments,
         size_t count) -> jsi::Value {
        const auto &obj = arguments[0].asObject(rt);
        if (!obj.hasNativeState(rt)) {
          return jsi::Value::undefined();
        }
        return jsi::Value(obj.getNativeState<MyNativeState>(rt)->number_);
      });
  rnRuntime.global().setProperty(
      rnRuntime, "getNativeState", std::move(getNativeState));
ShareablesExample.tsx
function NativeStateJSToUIDemo() {
  const handlePress = () => {
    const obj = {};
    // @ts-expect-error
    const { setNativeState, getNativeState } = global;
    setNativeState(obj, Math.random() * 1000);
    console.log(_WORKLET ? 'UI' : 'JS', getNativeState(obj));
    runOnUI(() => {
      console.log(_WORKLET ? 'UI' : 'JS', getNativeState(obj));
    })();
  };

  return <Button title="NativeState JS->UI" onPress={handlePress} />;
}

function NativeStateUIToJSDemo() {
  const handlePress = () => {
    // @ts-expect-error
    const { setNativeState, getNativeState } = global;
    function jsCallback(obj: object) {
      console.log(_WORKLET ? 'UI' : 'JS', getNativeState(obj));
    }
    runOnUI(() => {
      const obj = {};
      setNativeState(obj, Math.random() * 1000);
      console.log(_WORKLET ? 'UI' : 'JS', getNativeState(obj));
      runOnJS(jsCallback)(obj);
    })();
  };

  return <Button title="NativeState UI->JS" onPress={handlePress} />;
}

@tomekzaw tomekzaw requested review from piaskowyk and WoLewicki March 19, 2024 16:44
Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

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

👏

@tomekzaw tomekzaw added this pull request to the merge queue Mar 20, 2024
Merged via the queue into software-mansion:main with commit 167241b Mar 20, 2024
14 checks passed
@mrousavy
Copy link
Contributor Author

Awesome! Thanks guys!

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.

4 participants