-
-
Notifications
You must be signed in to change notification settings - Fork 651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade to RN v0.64 #4426
Comments
We'll be jumping to Flow 0.137.0, which means (among other things) that something called "types-first" has been both introduced and enabled by default. We can turn it off, though, by adding The thing we'll have to do to enable it is annotate almost all the exports of our modules:
Not annotating an export sufficiently will raise a It looks like we'll be able to work toward that incrementally by selecting certain directories where we want to check the exports of the contained modules. They mention a codemod, but I suspect, if we use it at all, we'll want to use it on small sections of the code, one at a time, and check its work. |
When we start using Flow v0.137, along with React Native v0.64 (that's issue zulip#4426), we'll be asked to specify what rule(s) we're suppressing with each suppression comment. Conveniently, the warnings will tell us the applicable rule(s). So, using those warnings, get a head start on doing so. There will probably be more of these (we haven't yet started enforcing the requirement now, so some unmarked `$FlowFixMe`s will probably accumulate), but we might as well get this chunk out of the way.
When we start using Flow v0.137+, along with React Native v0.64 (that's issue zulip#4426), we'll be asked to specify what rule(s) we're suppressing with each suppression comment. Conveniently, the warnings will tell us the applicable rule(s) we need to write down. So, having peeked ahead at the warnings seen in the RN 0.64.0-rc.2 world, get a head start on doing so. There will probably be more of these (we haven't yet started enforcing the requirement now, so some unmarked `$FlowFixMe`s will naturally accumulate), but we might as well get this chunk out of the way.
When we start using Flow v0.137+, along with React Native v0.64 (that's issue zulip#4426), we'll be asked to specify what rule(s) we're suppressing with each suppression comment. Conveniently, the warnings will tell us the applicable rule(s) we need to write down. So, having peeked ahead at the warnings seen in the RN 0.64.0-rc.2 world, get a head start on doing so. There will probably be more of these (we haven't yet started enforcing the requirement now, so some unmarked `$FlowFixMe`s will naturally accumulate), but we might as well get this chunk out of the way.
Cool! The performance benefits of that won't be as big for us as they are for a giant codebase like Facebook's, but we should see some. I think the quality of the type errors may improve, too. Re the codemod, we'll definitely want to check its work: making readable commits and reviewing them normally. I suspect we'll want to start by trying it on relatively self-contained swathes of code like And if the codemod is really bad at its job, we can always ignore it and do the changes manually. But I'm hopeful that it'll be good enough, automating enough boring stuff accurately enough, that it'll be worth using. FTR here's the full list of changes we'll see -- up to this point in the changelog: Other changes I see in a quick scan which we might appreciate, or might have to adapt for:
This should let us clean up a few types.
The second part of this is #4433. The first means we'll have to drop
I'm hopeful these will translate to some nice improvements in the VS Code editing experience!
Hmm, I wonder if this will defeat the workaround we've used for a Flow bug involving |
Oh also: this might be exactly the right thing to do for the RN upgrade. We'll want to do this if the types-first migration seems to be any substantial amount of work, I think. But then we'll want to follow up by doing the types-first migration. A newer version of Flow (v0.143.0, out just this month) makes that the only available mode, so we'll presumably get that change in the next version of RN, v0.65. |
When we start using Flow v0.137+, along with React Native v0.64 (that's issue zulip#4426), we'll be asked to specify what rule(s) we're suppressing with each suppression comment. Conveniently, the warnings will tell us the applicable rule(s) we need to write down. So, having peeked ahead at the warnings seen in the RN 0.64.0-rc.2 world, get a head start on doing so. There will probably be more of these (we haven't yet started enforcing the requirement now, so some unmarked `$FlowFixMe`s will naturally accumulate), but we might as well get this chunk out of the way.
When we start using Flow v0.137+, along with React Native v0.64 (that's issue #4426), we'll be asked to specify what rule(s) we're suppressing with each suppression comment. Conveniently, the warnings will tell us the applicable rule(s) we need to write down. So, having peeked ahead at the warnings seen in the RN 0.64.0-rc.2 world, get a head start on doing so. There will probably be more of these (we haven't yet started enforcing the requirement now, so some unmarked `$FlowFixMe`s will naturally accumulate), but we might as well get this chunk out of the way.
When we start using Flow v0.137+, along with React Native v0.64 (that's issue zulip#4426), we'll be asked to specify what rule(s) we're suppressing with each suppression comment. Conveniently, the warnings will tell us the applicable rule(s) we need to write down. So, having peeked ahead at the warnings seen in the RN 0.64.0-rc.2 world, get a head start on doing so. There will probably be more of these (we haven't yet started enforcing the requirement now, so some unmarked `$FlowFixMe`s will naturally accumulate), but we might as well get this chunk out of the way.
And, v0.64 was released yesterday! Here's the blog post: https://reactnative.dev/blog/2021/03/11/version-0.64 |
One small followup to do after this upgrade (or the relevant part of it): the upgraded Metro should fix the issue that #4819 worked around. So then we should revert the workaround. |
To get a version that has the React Native peer dep range bumped to include React Native v0.64, which we hope to upgrade to soon (zulip#4426). There is one announced breaking change for Android; the `setSupportMultipleWindows` prop is introduced, defaulting to `true` [1]. This is to "mitigate the security advisory CVE-2020-6506". The advisory says, "This vulnerability affects React Native apps which use a react-native-webview that allows navigation to arbitrary URLs, and when that app runs on systems with an Android WebView version prior to 83.0.4103.106." I'm skeptical that we were affected, because I don't think we allow navigation to arbitrary URLs; see our comments on our use of the `originWhitelist` and `onShouldStartLoadWithRequest`. But good that they're addressing reported vulnerabilities. [1] https://github.com/react-native-webview/react-native-webview/releases/tag/v11.0.0
To get a version that has the React Native peer dep range bumped to include React Native v0.64, which we hope to upgrade to soon (zulip#4426). There is one announced breaking change for Android; the `setSupportMultipleWindows` prop is introduced, defaulting to `true` [1]. This is to "mitigate the security advisory CVE-2020-6506". The advisory says, "This vulnerability affects React Native apps which use a react-native-webview that allows navigation to arbitrary URLs, and when that app runs on systems with an Android WebView version prior to 83.0.4103.106." I'm skeptical that we were affected, because I don't think we allow navigation to arbitrary URLs; see our comments on our use of the `originWhitelist` and `onShouldStartLoadWithRequest` props. But good that they're addressing reported vulnerabilities. [1] https://github.com/react-native-webview/react-native-webview/releases/tag/v11.0.0
Just opened #4843 to get to a version of react-native-webview whose peer-dep range for React Native include v0.64. I didn't notice any other current third-party dependencies we'll have to do this for! 🎉 |
To get a version that has the React Native peer dep range bumped to include React Native v0.64, which we hope to upgrade to soon (zulip#4426). There is one announced breaking change for Android; the `setSupportMultipleWindows` prop is introduced, defaulting to `true` [1]. This is to "mitigate the security advisory CVE-2020-6506". The advisory says, "This vulnerability affects React Native apps which use a react-native-webview that allows navigation to arbitrary URLs, and when that app runs on systems with an Android WebView version prior to 83.0.4103.106." I'm skeptical that we were affected, because I don't think we allow navigation to arbitrary URLs; see our comments on our use of the `originWhitelist` and `onShouldStartLoadWithRequest` props. But good that they're addressing reported vulnerabilities. [1] https://github.com/react-native-webview/react-native-webview/releases/tag/v11.0.0
To get a version that has the React Native peer dep range bumped to include React Native v0.64, which we hope to upgrade to soon (zulip#4426). There is one announced breaking change for Android; the `setSupportMultipleWindows` prop is introduced, defaulting to `true` [1]. This is to "mitigate the security advisory CVE-2020-6506". The advisory says, "This vulnerability affects React Native apps which use a react-native-webview that allows navigation to arbitrary URLs, and when that app runs on systems with an Android WebView version prior to 83.0.4103.106." I'm skeptical that we were affected, because I don't think we allow navigation to arbitrary URLs; see our comments on our use of the `originWhitelist` and `onShouldStartLoadWithRequest` props. But good that they're addressing reported vulnerabilities. [1] https://github.com/react-native-webview/react-native-webview/releases/tag/v11.0.0
Just while we wait for the code in this package to be error-free when exact_by_default is enabled [1]. We plan to enable it for the RN v0.64 upgrade (zulip#4426). The PR is react-native-cameraroll/react-native-cameraroll#328. It's very possible that some of those object types would be better as exact instead of inexact. But: 1. Just getting the library to pass the implicit-inexact-object lint is enough to unblock switching on exact_by_default (for us and for other projects that want to do that too). 2. I guess we could go through and decide what should stay inexact and what should be made exact. But then each of those choices would have to be approved, and some of them would probably be breaking changes that we'd need to write changelog entries for. (Not ones that would affect an app at runtime -- but still, apps that use Flow would predictably have to change their code to adapt.) [1] https://medium.com/flow-type/how-to-upgrade-to-exact-by-default-object-type-syntax-7aa44b4d08ab
Just while we wait for the code in this package to be error-free when exact_by_default is enabled [1]. We plan to enable it for the RN v0.64 upgrade (zulip#4426). The PR is react-native-cameraroll/react-native-cameraroll#328. It's very possible that some of those object types would be better as exact instead of inexact. But: 1. Just getting the library to pass the implicit-inexact-object lint is enough to unblock switching on exact_by_default (for us and for other projects that want to do that too). 2. I guess we could go through and decide what should stay inexact and what should be made exact. But then each of those choices would have to be approved, and some of them would probably be breaking changes that we'd need to write changelog entries for. (Not ones that would affect an app at runtime -- but still, apps that use Flow would predictably have to change their code to adapt.) So instead, the PR just makes all the implicit inexact object types explicitly inexact. It's based directly on v4.0.4, the version we've been using, so it doesn't pull in any other changes. [1] https://medium.com/flow-type/how-to-upgrade-to-exact-by-default-object-type-syntax-7aa44b4d08ab
From Flow's perspective, we're fine to delay types-first until we're on v0.143, which we'd normally take along with the RN v0.65 upgrade. But, from experimentation, React Native v0.64's internal code seems to have been written with Types-First, such that a few Flow errors would show up if we tried to use RN v0.64 without Types-First enabled. So, enable it, now that we can, and now that the RN v0.64 upgrade is on the horizon (zulip#4426). Fixes: zulip#4907
From Flow's perspective, we're fine to delay types-first until we're on v0.143, which we'd normally take along with the RN v0.65 upgrade. But, from experimentation, React Native v0.64's internal code seems to have been written with Types-First, such that a few Flow errors would show up if we tried to use RN v0.64 without Types-First enabled. So, enable it, now that we can, and now that the RN v0.64 upgrade is on the horizon (zulip#4426). Fixes: zulip#4907
…v0.64. This would be handled already if our transformIgnorePatterns extended the transformIgnorePatterns in React Native's Jest preset, because that preset was adapted in facebook/react-native@a77f2c40d, released in RN v0.64. It would also be handled already if our transformIgnorePatterns extended the transformIgnorePatterns in `jest-expo`'s Jest preset after *that* preset was adapted, in the (as-yet-unreleased) change in expo/expo@24bce422c. But we ignore and clobber transformIgnorePatterns in those presets, so we add this line ourselves. If we upgrade RN before taking that `jest-expo` change, we'll see the following harmless warning when we run Jest: react-native/jest-preset contained different transformIgnorePatterns than expected. It's harmless because we don't use `jest-expo`'s transformIgnorePatterns anyway; we fully specify our own instead. Related: zulip#4426
In this commit: - Update `react-native`, `react`, and `flow-bin` in package.json, and bump the Flow version in the Flow config. All of this is normal. - Add a `resolutions` line for `react-test-renderer`, while we wait for `jest-expo` to officially target RN v0.64. I'd opened the relevant change in expo/expo#13549, but it landed first in expo/expo@d49e7070a. As of 2021-09-03, that change hasn't been released, but it might be soon. This line raises the following warning from Yarn: warning Resolution field "react-test-renderer@17.0.1" is incompatible with requested version "react-test-renderer@~16.11.0" This is expected. The doc says [1], "You will receive a warning if your resolution version or range is not compatible with the original version range." We've managed to get away with incompatible `resolutions` lines in the past; I guess that's a Yarn bug that we're not running into this time. - Enable `exact_by_default`, to follow facebook/react-native@050a7dd01. (We'd done a lot of work to prepare for this.) We couldn't enable it earlier because RN v0.63 didn't pass type-checking with it on. - Do two find-and-replace-all tasks, for some places where we're importing from React Native's internals and their paths have changed: 'react-native/Libraries/Animated/src/nodes/AnimatedValue' -> 'react-native/Libraries/Animated/nodes/AnimatedValue', from facebook/react-native@9c9e67791. 'react-native/Libraries/StyleSheet/StyleSheetTypes' -> 'react-native/Libraries/StyleSheet/StyleSheet' from facebook/react-native@0a6713312. And, for this one, they give the reason as "to encourage its use in product code"! 🎉 - And that's it! [1] https://classic.yarnpkg.com/en/docs/selective-version-resolutions/#toc-tips-tricks Fixes: zulip#4426
Since we're about to merge #4991 to complete this upgrade 🎉 , I just scanned this thread to look for follow-ups. It looks like there's just one:
|
…v0.64. This would be handled already if our transformIgnorePatterns extended the transformIgnorePatterns in React Native's Jest preset, because that preset was adapted in facebook/react-native@a77f2c40d, released in RN v0.64. It would also be handled already if our transformIgnorePatterns extended the transformIgnorePatterns in `jest-expo`'s Jest preset after *that* preset was adapted, in the (as-yet-unreleased) change in expo/expo@24bce422c. But we ignore and clobber transformIgnorePatterns in those presets, because the API just isn't suitable to taking a list from elsewhere and extending it [1]. So we add this line ourselves. Since we're upgrading RN before taking that `jest-expo` change, we'll see the following harmless warning for a while when we run Jest: react-native/jest-preset contained different transformIgnorePatterns than expected. It's harmless because we don't use `jest-expo`'s transformIgnorePatterns anyway; we fully specify our own instead. [1] zulip#4991 (comment) Related: zulip#4426
…ngView The immediate catalyst for this is zulip#5162, but I've been frustrated for some time about the quality of KeyboardAvoidingView. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. We haven't yet successfully forked react-native; my current understanding is that that'd be kind of a project, but I could be wrong. None of my PRs can be accepted until RN starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. RN upgrades take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. We haven't yet successfully forked react-native; my current understanding is that that'd be kind of a project, but I could be wrong. None of my PRs to React Native can be accepted until RN starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. We haven't yet successfully forked react-native; my current understanding is that that'd be kind of a project, but I could be wrong. None of my PRs to React Native can be accepted until RN starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until RN starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView including these two, which raise the same issue as zulip#5162 and haven't been resolved: facebook/react-native#29974 facebook/react-native#31484 (patch-package isn't a resolution) I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until RN starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView including these two, which raise the same issue as zulip#5162 and haven't been resolved: facebook/react-native#29974 facebook/react-native#31484 (patch-package isn't a resolution) I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until Facebook starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView including these two, which raise the same issue as zulip#5162 and haven't been resolved: facebook/react-native#29974 facebook/react-native#31484 (patch-package isn't a resolution) I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until Facebook starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView including these two, which raise the same issue as zulip#5162 and haven't been resolved: facebook/react-native#29974 facebook/react-native#31484 (patch-package isn't a resolution) I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until Facebook starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView including these two, which raise the same issue as zulip#5162 and haven't been resolved: facebook/react-native#29974 facebook/react-native#31484 (patch-package isn't a resolution) I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until Facebook starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
…ngView The immediate catalyst for this is zulip#5162, but I've had earlier encounters with React Native's KeyboardAvoidingView, and I don't love it. See - our 70eca07; that took a lot of painstaking investigation that would have been unnecessary if `keyboardVerticalOffset` had had a clear design from the beginning - the implementation, at https://github.com/facebook/react-native/blob/v0.64.2/Libraries/Components/Keyboard/KeyboardAvoidingView.js - the long list of open issues in RN that mention it: https://github.com/facebook/react-native/issues?q=is%3Aissue+is%3Aopen+KeyboardAvoidingView including these two, which raise the same issue as zulip#5162 and haven't been resolved: facebook/react-native#29974 facebook/react-native#31484 (patch-package isn't a resolution) I'm not optimistic that we can get zulip#5162 fixed in bounded time unless we abandon KeyboardAvoidingView and implement the necessary logic ourselves. Fortunately, that seems pretty doable. I don't yet see a quick fix in KeyboardAvoidingView that doesn't risk making its code even more brittle and hard to reason about. My threshold for that is pretty low in this case: with the code in its current state, each additional piece of logic (even sound logic) comes with an unusually large cost of understanding/maintaining it. My current understanding is that the initial setup for maintaining our own react-native fork would be kind of a big project, but I could be wrong. None of my PRs to React Native can be accepted until Facebook starts checking its CLA-related mail; I should be covered by the corporate CLA signed by Kandra. See, e.g., facebook/react-native#29926 (comment) . RN upgrades tend to take a while for us to do; see our most recent, to RN v0.64: zulip#4426. RN v0.67 is already doing release candidates. Tested on my iPhone 13 Pro running iOS 15, with and without the "Prefer Cross-Fade Transitions" setting (for the sake of zulip#5162). Fixes: zulip#5162
The next round after #4245. This is the first time I'm opening one of these when we're already on the previous latest release, and before the new one is even out yet!
The RN community release managers are working on the next release now, and it's anticipated in the next few weeks. Details at react-native-community/releases#214 and particularly at react-native-community/releases#214 (comment) .
There's already a release candidate, so that would be a way to get an early check on any issues we'll run into.
The text was updated successfully, but these errors were encountered: