Skip to content

Commit 764ba56

Browse files
CAMOBAPclaude
andcommitted
fix: address review issues in Expo managed workflow support
- Fix stale comment referencing RCTJavaScriptDidLoadNotification (the implementation uses KVO, not the notification) - Forward unhandled KVO observations to super in observeValueForKeyPath:ofObject:change:context: - Remove duplicate #import <objc/runtime.h> in SandboxReactNativeDelegate.mm - Use a typed static context pointer (kExpoSourceUrlKVOContext) for KVO registration/removal instead of nil, preventing accidental dispatch of superclass observations into our handler - Correct infiniteLoop comment: hangs sandbox JS thread only, host app stays responsive (each sandbox runs in its own Hermes instance) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6bfb1c6 commit 764ba56

3 files changed

Lines changed: 14 additions & 10 deletions

File tree

apps/expo-demo/CrashIfYouCanDemo.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ export default function CrashIfYouCanDemo() {
3131
.catch((err: any) => console.log(err.message))
3232
}
3333

34+
// Hangs the sandbox JS thread only — the host app stays responsive because
35+
// each sandbox runs in its own Hermes instance and JS thread.
3436
const infiniteLoop = () => {
3537
while (true) {}
3638
}

packages/react-native-sandbox/ios/SandboxReactNativeDelegate.mm

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,10 @@
1717
#import <React/RCTBridge+Private.h>
1818
#import <React/RCTBridge.h>
1919
#import <React/RCTBundleURLProvider.h>
20-
#import <objc/runtime.h>
2120
#import <React/RCTFollyConvert.h>
2221
#import <ReactAppDependencyProvider/RCTAppDependencyProvider.h>
2322
#import <ReactCommon/RCTInteropTurboModule.h>
2423
#import <ReactCommon/RCTTurboModule.h>
25-
2624
#import <objc/runtime.h>
2725

2826
#include <fmt/format.h>

packages/react-native-sandbox/ios/SandboxReactNativeViewComponentView.mm

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ static void ensureSharedFactories()
3939
});
4040
}
4141

42+
static void *kExpoSourceUrlKVOContext = &kExpoSourceUrlKVOContext;
43+
4244
#pragma mark - SandboxReactNativeViewComponentView
4345

4446
@interface SandboxReactNativeViewComponentView () <RCTSandboxReactNativeViewViewProtocol>
@@ -183,11 +185,13 @@ - (void)observeValueForKeyPath:(NSString *)keyPath
183185
change:(NSDictionary *)change
184186
context:(void *)context
185187
{
186-
if ([keyPath isEqualToString:@"sourceUrl"] && change[NSKeyValueChangeNewKey] != [NSNull null]) {
187-
[object removeObserver:self forKeyPath:@"sourceUrl"];
188+
if (context == kExpoSourceUrlKVOContext && change[NSKeyValueChangeNewKey] != [NSNull null]) {
189+
[object removeObserver:self forKeyPath:@"sourceUrl" context:kExpoSourceUrlKVOContext];
188190
self.isObservingExpoSourceUrl = NO;
189191
self.didScheduleLoad = NO;
190192
[self scheduleReactViewLoad];
193+
} else {
194+
[super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
191195
}
192196
}
193197

@@ -227,9 +231,9 @@ - (void)loadReactNativeView
227231
if (!self.reactNativeFactory) {
228232
// Guard: verify the bundle URL is resolvable before creating the factory.
229233
// In Expo dev-client builds EXDevLauncherController.sourceUrl is nil until the
230-
// host app JS bundle finishes loading (set in _initAppWithUrl:bundleUrl: on main thread).
231-
// Rather than polling, subscribe once to RCTJavaScriptDidLoadNotification which fires
232-
// after the host bundle loads — at that point sourceUrl is guaranteed to be set.
234+
// host app JS bundle finishes loading (~1-2s after deep-link, set asynchronously on
235+
// main thread after the network check in EXDevLauncherController completes).
236+
// If it's nil we register a one-shot KVO observer and retry when it becomes non-nil.
233237
NSURL *resolvedURL = [self.reactNativeDelegate bundleURL];
234238
if (!resolvedURL) {
235239
#if DEBUG
@@ -246,7 +250,7 @@ - (void)loadReactNativeView
246250
[controller addObserver:self
247251
forKeyPath:@"sourceUrl"
248252
options:NSKeyValueObservingOptionNew
249-
context:nil];
253+
context:kExpoSourceUrlKVOContext];
250254
}
251255
}
252256
}
@@ -363,7 +367,7 @@ - (void)prepareForRecycle
363367
if (devLauncherClass) {
364368
id controller = [devLauncherClass performSelector:@selector(sharedInstance)];
365369
if (controller) {
366-
[controller removeObserver:self forKeyPath:@"sourceUrl"];
370+
[controller removeObserver:self forKeyPath:@"sourceUrl" context:kExpoSourceUrlKVOContext];
367371
}
368372
}
369373
self.isObservingExpoSourceUrl = NO;
@@ -380,7 +384,7 @@ - (void)dealloc
380384
if (devLauncherClass) {
381385
id controller = [devLauncherClass performSelector:@selector(sharedInstance)];
382386
if (controller) {
383-
[controller removeObserver:self forKeyPath:@"sourceUrl"];
387+
[controller removeObserver:self forKeyPath:@"sourceUrl" context:kExpoSourceUrlKVOContext];
384388
}
385389
}
386390
self.isObservingExpoSourceUrl = NO;

0 commit comments

Comments
 (0)