[0.69] Use Content-Location header in bundle response as JS source URL (#37501)
#38179
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…7501) Summary: Pull Request resolved: #37501 This is the iOS side of the fix for #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: 170fcd63a098f81bdcba55ebde0cf3569dceb88d
facebook-github-bot
added
CLA Signed
williscool
added a commit
to williscool/CalendarNotification
that referenced
this pull request
Oct 11, 2023
MAN THAT WAS HARD!
had to
- downgrade react-native to 0.71
- upgrade kotlin to 1.8
- downgrade gradle to 7.6
- change the location of @react-native/gradle-plugin to
react-native-gradle-plugin
- setup the expo config to use the hermes compiler (not mention in doc
:/ )
facebook/react-native#38179
expo/expo#22008
williscool
added a commit
to williscool/CalendarNotification
that referenced
this pull request
Oct 14, 2023
* bring over stuff that looks good from standard native modules attempt #10 * expo config setup init MAN THAT WAS HARD! had to - downgrade react-native to 0.71 - upgrade kotlin to 1.8 - downgrade gradle to 7.6 - change the location of @react-native/gradle-plugin to react-native-gradle-plugin - setup the expo config to use the hermes compiler (not mention in doc :/ ) facebook/react-native#38179 expo/expo#22008 * missed a spot * got everything running with no errors ran into this ugly one called java.lang.IllegalStateException: Current Activity is of incorrect class, expected AppCompatActivity, received ui.MyReactActivity hard to debug for real for real. ended up looking into expo docs that said they are getting rid of the bare instructions and then expo/expo#18022 then that had me be like where is that erorr even from... which lead me to git blame packages/expo-modules-core/android/src/main/java/expo/modules/kotlin/AppContext.kt which lead me to expo/expo#19573 which lead me to s/Activity/AppCompatActivity/g in android/app/src/main/java/com/github/quarck/calnotify/ui/MyReactActivity.kt * fix: forgot to switch to matching gradle version for android build plugin * def don't need react native picker hope we don't need expo install modules either * kinda crazy but got a (mostly) WORKING NATIVE MODULE! in modules/my-module/android/src/main/java/expo/modules/mymodule/MyModule.kt Constants, Events, and AsyncFunction work beautifully! View seems to work but I don't really understand it weirdly Function DOES NOT WORK AT ALL!!! it just says is not a function and since it seemed like the easiest thing and was what I looked at first it made me thing everything else was broken too :/ lost A LOT of time to that * Logcat instead of println and hello on button press * document why chrome debugging doesn't work as expected by default to save myself time in the future * debugging with vscode works!!!! unfortunately debugging syncronous native modules in it does not :/ * more docs on debugging * react-native-devsettings because why not * flipper setup again because why not? * drop a android studio thing that didn't work * fix readme * drop a dependency we dont need that had a peer dependency that broke shit expo-module-scripts 3.1.0 depends on @expo/config 7.5 which breaks shit * Revert "flipper setup again because why not?" turns out it breaks build. like flipper but not worth build break This reverts commit 8612ff3. * do still want to set entryFile though
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a pick of #37501 into 0.69-stable
Summary:
Pull Request resolved: #37501
This is the iOS side of the fix for #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 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-Locationheader, 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-Locationheader in bundle response as JS source URLReviewed By: huntie
Differential Revision: D45950661
fbshipit-source-id: 170fcd63a098f81bdcba55ebde0cf3569dceb88d