RFC: JavaScriptCore-safe bundle URLs #646
Open
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
tido64
reviewed
May 9, 2023
| - Maintenance/complexity of supporting an additional URL format. | ||
| - Forking of bundle URL generation in React Native by platform/engine, assuming we use this new format on as narrow a range of target engines as possible. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
Not entirely familiar with all the bits involved, but I have a couple of questions:
- What prevents Safari from also stripping out
;&for security reasons? - Have you considered embedding a comment similar to
//# sourceMappingURL=for source maps?
There was a problem hiding this comment.
- What prevents Safari from also stripping out ;& for security reasons?
Nothing in theory, but my understanding is that query strings (which of course aren't fundamentally less secure than path components) are commonly used to append tracking or other sensitive information in a web context during redirection. From what I can gather (though the security bugzilla is private), the JSC change was to plug quite a specific hole where stack consumers could end up being sent the sensitive data added by a separate redirection service. Those services would have to motivation to make that private data leakable, and couldn't append ;& in the general case anyway because it will usually break routing.
I've also been in touch with Apple folks about building an opt-out for controlled, non-browser use cases like RN - they're open to that because the disruption to our use case was completely unintentional, so I don't think we're getting into an arms race here - just a workaround for a temporary situation.
- Have you considered embedding a comment similar to //# sourceMappingURL= for source maps?
Metro already adds these, but JavaScriptCore doesn't use them when populating error stacks. The stacks coming from the engine only use the sourceURL provided explicitly to the JSEvaluateScript call (docs).
robhogan
commented
May 15, 2023
|
|
||
| Use a character sequence from the "reserved" set specified in RFC 3986, namely `;&`, as an alternative delimiter in place of `?`, and percent-encode any `?` characters appearing within the subsequent query string, so that the result is a valid [`path`](https://www.rfc-editor.org/rfc/rfc3986#section-3.3). | ||
|
|
||
| If the client sends a URL in this format, Metro should respond as if `?` was seen in place of the alternative delimiter, with the following exceptions: |
There was a problem hiding this comment.
@motiz88 I think we need to specify what "in this format" means here. For example, what format is this?
`/path/with-;&-unencoded/index.bundle?platform=ios`
If we interpret it as a "URL of mixed format" as suggested below, then we'll break the specific edge case we claim to support, because clients are not presently required to percent-encode this sequence within a bundle request path (and in fact, don't). In practice, we could ensure that React Native escapes that sequence in future, but it'd still technically be a semver-breaking change to Metro, albeit in a narrow edge case.
It might be more practical to consider an input to be "in this format" if it contains both a ;& within the path, and no query component.
Another possibility is to use a character sequence that can't appear in (normalised) POSIX paths - there aren't many, but maybe //?
|
I've just made a substantial update to the RFC in a961392, in summary:
|
robhogan
added a commit
to robhogan/react-native
that referenced
this pull request
May 20, 2023
Summary: 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][Fixed] Use `Content-Location` header in bundle response as JS source URL Differential Revision: D45950661 fbshipit-source-id: 0c4f74c5cac4e2e0bf0e251614f55bd58685619d
robhogan
added a commit
to robhogan/metro
that referenced
this pull request
May 20, 2023
Summary: The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Differential Revision: D45983876 fbshipit-source-id: cbf749bae0256bc981588fa2b9e7b0d60eba7461
robhogan
added a commit
to robhogan/react-native
that referenced
this pull request
May 23, 2023
…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: a67fefadfda2d79009e1ce576e394661c1466601
robhogan
added a commit
to robhogan/react-native
that referenced
this pull request
May 23, 2023
…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: 35682369b77303674028291c86ac386f4bd12ce8
robhogan
added a commit
to robhogan/react-native
that referenced
this pull request
May 23, 2023
…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
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
May 23, 2023
…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
robhogan
added a commit
to robhogan/metro
that referenced
this pull request
May 24, 2023
…ook#989) Summary: Pull Request resolved: facebook#989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: b13c712521f7a3b6716c14d392cb3a057ede5936
facebook-github-bot
pushed a commit
to facebook/metro
that referenced
this pull request
May 24, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
facebook-github-bot
pushed a commit
to facebook/metro
that referenced
this pull request
May 24, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
kelset
pushed a commit
to facebook/react-native
that referenced
this pull request
May 25, 2023
…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
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 3, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
… place of `?`) Summary: The first part of implementing react-native-community/discussions-and-proposals#646 to address facebook/react-native#36794. This allows Metro to respond to bundle and symbolication requests that use URLs with `//&` in place of `?` as a query delimiter. ``` **[Feature]**: Support URLs for both bundling and symbolication requests using `//&` instead of `?` as a query string delimiter ``` (Note: This does *not* add support for registering HMR entry points in the JSC-safe format - that's not necessary at this point, if at all, and I'm keen to minimise the footprint of this stack for easier backporting.) Reviewed By: huntie Differential Revision: D45983877 fbshipit-source-id: e799f76cd26c2ca8026b4d1bf70a582814ae1790
robhogan
added a commit
to facebook/metro
that referenced
this pull request
Jun 5, 2023
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f
kelset
pushed a commit
to facebook/react-native
that referenced
this pull request
Jun 14, 2023
…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
douglowder
pushed a commit
to react-native-tvos/react-native-tvos
that referenced
this pull request
Jun 19, 2023
…7501) Summary: Pull Request resolved: facebook/react-native#37501 This is the iOS side of the fix for facebook/react-native#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
robhogan
added a commit
to facebook/react-native
that referenced
this pull request
Jul 4, 2023
…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
kelset
pushed a commit
to facebook/react-native
that referenced
this pull request
Jul 5, 2023
…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
Saadnajmi
added a commit
to microsoft/react-native-macos
that referenced
this pull request
Jul 6, 2023
* [LOCAL] update podlock post release to fix CI * Fix measurement of uncontrolled TextInput after edit Summary: D42721684 (facebook@be69c8b) left a pretty bad bug when using Fabric for Android. I missed that in Fabric specifically, on edit we will cache the Spannable backing the EditText for use in future measurement. Because we've stripped the sizing spans, Spannable measurement has incorrect font size, and the TextInput size will change (collapsing) after the first edit. This effectively breaks any uncontrolled TextInput which does not have explicit dimensions set. Changelog: [Android][Fixed] - Fix measurement of uncontrolled TextInput after edit Reviewed By: sammy-SC Differential Revision: D43158407 fbshipit-source-id: 51602eab06c9a50e2b60ef0ed87bdb4df025e51e * Minimize EditText Spans 1/9: Fix precedence (facebook#36543) Summary: Pull Request resolved: facebook#36543 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. We cache the backing EditText span on text change to later measure. To measure outside of a TextInput we need to restore any spans we removed. Spans may overlap, so base attributes should be behind everything else. The logic here for dealing with precedence is incorrect, and we should instead accomplish this by twiddling with the `SPAN_PRIORITY` bits. Changelog: [Android][Fixed] - Minimize Spans 1/N: Fix precedence Reviewed By: javache Differential Revision: D44240779 fbshipit-source-id: f731b353587888faad946b8cf1e868095cdeced3 * Minimize EditText Spans 2/9: Make stripAttributeEquivalentSpans generic (facebook#36546) Summary: Pull Request resolved: facebook#36546 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change generalizes `stripAttributeEquivalentSpans()` to allow plugging in different spans. Changelog: [Internal] Reviewed By: rshest Differential Revision: D44240781 fbshipit-source-id: 89005266020f216368e9ad9ce382699bd8db85a8 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java * Minimize EditText Spans 3/9: ReactBackgroundColorSpan (facebook#36547) Summary: Pull Request resolved: facebook#36547 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This adds `ReactBackgroundColorSpan` to the list of spans eligible to be stripped. Changelog: [Android][Fixed] - Minimize Spans 3/N: ReactBackgroundColorSpan Reviewed By: javache Differential Revision: D44240782 fbshipit-source-id: 2ded1a1687a41cf6d5f83e89ffadd2d932089969 * Minimize EditText Spans 4/9: ReactForegroundColorSpan (facebook#36545) Summary: Pull Request resolved: facebook#36545 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This adds ReactForegroundColorSpan to the list of spans eligible to be stripped. Changelog: [Android][Fixed] - Minimize Spans 4/N: ReactForegroundColorSpan Reviewed By: javache Differential Revision: D44240780 fbshipit-source-id: d86939cc2d7ed9116a4167026c7d48928fc51757 * Minimize EditText Spans 5/9: Strikethrough and Underline (facebook#36544) Summary: Pull Request resolved: facebook#36544 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change makes us apply strikethrough and underline as paint flags to the underlying EditText, instead of just the spans. We then opt ReactUnderlineSpan and ReactStrikethroughSpan into being strippable. This does actually create visual behavior changes, where child text will inherit any underline or strikethrough of the root EditText (including if the child specifies `textDecorationLine: "none"`. The new behavior is consistent with both iOS and web though, so it seems like more of a bugfix than a regression. Changelog: [Android][Fixed] - Minimize Spans 5/N: Strikethrough and Underline Reviewed By: rshest Differential Revision: D44240778 fbshipit-source-id: d564dfc0121057a5e3b09bb71b8f5662e28be17e * Minimize EditText Spans 6/9: letterSpacing (facebook#36548) Summary: Pull Request resolved: facebook#36548 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change lets us set `letterSpacing` on the EditText instead of using our custom span. Changelog: [Android][Fixed] - Minimize EditText Spans 6/N: letterSpacing Reviewed By: rshest Differential Revision: D44240777 fbshipit-source-id: 9bd10c3261257037d8cacf37971011aaa94d1a77 * Minimize EditText Spans 7/9: Avoid temp list (facebook#36576) Summary: Pull Request resolved: facebook#36576 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change addresses some minor CR feedback and removes the temporary list of spans in favor of applying them directly. Changelog: [Internal] Reviewed By: javache Differential Revision: D44295190 fbshipit-source-id: bd784e2c514301d45d0bacd8ee6de5c512fc565c * Minimize EditText Spans 8/9: CustomStyleSpan (facebook#36577) Summary: Pull Request resolved: facebook#36577 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change allows us to strip CustomStyleSpan. We already set all but `fontVariant` on the underlying EditText, so we just need to route that through as well. Note that because this span is non-parcelable, it is seemingly not subject to the buggy behavior on Samsung devices of infinitely cloning the spans, but non-parcelable spans have different issues on the devices (they disappear), so moving `fontVariant` to the top-level makes sense here. Changelog: [Android][Fixed] - Minimize EditText Spans 8/N: CustomStyleSpan Reviewed By: javache Differential Revision: D44297384 fbshipit-source-id: ed4c000e961dd456a2a8f4397e27c23a87defb6e * Mimimize EditText Spans 9/9: Remove addSpansForMeasurement() (facebook#36575) Summary: Pull Request resolved: facebook#36575 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. D23670779 addedd a previous mechanism to add spans for measurement caching, like we needed to do as part of this change. It is called in more specific cases (e.g. when there is a text hint but no text), but it edits the live EditText spannable instead of the cache copy, and does not handle nested text at all. We are already adding spans back to the input after this, behind everything else, and can replace it with the code we have been adding. Changelog: [Android][Fixed] - Mimimize EditText Spans 9/9: Remove `addSpansForMeasurement()` Reviewed By: javache Differential Revision: D44298159 fbshipit-source-id: 1af44a39de7550b7e66e45db9ebc3523ae9ff002 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java * Use RN Build Utils for Building Hermes Artifacts Summary: We moved Hermes some build utils from [Hermes repo](https://github.com/facebook/hermes/tree/main/utils) to [React Navtie repo](https://github.com/facebook/react-native/tree/main/sdks/hermes-engine/utils) a while ago to have more control over them. However some paths on the CI were not updated. We continued to use old build scripts for Hermes prebuilds. Some unfortunate side effects are: - `HERMES_ENABLE_DEBUGGER` is [hardcoded to true](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L65). That makes Hermes much slower in Release configuration. - BUILD_TYPE is [set to Release](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L10) instead of `MinSizeRel` which inreases Hermes binary size. This diff copies these build utils from RN to Hermes source directory before we perform Hermes build. Changelog [Internal] Reviewed By: cipolleschi Differential Revision: D44066721 fbshipit-source-id: f45ad6a31fb01c10199f69cc7bbcbbc83b793d34 * Fix TextView alignment being reset on state updates Summary: Changelog: [Android][Fixed] Resolved bug with Text components in new arch losing text alignment state. Reviewed By: mdvacca Differential Revision: D34108943 fbshipit-source-id: 3992e9406345be919b5e3595fc1f9e61cf67a699 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java * [0.71.7] Bump version numbers * Do not send extra onChangeText even wnen instantianting multiline TextView (facebook#36930) Summary: Pull Request resolved: facebook#36930 This diff fixes facebook#36494 Now this code matches its [Fabric counterpart](facebook@7b48899). There is also one extra check that `_lastStringStateWasUpdatedWith != nil`. With that in place we don't send extra `onChangeText` event when `attributedText` is assigned for the first time on TextView construction. Changelog: [IOS][Fixed] - Do not send extra onChangeText even wnen instantianting multiline TextView Reviewed By: sammy-SC Differential Revision: D45049135 fbshipit-source-id: 62fa281308b9d2e0a807d024f08d8a214bd99b5e * Read Maven group from GROUP property (facebook#37204) Summary: The [React Native TV repo](https://github.com/react-native-tvos/react-native-tvos) shares most of the same Android code as the core repo. Beginning in 0.71, it needs to also publish Android Maven artifacts for the `react-android` and `hermes-android` libraries. In order to avoid conflicts, it needs to publish the artifacts to a different group name. However, `react-native-gradle-plugin` uses a hardcoded group name (`com.facebook.react`). Solution: read the group name from the existing `GROUP` property in `ReactAndroid/gradle.properties`. ## Changelog: [Android] [Fixed] - read GROUP name in gradle-plugin dependency code Pull Request resolved: facebook#37204 Test Plan: - Android unit tests have been added for the new code and new method in `DependencyUtils.kt`. - Existing tests should pass - The new code defaults to the correct group (`com.facebook.react`) so no functional change is expected in the core repo. Reviewed By: luluwu2032 Differential Revision: D45576700 Pulled By: cortinico fbshipit-source-id: 6297ab515b4bdbb17024989c7d3035b0a2ded0ae # Conflicts: # packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactPlugin.kt * bumped packages versions #publish-packages-to-npm * [LOCAL] bump RNGP to 0.71.18 * [LOCAL] update podlock * [0.71.8] Bump version numbers * fix(types): cross platform autoComplete for TextInput (facebook#36931) Summary: Since v0.71 the autoComplete prop on TextInput is available on iOS ([release notes](https://reactnative.dev/blog/2023/01/12/version-071#component-specific-behavior)). However, this change is not reflected in the types. Original types PR here - DefinitelyTyped/DefinitelyTyped#65144 by chwallen ## Changelog: [GENERAL] [FIXED] - Fix autoComplete type for TextInput Pull Request resolved: facebook#36931 Test Plan: Setting the autoComplete prop on TextInput to `nickname`, `organization`, `organization-title`, or `url` should not result in typescript errors. Reviewed By: NickGerleman Differential Revision: D45052350 Pulled By: javache fbshipit-source-id: 40993833b4ed14f91e3bf3521a264ea93517a0c9 * Allow string `transform` style in TypeScript (facebook#37569) Summary: Pull Request resolved: facebook#37569 Fixes facebook#37543 Missed as part of D39423409 (or maybe we didn't have TS types inline yet?) Changelog: [General][Fixed] - Allow string `transform` style in TypeScript Reviewed By: cortinico Differential Revision: D46161450 fbshipit-source-id: 24ee9e19365b7209ec0a2c8fb5a5d7ac78203f4d * fix: [gradle-plugin] 3rd party lib dependency substitution (facebook#37445) Summary: For 3rd party libraries to work with a React Native fork (such as the TV repo) that uses a different Maven group for `react-android` and `hermes-android` artifacts, an additional dependency substitution is required. ## Changelog: [Android][fixed] RNGP dependency substitutions for fork with different Maven group Pull Request resolved: facebook#37445 Test Plan: - Manual tested with an existing project - Unit tests pass Reviewed By: rshest, dmytrorykun Differential Revision: D45948901 Pulled By: cortinico fbshipit-source-id: 4151a1d3616172a92c68812c3a0034c98b330d67 * fix: fix virtualizedList scrollToEnd for 0 items (facebook#36067) Summary: Fixes facebook#36066 ## Changelog [GENERAL] [FIXED] - VirtualizedList scrollToEnd with no data Pull Request resolved: facebook#36067 Test Plan: Run `yarn test VirtualizedList-test` Reviewed By: jacdebug Differential Revision: D43041763 Pulled By: javache fbshipit-source-id: d4d5e871284708a89bf9911d82e9aa97d7625aca * [LOCAL] bump hermes version post release * Make CircleCI caches for hermesc be version dependent (facebook#37452) Summary: Pull Request resolved: facebook#37452 Fixes facebook#37428 We do have cache poisoning for hermesc on Windows and Linux due to reusing the same cache key among different React Native version. This fixes it by specifying a cache key which is version dependent + it invalidates the caches by defining a new key. Changelog: [Internal] [Fixed] - Make CircleCI caches for hermesc be version dependent Reviewed By: cortinico Differential Revision: D45909178 fbshipit-source-id: 830c87ae45739c7053342a68dac2ee7581945c1d * [0.71.9] Bump version numbers * bumped packages versions #publish-packages-to-npm * bump RNGP to 0.71.19 * [0.71.10] Bump version numbers * [LOCAL] update podlock to fix CI * Revert "fix: border width top/bottom not matching the border radius (facebook#34362)" This reverts commit cd6a913. * Fix invalid context cast in ReactRootView (facebook#36121) Summary: Hi 👋 I'm one of the Expo contributors. When upgrading the Stripe dependency in Expo for SDK48 release on Android, we noticed the following error: ```java.lang.ClassCastException: android.view.ContextThemeWrapper cannot be cast to android.app.Activity``` With Kudo we narrowed it down to an underlying issue in the following cast in `ReactRootView` done during checking for keyboard events: ```((Activity) getContext())``` The `getContext()` is actually a `ContextThemeWrapper` for `ExperienceActivity`, so we should not cast it to an `Activity` directly - instead, we unwrap it using `getBaseContext()`. Implementing the following fix into the Expo fork fixed the crash for us: expo@0e2c9ca ## Changelog [ANDROID] [FIXED] - Fixed crash occurring in certain native views when handling keyboard events. <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> Pull Request resolved: facebook#36121 Test Plan: Tested manually by comparing two builds before and after this change. The main branch build crashes when the [Stripe 0.23.1](https://github.com/stripe/stripe-react-native) Card Element from the example app is mounted on screen. Applying the change fixed the issue. Happy to make a more isolated reproduction if necessary. --- Full stack trace: ``` AndroidRuntime D Shutting down VM E FATAL EXCEPTION: main E Process: host.exp.exponent, PID: 8849 E java.lang.ClassCastException: android.view.ContextThemeWrapper cannot be cast to android.app.Activity E at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:937) E at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:913) E at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061) E at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3352) E at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2286) E at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8948) E at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1231) E at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1239) E at android.view.Choreographer.doCallbacks(Choreographer.java:899) E at android.view.Choreographer.doFrame(Choreographer.java:832) E at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1214) E at android.os.Handler.handleCallback(Handler.java:942) E at android.os.Handler.dispatchMessage(Handler.java:99) E at android.os.Looper.loopOnce(Looper.java:201) E at android.os.Looper.loop(Looper.java:288) E at android.app.ActivityThread.main(ActivityThread.java:7898) E at java.lang.reflect.Method.invoke(Native Method) E at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) E at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936) ``` Reviewed By: cortinico Differential Revision: D43186530 Pulled By: javache fbshipit-source-id: 2143495f6b0c71f342eba6d5abb2bfa4a529fbdd * Prevent crash in runAnimationStep on OnePlus and Oppo devices (facebook#37487) Summary: We've been encountering a crash in `runAnimationStep` with "Calculated frame index should never be lower than 0" facebook#35766 with OnePlus/Oppo devices as well, but don't have one on hand to test. This just works around the issue: if the time is before the start time of an animation, we shouldn't do anything anyways, so we just log a message instead of throwing while in production. We still throw in debug mode though for easier debugging. ### Hypothesis of the root cause Based on stacktrace in facebook#35766 (which is the same one we see) Normally, this should happen 1. Choreographer.java constructs a FrameDisplayEventReceiver 2. FrameDisplayEventReceiver.onVSync gets called, which sets the `mTimestampNanos` 3. FrameDisplayEventReceiver.run gets called, which then eventually calls our `doFrame` callback with `mTimestampNanos`. This then causes `FrameBasedAnimationDriver.runAnimationStep` to be called with the same timestamp I suspect what's happening on OnePlus devices is that the `onVSync` call either doesn't happen or happens rarely enough that the `mTimestampNanos` when `run` is called is sometime in the past ### Fix 1. Add logging so we get the parameters to debug more if we end up getting this error 2. In production, just ignore past times instead of throwing an Error ## Changelog: Pick one each for the category and type tags: [ANDROID] [FIXED] - Prevent crash on OnePlus/Oppo devices in runAnimationStep Pull Request resolved: facebook#37487 Test Plan: Ran our app using patched version and verified no issues showed up when using it Reviewed By: cipolleschi Differential Revision: D46102968 Pulled By: cortinico fbshipit-source-id: bcb36a0c2aed0afdb8e7e68b141a3db4eb02695a * Use `Content-Location` header in bundle response as JS source URL (facebook#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: 170fcd63a098f81bdcba55ebde0cf3569dceb88d * [LOCAL] Make 0.70 compatible with Xcode 15 (thanks to @AlexanderEggers for the commit in main) * [LOCAL] Bump CLI to 10.2.4 and Metro to 0.73.10 * Add web compat unit tests and fixes (facebook#35316) Summary: Adds compat with W3C logical CSS properties. See facebook#34425 This is a replacement for reverted facebook#34590, which can no longer be imported internally. [General][Added] - Added CSS logical properties. Pull Request resolved: facebook#35316 Test Plan: Unit test snapshots. Reviewed By: NickGerleman Differential Revision: D41230978 Pulled By: necolas fbshipit-source-id: 40e93d0d697f0cb28390480ce2b4bcbce18da70a * resolve some merge conflicts * fix tests * [0.71.11] Bump version numbers * [LOCAL] update podlock for CI * fix(ios): fix `pod install --project-directory=ios` failing (facebook#37993) * Prevent LogBox from crashing on long messages (facebook#38005) Summary: Pull Request resolved: facebook#38005 Fixes facebook#32093 by guarding the expensive `BABEL_CODE_FRAME_ERROR_FORMAT` regex with a cheaper initial scan. (Longer term, we should reduce our reliance on string parsing and propagate more structured errors.) Changelog: [General][Fixed] Prevent LogBox from crashing on very long messages Reviewed By: GijsWeterings Differential Revision: D46892454 fbshipit-source-id: 3afadcdd75969c2589bbb06f47d1c4c1c2690abd # Conflicts: # Libraries/LogBox/Data/parseLogBoxLog.js # packages/react-native/package.json * [0.71.12] Bump version numbers --------- Co-authored-by: Lorenzo Sciandra <[email protected]> Co-authored-by: Nick Gerleman <[email protected]> Co-authored-by: Lorenzo Sciandra <[email protected]> Co-authored-by: Dmitry Rykun <[email protected]> Co-authored-by: Pieter De Baets <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Douglas Lowder <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Kyle Roach <[email protected]> Co-authored-by: Julien Brayere <[email protected]> Co-authored-by: Riccardo Cipolleschi <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: aleqsio <[email protected]> Co-authored-by: Harry Yu <[email protected]> Co-authored-by: Rob Hogan <[email protected]> Co-authored-by: Nicolas Gallagher <[email protected]> Co-authored-by: Alexander Eggers <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Tommy Nguyen <[email protected]> Co-authored-by: Moti Zilberman <[email protected]> Co-authored-by: Distiller <[email protected]>
Saadnajmi
added a commit
to microsoft/react-native-macos
that referenced
this pull request
Aug 23, 2023
* [LOCAL] update podlock post release to fix CI * Fix measurement of uncontrolled TextInput after edit Summary: D42721684 (facebook@be69c8b) left a pretty bad bug when using Fabric for Android. I missed that in Fabric specifically, on edit we will cache the Spannable backing the EditText for use in future measurement. Because we've stripped the sizing spans, Spannable measurement has incorrect font size, and the TextInput size will change (collapsing) after the first edit. This effectively breaks any uncontrolled TextInput which does not have explicit dimensions set. Changelog: [Android][Fixed] - Fix measurement of uncontrolled TextInput after edit Reviewed By: sammy-SC Differential Revision: D43158407 fbshipit-source-id: 51602eab06c9a50e2b60ef0ed87bdb4df025e51e * Minimize EditText Spans 1/9: Fix precedence (facebook#36543) Summary: Pull Request resolved: facebook#36543 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. We cache the backing EditText span on text change to later measure. To measure outside of a TextInput we need to restore any spans we removed. Spans may overlap, so base attributes should be behind everything else. The logic here for dealing with precedence is incorrect, and we should instead accomplish this by twiddling with the `SPAN_PRIORITY` bits. Changelog: [Android][Fixed] - Minimize Spans 1/N: Fix precedence Reviewed By: javache Differential Revision: D44240779 fbshipit-source-id: f731b353587888faad946b8cf1e868095cdeced3 * Minimize EditText Spans 2/9: Make stripAttributeEquivalentSpans generic (facebook#36546) Summary: Pull Request resolved: facebook#36546 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change generalizes `stripAttributeEquivalentSpans()` to allow plugging in different spans. Changelog: [Internal] Reviewed By: rshest Differential Revision: D44240781 fbshipit-source-id: 89005266020f216368e9ad9ce382699bd8db85a8 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java * Minimize EditText Spans 3/9: ReactBackgroundColorSpan (facebook#36547) Summary: Pull Request resolved: facebook#36547 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This adds `ReactBackgroundColorSpan` to the list of spans eligible to be stripped. Changelog: [Android][Fixed] - Minimize Spans 3/N: ReactBackgroundColorSpan Reviewed By: javache Differential Revision: D44240782 fbshipit-source-id: 2ded1a1687a41cf6d5f83e89ffadd2d932089969 * Minimize EditText Spans 4/9: ReactForegroundColorSpan (facebook#36545) Summary: Pull Request resolved: facebook#36545 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This adds ReactForegroundColorSpan to the list of spans eligible to be stripped. Changelog: [Android][Fixed] - Minimize Spans 4/N: ReactForegroundColorSpan Reviewed By: javache Differential Revision: D44240780 fbshipit-source-id: d86939cc2d7ed9116a4167026c7d48928fc51757 * Minimize EditText Spans 5/9: Strikethrough and Underline (facebook#36544) Summary: Pull Request resolved: facebook#36544 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change makes us apply strikethrough and underline as paint flags to the underlying EditText, instead of just the spans. We then opt ReactUnderlineSpan and ReactStrikethroughSpan into being strippable. This does actually create visual behavior changes, where child text will inherit any underline or strikethrough of the root EditText (including if the child specifies `textDecorationLine: "none"`. The new behavior is consistent with both iOS and web though, so it seems like more of a bugfix than a regression. Changelog: [Android][Fixed] - Minimize Spans 5/N: Strikethrough and Underline Reviewed By: rshest Differential Revision: D44240778 fbshipit-source-id: d564dfc0121057a5e3b09bb71b8f5662e28be17e * Minimize EditText Spans 6/9: letterSpacing (facebook#36548) Summary: Pull Request resolved: facebook#36548 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change lets us set `letterSpacing` on the EditText instead of using our custom span. Changelog: [Android][Fixed] - Minimize EditText Spans 6/N: letterSpacing Reviewed By: rshest Differential Revision: D44240777 fbshipit-source-id: 9bd10c3261257037d8cacf37971011aaa94d1a77 * Minimize EditText Spans 7/9: Avoid temp list (facebook#36576) Summary: Pull Request resolved: facebook#36576 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change addresses some minor CR feedback and removes the temporary list of spans in favor of applying them directly. Changelog: [Internal] Reviewed By: javache Differential Revision: D44295190 fbshipit-source-id: bd784e2c514301d45d0bacd8ee6de5c512fc565c * Minimize EditText Spans 8/9: CustomStyleSpan (facebook#36577) Summary: Pull Request resolved: facebook#36577 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. This change allows us to strip CustomStyleSpan. We already set all but `fontVariant` on the underlying EditText, so we just need to route that through as well. Note that because this span is non-parcelable, it is seemingly not subject to the buggy behavior on Samsung devices of infinitely cloning the spans, but non-parcelable spans have different issues on the devices (they disappear), so moving `fontVariant` to the top-level makes sense here. Changelog: [Android][Fixed] - Minimize EditText Spans 8/N: CustomStyleSpan Reviewed By: javache Differential Revision: D44297384 fbshipit-source-id: ed4c000e961dd456a2a8f4397e27c23a87defb6e * Mimimize EditText Spans 9/9: Remove addSpansForMeasurement() (facebook#36575) Summary: Pull Request resolved: facebook#36575 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior. D23670779 addedd a previous mechanism to add spans for measurement caching, like we needed to do as part of this change. It is called in more specific cases (e.g. when there is a text hint but no text), but it edits the live EditText spannable instead of the cache copy, and does not handle nested text at all. We are already adding spans back to the input after this, behind everything else, and can replace it with the code we have been adding. Changelog: [Android][Fixed] - Mimimize EditText Spans 9/9: Remove `addSpansForMeasurement()` Reviewed By: javache Differential Revision: D44298159 fbshipit-source-id: 1af44a39de7550b7e66e45db9ebc3523ae9ff002 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java * Use RN Build Utils for Building Hermes Artifacts Summary: We moved Hermes some build utils from [Hermes repo](https://github.com/facebook/hermes/tree/main/utils) to [React Navtie repo](https://github.com/facebook/react-native/tree/main/sdks/hermes-engine/utils) a while ago to have more control over them. However some paths on the CI were not updated. We continued to use old build scripts for Hermes prebuilds. Some unfortunate side effects are: - `HERMES_ENABLE_DEBUGGER` is [hardcoded to true](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L65). That makes Hermes much slower in Release configuration. - BUILD_TYPE is [set to Release](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L10) instead of `MinSizeRel` which inreases Hermes binary size. This diff copies these build utils from RN to Hermes source directory before we perform Hermes build. Changelog [Internal] Reviewed By: cipolleschi Differential Revision: D44066721 fbshipit-source-id: f45ad6a31fb01c10199f69cc7bbcbbc83b793d34 * Fix TextView alignment being reset on state updates Summary: Changelog: [Android][Fixed] Resolved bug with Text components in new arch losing text alignment state. Reviewed By: mdvacca Differential Revision: D34108943 fbshipit-source-id: 3992e9406345be919b5e3595fc1f9e61cf67a699 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java * [0.71.7] Bump version numbers * Do not send extra onChangeText even wnen instantianting multiline TextView (facebook#36930) Summary: Pull Request resolved: facebook#36930 This diff fixes facebook#36494 Now this code matches its [Fabric counterpart](facebook@7b48899). There is also one extra check that `_lastStringStateWasUpdatedWith != nil`. With that in place we don't send extra `onChangeText` event when `attributedText` is assigned for the first time on TextView construction. Changelog: [IOS][Fixed] - Do not send extra onChangeText even wnen instantianting multiline TextView Reviewed By: sammy-SC Differential Revision: D45049135 fbshipit-source-id: 62fa281308b9d2e0a807d024f08d8a214bd99b5e * Read Maven group from GROUP property (facebook#37204) Summary: The [React Native TV repo](https://github.com/react-native-tvos/react-native-tvos) shares most of the same Android code as the core repo. Beginning in 0.71, it needs to also publish Android Maven artifacts for the `react-android` and `hermes-android` libraries. In order to avoid conflicts, it needs to publish the artifacts to a different group name. However, `react-native-gradle-plugin` uses a hardcoded group name (`com.facebook.react`). Solution: read the group name from the existing `GROUP` property in `ReactAndroid/gradle.properties`. ## Changelog: [Android] [Fixed] - read GROUP name in gradle-plugin dependency code Pull Request resolved: facebook#37204 Test Plan: - Android unit tests have been added for the new code and new method in `DependencyUtils.kt`. - Existing tests should pass - The new code defaults to the correct group (`com.facebook.react`) so no functional change is expected in the core repo. Reviewed By: luluwu2032 Differential Revision: D45576700 Pulled By: cortinico fbshipit-source-id: 6297ab515b4bdbb17024989c7d3035b0a2ded0ae # Conflicts: # packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactPlugin.kt * bumped packages versions #publish-packages-to-npm * [LOCAL] bump RNGP to 0.71.18 * [LOCAL] update podlock * [0.71.8] Bump version numbers * fix(types): cross platform autoComplete for TextInput (facebook#36931) Summary: Since v0.71 the autoComplete prop on TextInput is available on iOS ([release notes](https://reactnative.dev/blog/2023/01/12/version-071#component-specific-behavior)). However, this change is not reflected in the types. Original types PR here - DefinitelyTyped/DefinitelyTyped#65144 by chwallen ## Changelog: [GENERAL] [FIXED] - Fix autoComplete type for TextInput Pull Request resolved: facebook#36931 Test Plan: Setting the autoComplete prop on TextInput to `nickname`, `organization`, `organization-title`, or `url` should not result in typescript errors. Reviewed By: NickGerleman Differential Revision: D45052350 Pulled By: javache fbshipit-source-id: 40993833b4ed14f91e3bf3521a264ea93517a0c9 * Allow string `transform` style in TypeScript (facebook#37569) Summary: Pull Request resolved: facebook#37569 Fixes facebook#37543 Missed as part of D39423409 (or maybe we didn't have TS types inline yet?) Changelog: [General][Fixed] - Allow string `transform` style in TypeScript Reviewed By: cortinico Differential Revision: D46161450 fbshipit-source-id: 24ee9e19365b7209ec0a2c8fb5a5d7ac78203f4d * fix: [gradle-plugin] 3rd party lib dependency substitution (facebook#37445) Summary: For 3rd party libraries to work with a React Native fork (such as the TV repo) that uses a different Maven group for `react-android` and `hermes-android` artifacts, an additional dependency substitution is required. ## Changelog: [Android][fixed] RNGP dependency substitutions for fork with different Maven group Pull Request resolved: facebook#37445 Test Plan: - Manual tested with an existing project - Unit tests pass Reviewed By: rshest, dmytrorykun Differential Revision: D45948901 Pulled By: cortinico fbshipit-source-id: 4151a1d3616172a92c68812c3a0034c98b330d67 * fix: fix virtualizedList scrollToEnd for 0 items (facebook#36067) Summary: Fixes facebook#36066 ## Changelog [GENERAL] [FIXED] - VirtualizedList scrollToEnd with no data Pull Request resolved: facebook#36067 Test Plan: Run `yarn test VirtualizedList-test` Reviewed By: jacdebug Differential Revision: D43041763 Pulled By: javache fbshipit-source-id: d4d5e871284708a89bf9911d82e9aa97d7625aca * [LOCAL] bump hermes version post release * Make CircleCI caches for hermesc be version dependent (facebook#37452) Summary: Pull Request resolved: facebook#37452 Fixes facebook#37428 We do have cache poisoning for hermesc on Windows and Linux due to reusing the same cache key among different React Native version. This fixes it by specifying a cache key which is version dependent + it invalidates the caches by defining a new key. Changelog: [Internal] [Fixed] - Make CircleCI caches for hermesc be version dependent Reviewed By: cortinico Differential Revision: D45909178 fbshipit-source-id: 830c87ae45739c7053342a68dac2ee7581945c1d * [0.71.9] Bump version numbers * bumped packages versions #publish-packages-to-npm * bump RNGP to 0.71.19 * [0.71.10] Bump version numbers * [LOCAL] update podlock to fix CI * Revert "fix: border width top/bottom not matching the border radius (facebook#34362)" This reverts commit cd6a913. * Fix invalid context cast in ReactRootView (facebook#36121) Summary: Hi 👋 I'm one of the Expo contributors. When upgrading the Stripe dependency in Expo for SDK48 release on Android, we noticed the following error: ```java.lang.ClassCastException: android.view.ContextThemeWrapper cannot be cast to android.app.Activity``` With Kudo we narrowed it down to an underlying issue in the following cast in `ReactRootView` done during checking for keyboard events: ```((Activity) getContext())``` The `getContext()` is actually a `ContextThemeWrapper` for `ExperienceActivity`, so we should not cast it to an `Activity` directly - instead, we unwrap it using `getBaseContext()`. Implementing the following fix into the Expo fork fixed the crash for us: expo@0e2c9ca ## Changelog [ANDROID] [FIXED] - Fixed crash occurring in certain native views when handling keyboard events. <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> Pull Request resolved: facebook#36121 Test Plan: Tested manually by comparing two builds before and after this change. The main branch build crashes when the [Stripe 0.23.1](https://github.com/stripe/stripe-react-native) Card Element from the example app is mounted on screen. Applying the change fixed the issue. Happy to make a more isolated reproduction if necessary. --- Full stack trace: ``` AndroidRuntime D Shutting down VM E FATAL EXCEPTION: main E Process: host.exp.exponent, PID: 8849 E java.lang.ClassCastException: android.view.ContextThemeWrapper cannot be cast to android.app.Activity E at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:937) E at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:913) E at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061) E at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3352) E at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2286) E at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8948) E at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1231) E at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1239) E at android.view.Choreographer.doCallbacks(Choreographer.java:899) E at android.view.Choreographer.doFrame(Choreographer.java:832) E at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1214) E at android.os.Handler.handleCallback(Handler.java:942) E at android.os.Handler.dispatchMessage(Handler.java:99) E at android.os.Looper.loopOnce(Looper.java:201) E at android.os.Looper.loop(Looper.java:288) E at android.app.ActivityThread.main(ActivityThread.java:7898) E at java.lang.reflect.Method.invoke(Native Method) E at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) E at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936) ``` Reviewed By: cortinico Differential Revision: D43186530 Pulled By: javache fbshipit-source-id: 2143495f6b0c71f342eba6d5abb2bfa4a529fbdd * Prevent crash in runAnimationStep on OnePlus and Oppo devices (facebook#37487) Summary: We've been encountering a crash in `runAnimationStep` with "Calculated frame index should never be lower than 0" facebook#35766 with OnePlus/Oppo devices as well, but don't have one on hand to test. This just works around the issue: if the time is before the start time of an animation, we shouldn't do anything anyways, so we just log a message instead of throwing while in production. We still throw in debug mode though for easier debugging. ### Hypothesis of the root cause Based on stacktrace in facebook#35766 (which is the same one we see) Normally, this should happen 1. Choreographer.java constructs a FrameDisplayEventReceiver 2. FrameDisplayEventReceiver.onVSync gets called, which sets the `mTimestampNanos` 3. FrameDisplayEventReceiver.run gets called, which then eventually calls our `doFrame` callback with `mTimestampNanos`. This then causes `FrameBasedAnimationDriver.runAnimationStep` to be called with the same timestamp I suspect what's happening on OnePlus devices is that the `onVSync` call either doesn't happen or happens rarely enough that the `mTimestampNanos` when `run` is called is sometime in the past ### Fix 1. Add logging so we get the parameters to debug more if we end up getting this error 2. In production, just ignore past times instead of throwing an Error ## Changelog: Pick one each for the category and type tags: [ANDROID] [FIXED] - Prevent crash on OnePlus/Oppo devices in runAnimationStep Pull Request resolved: facebook#37487 Test Plan: Ran our app using patched version and verified no issues showed up when using it Reviewed By: cipolleschi Differential Revision: D46102968 Pulled By: cortinico fbshipit-source-id: bcb36a0c2aed0afdb8e7e68b141a3db4eb02695a * Use `Content-Location` header in bundle response as JS source URL (facebook#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: 170fcd63a098f81bdcba55ebde0cf3569dceb88d * [LOCAL] Make 0.70 compatible with Xcode 15 (thanks to @AlexanderEggers for the commit in main) * [LOCAL] Bump CLI to 10.2.4 and Metro to 0.73.10 * Add web compat unit tests and fixes (facebook#35316) Summary: Adds compat with W3C logical CSS properties. See facebook#34425 This is a replacement for reverted facebook#34590, which can no longer be imported internally. [General][Added] - Added CSS logical properties. Pull Request resolved: facebook#35316 Test Plan: Unit test snapshots. Reviewed By: NickGerleman Differential Revision: D41230978 Pulled By: necolas fbshipit-source-id: 40e93d0d697f0cb28390480ce2b4bcbce18da70a * resolve some merge conflicts * fix tests * [0.71.11] Bump version numbers * [LOCAL] update podlock for CI * fix(ios): fix `pod install --project-directory=ios` failing (facebook#37993) * Prevent LogBox from crashing on long messages (facebook#38005) Summary: Pull Request resolved: facebook#38005 Fixes facebook#32093 by guarding the expensive `BABEL_CODE_FRAME_ERROR_FORMAT` regex with a cheaper initial scan. (Longer term, we should reduce our reliance on string parsing and propagate more structured errors.) Changelog: [General][Fixed] Prevent LogBox from crashing on very long messages Reviewed By: GijsWeterings Differential Revision: D46892454 fbshipit-source-id: 3afadcdd75969c2589bbb06f47d1c4c1c2690abd # Conflicts: # Libraries/LogBox/Data/parseLogBoxLog.js # packages/react-native/package.json * [0.71.12] Bump version numbers * [LOCAL] Bump podfile.lock * [LOCAL] Download artifacts from CI to speed up testing (facebook#37971) (facebook#38612) Co-authored-by: Riccardo Cipolleschi <[email protected]> resolved: facebook#37971 * fix: mount devtools overlay only if react devtools are connected (facebook#38784) * Allow RCTBundleURLProvider to request an inline source map (facebook#37878) (facebook#38995) 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 over the network. Instead, as far as I can tell, it expects you to pass the source URL at the time you load your bundle: https://developer.apple.com/documentation/javascriptcore/jscontext/1451384-evaluatescript?language=objc . This leads to a very sub-par developer experience debugging the JSbundle directly. It will however, pick up an inline source map. Therefore, let's add a way to have React Native tell metro to request an inline source map. I did this by modifying `RCTBundleURLProvider` to have a new query parameter for `inlineSourceMap`, and set to true by default for JSC. [IOS] [ADDED] - Added support to inline the source map via RCTBundleURLProvider Pull Request resolved: facebook#37878 Test Plan: I can put a breakpoint in RNTester, via Safari Web Inspector, in human readable code :D <img width="1728" alt="Screenshot 2023-06-14 at 4 09 03 AM" src="https://github.com/facebook/react-native/assets/6722175/055277fa-d887-4566-9dc6-3ea07a1a60b0"> Reviewed By: motiz88 Differential Revision: D46855418 Pulled By: huntie fbshipit-source-id: 2134cdbcd0a3e81052d26ed75f83601ae4ddecfe * chore(releases): improve bump oss script to allow less human errors (facebook#38666) (facebook#38890) Summary: One of the limitations of the existing flow for the release crew is that they need to manually remember to publish all the other packages in the monorepo ahead of a new patch release - this PR modifies the logic for the bump-oss-version script (and makes it available via yarn) so that it will not run if: * there are git changes lying around * if some of the packages need a new release it required a bit of refactoring to extract some portions of the logic from the bump-all-package-versions script, but I think the end result is pretty decent. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [CHANGED] - improve bump oss script to allow less human errors Pull Request resolved: facebook#38666 Test Plan: * checkout this branch * comment L54 of bump-oss-version.js (to remove the check on the branch name) * run `yarn bump-all-updated-packages`, verify that it works and that it detects that some packages have unreleased code * run `yarn bump-oss-version -t asd -v asd` (the "fake" parameters are needed to pass the yargs check), verify that it will throw an error because it finds a package that has unreleased code Reviewed By: mdvacca Differential Revision: D48156963 Pulled By: cortinico fbshipit-source-id: 2473ad5a84578c5236c905fd9aa9a88113fe8d22 # Conflicts: # scripts/publish-npm.js re-add the file nit # Conflicts: # package.json * Add scripts and pipeline to poll for maven (facebook#38980) (facebook#39039) * Fix onChangeText not firing when clearing the value of TextInput with multiline=true on iOS (facebook#37958) Summary: This fix fixes the TextInput issue in facebook#37784 with onChangeText not working on iOS only. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [FIXED] - Fix onChangeText not firing when clearing the value of TextInput with multiline=true on iOS Pull Request resolved: facebook#37958 Test Plan: 1. Run the repro code given in the issue (facebook#37784). 2. Verified that onChangeText works after pressing the submit button. 3. Run the repro code from the issue (facebook#36494) that caused this issue. 4. Verified that issue (facebook#36494) is not reoccurring. Reviewed By: rshest Differential Revision: D47185775 Pulled By: dmytrorykun fbshipit-source-id: 1a1a6534d6bf8b5bb8cf1090734dd894bab43f82 * For targeting SDK 34 - Added RECEIVER_EXPORTED/RECEIVER_NOT_EXPORTED flag support in DevSupportManagerBase (facebook#38256) Summary: Pull Request resolved: facebook#38256 Add RECEIVER_EXPORTED/RECEIVER_NOT_EXPORTED flag support to DevSupportManagerBase for Android 14 change. See https://developer.android.com/about/versions/14/behavior-changes-14#runtime-receivers-exported for details. Without this fix, app crashes during launch because of : ```SecurityException: {package name here}: One of RECEIVER_EXPORTED or RECEIVER_NOT_EXPORTED should be specified when a receiver isn't being registered exclusively for system broadcasts``` Changelog: [Targeting SDK 34] Added RECEIVER_EXPORTED/RECEIVER_NOT_EXPORTED flag support in DevSupportManagerBase Reviewed By: javache Differential Revision: D47313501 fbshipit-source-id: 12e8299559d08b4ff87b4bdabb0a29d27763c698 * [LOCAL] skip un-semver packages in 71 branch * Revert "[LOCAL] skip un-semver packages in 71 branch" This reverts commit a90485a. * [LOCAL] augment forEachPackage to accept optional excludes and add for the bump and trigger release script * [0.71.13] Bump version numbers * Re-enable direct debugging with JSC on iOS 16.4+ (facebook#37914) Summary: Pull Request resolved: facebook#37914 Restores facebook#37874 (reverted earlier today), with fix for `JSCRuntime.cpp` build on Android. Changelog: None Reviewed By: cortinico Differential Revision: D46762984 fbshipit-source-id: 6d56f81b9d0c928887860993b2b729ed96c0734c * Guard `JSGlobalContextSetInspectable` behind a compile time check for Xcode 14.3+ (facebook#39037) Summary: An earlier [change](facebook@8b1bf05) I made (that huntie resubmitted) only works on Xcode 14.3+ (See more info [here](react-native-community/discussions-and-proposals#687)). This change adds the appropriate compiler checks so that the change is compatible with Xcode 14.2 and earlier, and therefore cherry-pickable to 0.71 and 0.72. The check works by checking if iOS 16.4+ is defined, which is the closest proxy I could find for "Is this Xcode 14.3". ## Changelog: [IOS] [CHANGED] - Guard `JSGlobalContextSetInspectable` behind a compile time check for Xcode 14.3+ Pull Request resolved: facebook#39037 Test Plan: I can't actually test on Xcode 14.2 (it won't launch on my MacBook 😢), but I made a similar [PR](#1848) in React Native macOS, whose CI checks run against Xcode 14.2 and I'm getting passing checks there. Reviewed By: huntie Differential Revision: D48414196 Pulled By: NickGerleman fbshipit-source-id: ba10a6505dd11d982cc56c02bf9f7dcdc104bbec * Fix some issues --------- Co-authored-by: Lorenzo Sciandra <[email protected]> Co-authored-by: Nick Gerleman <[email protected]> Co-authored-by: Lorenzo Sciandra <[email protected]> Co-authored-by: Dmitry Rykun <[email protected]> Co-authored-by: Pieter De Baets <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Douglas Lowder <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Kyle Roach <[email protected]> Co-authored-by: Julien Brayere <[email protected]> Co-authored-by: Riccardo Cipolleschi <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: aleqsio <[email protected]> Co-authored-by: Harry Yu <[email protected]> Co-authored-by: Rob Hogan <[email protected]> Co-authored-by: Nicolas Gallagher <[email protected]> Co-authored-by: Alexander Eggers <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Tommy Nguyen <[email protected]> Co-authored-by: Moti Zilberman <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Riccardo Cipolleschi <[email protected]> Co-authored-by: Riccardo Cipolleschi <[email protected]> Co-authored-by: Ruslan Lesiutin <[email protected]> Co-authored-by: Koichi Nagaoka <[email protected]> Co-authored-by: Kun Wang <[email protected]> Co-authored-by: Distiller <[email protected]> Co-authored-by: Alex Hunt <[email protected]>
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.
Proposes a solution to the issues (facebook/react-native#36794, expo/expo#22008) caused by the change to JavaScriptCore that strips information from stack trace source URLs.
View the rendered RFC