[iOS] Allow RCTBundleURLProvider to request an inline source map

[Saadnajmi]

Summary:

See: http://blog.nparashuram.com/2019/10/debugging-react-native-ios-apps-with.html When using direct debugging with JavaScriptCore, Safari Web Inspector doesn't pick up the source map. This leads to a very sub-par developer experience debugging the JSbundle directly. The solution is to tell metro to inline the source map.

I did this by modifying RCTBundleURLProvider to have a new query parameter for inlineSourceMap, and set to true by default for JSC.

Changelog:

[IOS] [ADDED] - Added support to inline the source map via RCTBundleURLProvider

Test Plan:

I can put a breakpoint in RNTester, via Safari Web Inspector, in human readable code :D

[Saadnajmi]

Note: I think I have to limit this change to simulator only

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,755,253	-204,801
android	hermes	armeabi-v7a	8,067,826	-153,411
android	hermes	x86	9,247,854	-225,856
android	hermes	x86_64	9,096,987	-219,191
android	jsc	arm64-v8a	9,316,459	-204,522
android	jsc	armeabi-v7a	8,506,360	-153,152
android	jsc	x86	9,379,949	-225,579
android	jsc	x86_64	9,633,189	-218,906

Base commit: 5f7c5a88a1022c0307579d6d96899ae4819bc2ed Branch: main

[robhogan]

Hi @Saadnajmi - thanks for this. The wiring looks useful (and thanks for modernising that query string construction!). We do have some concerns about inlining source maps by default, though (even if only on simulator + JSC), mainly around source map generation speed and the devx impact of blocking bundling on a process fundamentally not designed for performance.

We have a project ongoing at the moment to essentially rethink RN debugging - CC @blakef and @huntie about where Safari might fit in or what alternatives we have.

[Saadnajmi]

Hi @Saadnajmi - thanks for this. The wiring looks useful (and thanks for modernising that query string construction!). We do have some concerns about inlining source maps by default, though (even if only on simulator + JSC), mainly around source map generation speed and the devx impact of blocking bundling on a process fundamentally not designed for performance.

We have a project ongoing at the moment to essentially rethink RN debugging - CC @blakef and @huntie about where Safari might fit in or what alternatives we have.

I see, that makes sense. I hope this and my other PR can help with some of the "current state of JSC direct debugging" discussions =). If I removed the setting of it as default but kept the wiring, would that be alright? That way, users can still enable it manually? I mostly wanted to avoid the mental string concatenation the link I referenced did.

[robhogan]

If I removed the setting of it as default but kept the wiring, would that be alright? That way, users can still enable it manually?

Yeah absolutely - making inlining an easier + more robust userland opt-in sounds great. I'll pass this on to iOS folks for reviewing the detail, but everything short of changing the default LGTM.

[blakef]

Just to add a bit more colour to this, it looks like direct debugging doesn't support sourcemap urls. I wonder how difficult it would be to add support for the basic case of fetching sourcemap urls?

[Saadnajmi]

Thanks for doing all the changes. I left some answers and a couple of nits, up to you to decide whether to implement them or not (they are really a matter of personal preference! ;) )

Fixed some of the nits, good to go on my end!

[cipolleschi]

mmmh... there are three tests that consistently fails, and they seems related to your changes:

- [RCTBundleURLProviderTests testBundleURL]
- [RCTBundleURLProviderTests testIPURL]
- [RCTBundleURLProviderTests testLocalhostURL]

Could you have a look at those? 🙏

[cipolleschi]

@robhogan from the iOS perspective, it looks good now. Do you want to import it yourself or should I proceed?

[facebook-github-bot]

@huntie has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@huntie has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[github-actions[bot]]

This pull request was successfully merged by @Saadnajmi in f7219ec02d71d2f0f6c71af4d5c3d4850a898fd8.

^{When will my fix make it into a release? | Upcoming Releases}