Skip to content

Commit

Permalink
fix: source-map matching in React Native DevTools (#787)
Browse files Browse the repository at this point in the history
* fix: populate namespace in SourceMapDevToolPlugin

* chore: add TODO for fixing directory structure in dev tools

* fix: proper sourceUrl on iOS

* chore: add uniqueName to tester-app

* fix: proper sourceUrl on Android

* refactor: align both platforms handling of query param

* chore: add changeset

* chore: clang format

* refactor: review fixes
  • Loading branch information
jbroma authored Nov 8, 2024
1 parent cf42d14 commit acdd0c8
Show file tree
Hide file tree
Showing 15 changed files with 88 additions and 41 deletions.
5 changes: 5 additions & 0 deletions .changeset/cold-apes-swim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@callstack/repack": patch
---

Fix sourceURL of bundles so source maps can be matched in dev tools
1 change: 1 addition & 0 deletions apps/tester-app/rspack.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export default (env) => {
filename: 'index.bundle',
chunkFilename: '[name].chunk.bundle',
publicPath: Repack.getPublicPath({ platform, devServer }),
uniqueName: 'tester-app',
},
optimization: {
minimize,
Expand Down
1 change: 1 addition & 0 deletions apps/tester-app/webpack.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export default (env) => {
filename: 'index.bundle',
chunkFilename: '[name].chunk.bundle',
publicPath: Repack.getPublicPath({ platform, devServer }),
uniqueName: 'tester-app',
},
optimization: {
minimize,
Expand Down
2 changes: 1 addition & 1 deletion apps/tester-federation-v2/rspack.config.host-app.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default (env) => {
filename: 'index.bundle',
chunkFilename: '[name].chunk.bundle',
publicPath: Repack.getPublicPath({ platform, devServer }),
uniqueName: 'MF2-HostApp',
uniqueName: 'MF2Tester-HostApp',
},
optimization: {
minimize,
Expand Down
2 changes: 1 addition & 1 deletion apps/tester-federation-v2/rspack.config.mini-app.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default (env) => {
filename: 'index.bundle',
chunkFilename: '[name].chunk.bundle',
publicPath: Repack.getPublicPath({ platform, devServer }),
uniqueName: 'MF2-MiniApp',
uniqueName: 'MF2Tester-MiniApp',
},
optimization: {
minimize,
Expand Down
2 changes: 1 addition & 1 deletion apps/tester-federation-v2/webpack.config.host-app.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default (env) => {
filename: 'index.bundle',
chunkFilename: '[name].chunk.bundle',
publicPath: Repack.getPublicPath({ platform, devServer }),
uniqueName: 'MFTester-HostApp',
uniqueName: 'MF2Tester-HostApp',
},
optimization: {
minimize,
Expand Down
2 changes: 1 addition & 1 deletion apps/tester-federation-v2/webpack.config.mini-app.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default (env) => {
filename: 'index.bundle',
chunkFilename: '[name].chunk.bundle',
publicPath: Repack.getPublicPath({ platform, devServer }),
uniqueName: 'MFTester-MiniApp',
uniqueName: 'MF2Tester-MiniApp',
},
optimization: {
minimize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ class FileSystemScriptLoader(private val reactContext: ReactContext, private val
val path = config.url.path
val file = File(path)
val code: ByteArray = FileInputStream(file).use { it.readBytes() }
nativeLoader.evaluate(code, config.uniqueId, promise)
nativeLoader.evaluate(code, config.sourceUrl, promise)
} else {
val assetName = config.url.file.split("/").last()
val inputStream = reactContext.assets.open(assetName)
val code: ByteArray = inputStream.use { it.readBytes() }
nativeLoader.evaluate(code, config.uniqueId, promise)
nativeLoader.evaluate(code, config.sourceUrl, promise)
}
} catch (error: Exception) {
promise.reject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class RemoteScriptLoader(val reactContext: ReactContext, private val nativeLoade
throw Exception("Script file exists but could not be read: $file")
}

nativeLoader.evaluate(code, config.uniqueId, promise)
nativeLoader.evaluate(code, config.sourceUrl, promise)
} catch (error: Exception) {
promise.reject(
ScriptLoadingError.ScriptEvalFailure.code,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,21 @@ import okhttp3.MediaType.Companion.toMediaType
import okhttp3.RequestBody
import okhttp3.RequestBody.Companion.toRequestBody
import java.net.URL
import java.net.URI

data class ScriptConfig(
val scriptId: String,
val url: URL,
val query: String?,
val fetch: Boolean,
val absolute: Boolean,
val method: String,
val body: RequestBody?,
val timeout: Int,
val headers: Headers,
val verifyScriptSignature: String,
val uniqueId: String
val scriptId: String,
val url: URL,
val query: String?,
val fetch: Boolean,
val absolute: Boolean,
val method: String,
val body: RequestBody?,
val timeout: Int,
val headers: Headers,
val verifyScriptSignature: String,
val uniqueId: String,
val sourceUrl: String
) {
companion object {
fun fromReadableMap(scriptId: String, value: ReadableMap): ScriptConfig {
Expand All @@ -33,13 +35,25 @@ data class ScriptConfig(
val verifyScriptSignature = requireNotNull(value.getString("verifyScriptSignature"))
val uniqueId = requireNotNull(value.getString("uniqueId"))

val url = URL(
if (query != null) {
"$urlString?$query"
} else {
urlString
}
)
val initialUrl = URL(urlString)
val uri = initialUrl.toURI()

val sourceUrl = initialUrl.toString()

// overrides any existing query in the URL with config.query
val finalUri = if (query != null) {
URI(
uri.scheme,
uri.authority,
uri.path,
query,
uri.fragment
)
} else {
uri
}

val url = finalUri.toURL()

val headers = Headers.Builder()
val keyIterator = headersMap?.keySetIterator()
Expand All @@ -55,17 +69,18 @@ data class ScriptConfig(
val body = bodyString?.toRequestBody(contentType)

return ScriptConfig(
scriptId,
url,
query,
fetch,
absolute,
method,
body,
timeout,
headers.build(),
verifyScriptSignature,
uniqueId
scriptId,
url,
query,
fetch,
absolute,
method,
body,
timeout,
headers.build(),
verifyScriptSignature,
uniqueId,
sourceUrl
)
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/repack/ios/ScriptConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ NS_ASSUME_NONNULL_BEGIN
@property (readonly) NSNumber *timeout;
@property (readonly) NSString *verifyScriptSignature;
@property (readonly) NSString *uniqueId;
@property (readonly) NSString *sourceUrl;

#ifdef RCT_NEW_ARCH_ENABLED
+ (ScriptConfig *)fromConfig:(JS::NativeScriptManager::NormalizedScriptLocator &)config
Expand All @@ -38,7 +39,8 @@ NS_ASSUME_NONNULL_BEGIN
withBody:(nullable NSData *)body
withTimeout:(NSNumber *)timeout
withVerifyScriptSignature:(NSString *)verifyScriptSignature
withUniqueId:(NSString *)uniqueId;
withUniqueId:(NSString *)uniqueId
withSourceUrl:(NSString *)sourceUrl;

NS_ASSUME_NONNULL_END

Expand Down
17 changes: 15 additions & 2 deletions packages/repack/ios/ScriptConfig.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,22 @@ @implementation ScriptConfig
@synthesize timeout = _timeout;
@synthesize verifyScriptSignature = _verifyScriptSignature;
@synthesize uniqueId = _uniqueId;
@synthesize sourceUrl = _sourceUrl;

#ifdef RCT_NEW_ARCH_ENABLED
+ (ScriptConfig *)fromConfig:(JS::NativeScriptManager::NormalizedScriptLocator &)config
withScriptId:(nonnull NSString *)scriptId
{
NSDictionary *_Nullable headers = (NSDictionary *)config.headers();

NSURLComponents *urlComponents = [NSURLComponents componentsWithString:config.url()];
NSString *sourceUrl = urlComponents.URL.absoluteString;

// overrides any existing query in the URL with config.query
if (config.query() != nil) {
urlComponents.percentEncodedQuery = config.query();
}

NSURL *url = urlComponents.URL;

return [[ScriptConfig alloc] initWithScript:scriptId
Expand All @@ -36,7 +42,8 @@ + (ScriptConfig *)fromConfig:(JS::NativeScriptManager::NormalizedScriptLocator &
withBody:[config.body() dataUsingEncoding:NSUTF8StringEncoding]
withTimeout:[NSNumber numberWithDouble:config.timeout()]
withVerifyScriptSignature:config.verifyScriptSignature()
withUniqueId:config.uniqueId()];
withUniqueId:config.uniqueId()
withSourceUrl:sourceUrl];
}
#else
+ (ScriptConfig *)fromConfig:(NSDictionary *)config withScriptId:(nonnull NSString *)scriptId
Expand All @@ -47,6 +54,9 @@ + (ScriptConfig *)fromConfig:(NSDictionary *)config withScriptId:(nonnull NSStri
}
NSURL *url = urlComponents.URL;

urlComponents.query = nil;
NSString *sourceUrl = urlComponents.URL.absoluteString;

return [[ScriptConfig alloc] initWithScript:scriptId
withURL:url
withMethod:config[@"method"]
Expand All @@ -57,7 +67,8 @@ + (ScriptConfig *)fromConfig:(NSDictionary *)config withScriptId:(nonnull NSStri
withBody:[config[@"body"] dataUsingEncoding:NSUTF8StringEncoding]
withTimeout:config[@"timeout"]
withVerifyScriptSignature:config[@"verifyScriptSignature"]
withUniqueId:config[@"uniqueId"]];
withUniqueId:config[@"uniqueId"]
withSourceUrl:sourceUrl];
}
#endif

Expand All @@ -80,6 +91,7 @@ - (ScriptConfig *)initWithScript:(NSString *)scriptId
withTimeout:(nonnull NSNumber *)timeout
withVerifyScriptSignature:(NSString *)verifyScriptSignature
withUniqueId:(NSString *)uniqueId
withSourceUrl:(nonnull NSString *)sourceUrl
{
_scriptId = scriptId;
_url = url;
Expand All @@ -92,6 +104,7 @@ - (ScriptConfig *)initWithScript:(NSString *)scriptId
_timeout = timeout;
_verifyScriptSignature = verifyScriptSignature;
_uniqueId = uniqueId;
_sourceUrl = sourceUrl;
return self;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/repack/ios/ScriptManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ - (void)execute:(ScriptConfig *)config resolve:(RCTPromiseResolveBlock)resolve r
@throw [NSError errorWithDomain:errorMessage code:0 userInfo:nil];
}

[self evaluateJavascript:data url:config.uniqueId resolve:resolve reject:reject];
[self evaluateJavascript:data url:config.sourceUrl resolve:resolve reject:reject];
} @catch (NSError *error) {
reject(CodeExecutionFailure, error.domain, nil);
}
Expand Down Expand Up @@ -304,7 +304,7 @@ - (void)executeFromFilesystem:(ScriptConfig *)config
filesystemScriptUrl = [[NSBundle mainBundle] URLForResource:scriptName withExtension:scriptExtension];
}
NSData *data = [[NSData alloc] initWithContentsOfFile:[filesystemScriptUrl path]];
[self evaluateJavascript:data url:config.uniqueId resolve:resolve reject:reject];
[self evaluateJavascript:data url:config.sourceUrl resolve:resolve reject:reject];
} @catch (NSError *error) {
reject(CodeExecutionFailure, error.localizedDescription, nil);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/repack/src/plugins/DevelopmentPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ export class DevelopmentPlugin implements RspackPluginInstance {
module: true,
columns: true,
noSources: false,
namespace:
compiler.options.output.devtoolNamespace ??
compiler.options.output.uniqueName,
}).apply(compiler);

// setup React Refresh manually instead of using the official plugin
Expand Down
7 changes: 7 additions & 0 deletions packages/repack/src/plugins/RepackPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,20 @@ export class RepackPlugin implements RspackPluginInstance {
}).apply(compiler);

if (this.config.sourceMaps) {
// TODO Fix sourcemap directory structure
// Right now its very messy and not every node module is inside of the node module
// like React Devtools backend etc or some symilinked module appear with relative path
// We should normalize this through a custom handler and provide an output similar to Metro
new compiler.webpack.SourceMapDevToolPlugin({
test: /\.(js)?bundle$/,
filename: '[file].map',
append: `//# sourceMappingURL=[url]?platform=${this.config.platform}`,
module: true,
columns: true,
noSources: false,
namespace:
compiler.options.output.devtoolNamespace ??
compiler.options.output.uniqueName,
}).apply(compiler);
}

Expand Down

0 comments on commit acdd0c8

Please sign in to comment.