Skip to content

Commit

Permalink
Use Content-Location header in bundle response as JS source URL (fa…
Browse files Browse the repository at this point in the history
…cebook#37501)

Summary:
Pull Request resolved: facebook#37501

This is the iOS side of the fix for facebook#36794.

That issue aside for the moment, the high-level idea here is to conceptually separate the bundle *request URL*, which represents a request for the *latest* bundle, from the *source URL* passed to JS engines, which should represent the code actually being executed. In future, we'd like to use this to refer to a point-in-time snapshot of the bundle, so that stack traces more often refer to the code that was actually run, even if it's since been updated on disk (actually implementing this isn't planned at the moment, but it helps describe the distinction).

Short term, this separation gives us a way to address the issue with JSC on iOS 16.4 by allowing Metro to provide the client with a [JSC-safe URL](react-native-community/discussions-and-proposals#646) to pass to the JS engine, even where the request URL isn't JSC-safe.

We'll deliver that URL to the client on HTTP bundle requests via the [`Content-Location`](https://www.rfc-editor.org/rfc/rfc9110#name-content-location) header, which is a published standard for communicating a location for the content provided in a successful response (typically used to provide a direct URL to an asset after content negotiation, but I think it fits here too).

For the long-term goal we should follow up with the same functionality on Android and out-of-tree platforms, but it's non-essential for anything other than iOS 16.4 at the moment.

For the issue fix to work end-to-end we'll also need to update Metro, but the two pieces are decoupled and non-breaking so it doesn't matter which lands first.

Changelog:
[iOS][Changed] Prefer `Content-Location` header in bundle response as JS source URL

Reviewed By: huntie

Differential Revision: D45950661

fbshipit-source-id: 35912ab0d4669709619b8b55d00a1b3491344ed3
  • Loading branch information
robhogan authored and facebook-github-bot committed May 23, 2023
1 parent c05d822 commit 6d01773
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 5 deletions.
12 changes: 11 additions & 1 deletion packages/react-native/React/Base/RCTJavaScriptLoader.mm
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,17 @@ static void attemptAsynchronousLoadOfBundleAtURL(
return;
}

RCTSource *source = RCTSourceCreate(scriptURL, data, data.length);
// Prefer `Content-Location` as the canonical source URL, if given, or fall back to scriptURL.
NSURL *sourceURL = scriptURL;
NSString *contentLocationHeader = headers[@"Content-Location"];
if (contentLocationHeader) {
NSURL *contentLocationURL = [NSURL URLWithString:contentLocationHeader relativeToURL:scriptURL];
if (contentLocationURL) {
sourceURL = contentLocationURL;
}
}

RCTSource *source = RCTSourceCreate(sourceURL, data, data.length);
parseHeaders(headers, source);
onComplete(nil, source);
}
Expand Down
13 changes: 9 additions & 4 deletions packages/react-native/React/CxxBridge/RCTCxxBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ - (void)start
// Load the source asynchronously, then store it for later execution.
dispatch_group_enter(prepareBridge);
__block NSData *sourceCode;
__block NSURL *sourceURL = self.bundleURL;

#if RCT_DEV_MENU && __has_include(<React/RCTDevLoadingViewProtocol.h>)
{
Expand All @@ -490,6 +491,9 @@ - (void)start
}

sourceCode = source.data;
if (source.url) {
sourceURL = source.url;
}
dispatch_group_leave(prepareBridge);
}
onProgress:^(RCTLoadingProgress *progressData) {
Expand All @@ -504,7 +508,7 @@ - (void)start
dispatch_group_notify(prepareBridge, dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{
RCTCxxBridge *strongSelf = weakSelf;
if (sourceCode && strongSelf.loading) {
[strongSelf executeSourceCode:sourceCode sync:NO];
[strongSelf executeSourceCode:sourceCode withSourceURL:sourceURL sync:NO];
}
});
RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @"");
Expand Down Expand Up @@ -1050,7 +1054,7 @@ - (void)registerModuleForFrameUpdates:(id<RCTBridgeModule>)module withModuleData
[_displayLink registerModuleForFrameUpdates:module withModuleData:moduleData];
}

- (void)executeSourceCode:(NSData *)sourceCode sync:(BOOL)sync
- (void)executeSourceCode:(NSData *)sourceCode withSourceURL:(NSURL *)url sync:(BOOL)sync
{
// This will get called from whatever thread was actually executing JS.
dispatch_block_t completion = ^{
Expand All @@ -1075,12 +1079,13 @@ - (void)executeSourceCode:(NSData *)sourceCode sync:(BOOL)sync
};

if (sync) {
[self executeApplicationScriptSync:sourceCode url:self.bundleURL];
[self executeApplicationScriptSync:sourceCode url:url];
completion();
} else {
[self enqueueApplicationScript:sourceCode url:self.bundleURL onComplete:completion];
[self enqueueApplicationScript:sourceCode url:url onComplete:completion];
}

// Use the original request URL here - HMRClient uses this to derive the /hot URL and entry point.
[self.devSettings setupHMRClientWithBundleURL:self.bundleURL];
}

Expand Down

0 comments on commit 6d01773

Please sign in to comment.