[ios][core] Create JSI host object to access the new expo modules #15847

tsapeta · 2022-01-08T21:00:17Z

Why

Step further to integrate our new modules architecture with JSI and running native functions synchronously.

How

Created ExpoModulesHostObject that installs into the runtime as global.ExpoModules. The host object is an abstract object where we can define our own property getter, allowing us to create JS object for each module lazily.
Since Swift doesn't understand C++ symbols, it has no direct access to JSI API. To work around this, I created another layer of abstraction to JavaScript runtime (JavaScriptRuntime) and objects (JavaScriptObject) in ObjC++ to expose some functionalities to Swift.
Added isAsync property to sweet functions and runSynchronously() modifier.
AppContext can now access the runtime, so the modules can access the runtime too!

Further plans

Make expo-random to be an expo module and be the first to use synchronous functions.
Right now all new modules can be accessed from the host object, however at the beginning we'd like to enable it only for some subset of modules (especially that events are not supported yet). Maybe useJavaScriptRuntime(true) definition component?
Cleanup stuff related to ExpoModulesProxySpec TurboModule which was an alternative implementation to NativeModulesProxy — the solution with host object is much better, more scalable and that lets us do much more magic in the future, so we can drop the TurboModule (it wasn't used by default anyway).
Ensure that JavaScriptRuntime and JavaScriptObject are thread-safe (another addition to bare JSI objects).
Move some responsibility of calling exported functions from ModuleHolder to the respective functions. See my comment in JavaScriptUtils.swift
Switch modules to use requireNativeModule(moduleName) instead of NativeModulesProxy (see related PR [core] Add requireNativeModule function #15848). We also need to replace JS examples in the docs to use this new function because we don't want to introduce developers into the proxy concept.

Test Plan

Used expo-haptics with a new synchronous function as a playground, all worked as expected.
Reloading the app works fine (it would crash on JSCRuntime deallocation if there are any unreleased objects)

packages/expo-modules-core/ios/JSI/JavaScriptObject.mm

tsapeta · 2022-01-10T09:17:06Z

packages/expo-modules-core/ios/JSI/JavaScriptObject.mm

+                   block:(nonnull JSAsyncFunctionBlock)block
+{
+  jsi::Function function = [_runtime createAsyncFunction:name argsCount:argsCount block:block];
+  _jsObjectPtr->setProperty(*[_runtime get], [name UTF8String], function);


I shoulda check if runtime is not null

I shoulda check if runtime is not null

Do you? You marked runtime as nonnullable above in the same file 🤔

It's nonnull only when it's passed as an argument to the initializer of JavaScriptObject. _runtime is marked as a __weak property, so it might be deallocated before the object gets deallocated.

packages/expo-modules-core/ios/JSI/JavaScriptRuntime.mm

bbarthec

The code looks good, even though I'm not accustomed to reading the mix of ObjC(++)/Swift code tangled together in one PR 😅
I've got one question - can you be even more explicit in regards to nullability in ObjC(++). You've annotated majority of the code, but there're some gaps (especially in functions params definitions).

packages/expo-modules-core/ios/JSI/JavaScriptObject.mm

bbarthec · 2022-01-10T11:26:22Z

packages/expo-modules-core/ios/JSI/JavaScriptObject.mm

+                   block:(nonnull JSAsyncFunctionBlock)block
+{
+  jsi::Function function = [_runtime createAsyncFunction:name argsCount:argsCount block:block];
+  _jsObjectPtr->setProperty(*[_runtime get], [name UTF8String], function);


I shoulda check if runtime is not null

Do you? You marked runtime as nonnullable above in the same file 🤔

packages/expo-modules-core/ios/JSI/ExpoModulesHostObject.h

Kudo · 2022-01-10T12:11:39Z

packages/expo-modules-core/ios/JSI/ExpoModulesHostObject.mm

+ExpoModulesHostObject::ExpoModulesHostObject(SwiftInteropBridge *swiftInterop) : swiftInterop(swiftInterop) {}
+
+ExpoModulesHostObject::~ExpoModulesHostObject() {
+  [swiftInterop setRuntime:nil];


is it intentionally to setRuntime:nil here? what if someone do global.ExpoModules = null; in javascript and after GC, will it also call to here and cleanup the runtime?

Yeah, it's intentional. Our JavaScriptRuntime wrapper holds the reference to jsi::Runtime and shared pointer to react::CallInvoker and so we should make sure they both can deallocate when the JS engine wants it.

Well, if you do global.ExpoModules = null then you actually opt into some unexpected behavior 🤪
We can also prevent this case with such JS code:

const ExpoModules = global.ExpoModules; delete global.ExpoModules; // And now use `ExpoModules` instead of `global.ExpoModules`.

This shouldn't deallocate the object 🤔 I can check out how that works in #15848

packages/expo-modules-core/ios/JSI/JavaScriptObject.h

packages/expo-modules-core/ios/JSI/JavaScriptObject.mm

Kudo

🚀

@tsapeta

Bump `expo-modules-core` to `0.7.0`: # Changelog: ## 0.7.0 — 2022-01-26 ### 🎉 New features - Allow accessing `RCTBridge` from the modules on iOS. ([#15816](expo/expo#15816) by [@tsapeta](https://github.com/tsapeta)) - Added support for native callbacks through the view props in Sweet API on iOS. ([#15731](expo/expo#15731) by [@tsapeta](https://github.com/tsapeta)) - Added support for native callbacks through the view props in Sweet API on Android. ([#15743](expo/expo#15743) by [@lukmccall](https://github.com/lukmccall)) - The `ModuleDefinition` will use class name if the `name` component wasn't provided in Sweet API on Android. ([#15738](expo/expo#15738) by [@lukmccall](https://github.com/lukmccall)) - Added `onViewDestroys` component to the `ViewManager` in Sweet API on Android. ([#15740](expo/expo#15740) by [@lukmccall](https://github.com/lukmccall)) - Added shortened `constants` component that takes `vargs Pair<String, Any?>` as an argument in Sweet API on Android. ([#15742](expo/expo#15742) by [@lukmccall](https://github.com/lukmccall)) - Introduced the concept of chainable exceptions in Sweet API on iOS. ([#15813](expo/expo#15813) by [@tsapeta](https://github.com/tsapeta)) - Sweet function closures can throw errors on iOS. ([#15849](expo/expo#15849) by [@tsapeta](https://github.com/tsapeta)) - Add `requireNativeModule` function to replace accessing native modules from `NativeModulesProxy`. ([#15848](expo/expo#15848) by [@tsapeta](https://github.com/tsapeta)) - Implemented basic functionality of JSI host object to replace `NativeModulesProxy` on iOS. ([#15847](expo/expo#15847) by [@tsapeta](https://github.com/tsapeta)) ### 🐛 Bug fixes - It's no longer possible to directly call methods from the `ModuleDefinition` in the `ViewManagers` on Android. ([#15741](expo/expo#15741) by [@lukmccall](https://github.com/lukmccall)) - Fix compatibility with react-native 0.66. ([#15914](expo/expo#15914) by [@Kudo](https://github.com/kudo)) ## 0.6.4 — 2022-01-05 ### 🐛 Bug fixes - Fix `ReactInstanceManager.onHostPause` exception from moving Android apps to background. ([#15748](expo/expo#15748) by [@Kudo](https://github.com/kudo)) ## 0.6.3 — 2021-12-16 ### 🐛 Bug fixes - Fixed the deep link wasn't passed to the application if the application wasn't running when the deep link was sent. ([#15593](expo/expo#15593) by [@lukmccall](https://github.com/lukmccall)) ## 0.6.2 — 2021-12-15 ### 🎉 New features - Add `onNewIntent` and `onBackPressed` support to `ReactActivityLifecycleListener`. ([#15550](expo/expo#15550) by [@Kudo](https://github.com/Kudo))

expo-bot added the bot: suggestions ExpoBot has some suggestions label Jan 8, 2022

tsapeta mentioned this pull request Jan 8, 2022

[core] Add requireNativeModule function #15848

Merged

tsapeta marked this pull request as ready for review January 9, 2022 19:22

tsapeta requested review from Kudo, bbarthec and lukmccall January 9, 2022 19:23

tsapeta commented Jan 10, 2022

View reviewed changes

tsapeta requested a review from wkozyra95 January 10, 2022 09:20

bbarthec reviewed Jan 10, 2022

View reviewed changes

Kudo reviewed Jan 10, 2022

View reviewed changes

wkozyra95 approved these changes Jan 10, 2022

View reviewed changes

packages/expo-modules-core/ios/JSI/JavaScriptObject.h Outdated Show resolved Hide resolved

packages/expo-modules-core/ios/JSI/JavaScriptObject.mm Outdated Show resolved Hide resolved

bbarthec approved these changes Jan 11, 2022

View reviewed changes

Kudo approved these changes Jan 11, 2022

View reviewed changes

tsapeta added 6 commits January 11, 2022 10:59

[ios][core] Make JSI host object for accessing the new modules

d695c78

Move Swift.h import to implementation file

f7e341b

virtual -> override

7b9cdf5

Various code review improvements

9155ce1

Return undefined when the module not exists

0ea0511

Add changelog entry [skip ci]

2c15564

tsapeta force-pushed the @tsapeta/ios/modules-host-object branch from 43121bf to 2c15564 Compare January 11, 2022 10:05

expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jan 11, 2022

tsapeta merged commit 6ad80de into master Jan 11, 2022

tsapeta deleted the @tsapeta/ios/modules-host-object branch January 11, 2022 10:09

tsapeta mentioned this pull request Jan 11, 2022

[ios][core] Release JS runtime objects when the runtime deallocates #15872

Merged

tsapeta changed the title ~~[ios][core] Make JSI host object for accessing the new modules~~ [ios][core] Create JSI host object to access the new expo modules Jan 11, 2022

tsapeta mentioned this pull request Jan 11, 2022

[ios] Make expo-random be an expo module #15875

Merged

lukmccall mentioned this pull request Jan 26, 2022

[modules-core] Bump version to 0.7.0 software-mansion-labs/expo-maps#24

Merged

tsapeta mentioned this pull request Mar 30, 2022

[ios] Revert TurboModule implementation of NativeModulesProxy #16825

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios][core] Create JSI host object to access the new expo modules #15847

[ios][core] Create JSI host object to access the new expo modules #15847

tsapeta commented Jan 8, 2022 •

edited

tsapeta Jan 10, 2022

bbarthec Jan 10, 2022 •

edited

tsapeta Jan 10, 2022

bbarthec left a comment •

edited

bbarthec Jan 10, 2022 •

edited

Kudo Jan 10, 2022

tsapeta Jan 10, 2022

Kudo left a comment

[ios][core] Create JSI host object to access the new expo modules #15847

[ios][core] Create JSI host object to access the new expo modules #15847

Conversation

tsapeta commented Jan 8, 2022 • edited

Why

How

Further plans

Test Plan

tsapeta Jan 10, 2022

Choose a reason for hiding this comment

bbarthec Jan 10, 2022 • edited

Choose a reason for hiding this comment

tsapeta Jan 10, 2022

Choose a reason for hiding this comment

bbarthec left a comment • edited

Choose a reason for hiding this comment

bbarthec Jan 10, 2022 • edited

Choose a reason for hiding this comment

Kudo Jan 10, 2022

Choose a reason for hiding this comment

tsapeta Jan 10, 2022

Choose a reason for hiding this comment

Kudo left a comment

Choose a reason for hiding this comment

tsapeta commented Jan 8, 2022 •

edited

bbarthec Jan 10, 2022 •

edited

bbarthec left a comment •

edited

bbarthec Jan 10, 2022 •

edited