-
-
Notifications
You must be signed in to change notification settings - Fork 656
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
RN v0.61 upgrade followup. #4152
Commits on Jul 15, 2020
-
deps: Upgrade
react-native-webview
to ~10.0.0.We'd like to upgrade to the latest, but versions 10.1.0 and above don't have support for RN v0.61; minimum supported is v0.62. (Hence the tilde.) Breaking changes announced: - v9: iOS: Props updates to `injectedJavaScript` are no longer immutable. We don't use the `injectedJavaScript` prop, so this is fine. - v10: gradle: The Android Gradle plugin is only required when opening the project stand-alone, not when it is included as a dependency. [...] This doesn't sound like a description of a breaking change. As Greg said [0], "a thing is no longer required in a case where it previously was". We hesitate to take an x.0.0 version -- what if there are important bugfixes in minor/patch versions for issues introduced in that x.0.0 version? -- but they've made it a custom to post a note at the release if they know of major regressions, telling you to use a later version [1]. And no such note exists at v10.0.0. As a concrete example, we were concerned that the "bug fixes" identified in the v10.1.0 release might have been aimed at regressions introduced in v10.0.0. But following links to the mentioned PR and its corresponding issues (the second looks like a resumption of the first), it looks like a fix for a bug reported all the way back in version 5. So this isn't a reason to use v9.x.x instead of v10.0.0. Greg points out [2]: """ In a project that takes major vs. minor releases seriously, where a minor release means cherry-picked backported changes, I'd assume from this context that the major release introduced the issue. (Projects like that include RN itself, and the Zulip server.) But with this project I get the impression that their releases are all a linear stream of snapshots from master. Also that they do the flavor of "semantic versioning" that's basically "bump the major version number frequently and at random". So that makes this less clear. """ Ah, well. Also, update the libdef according to changes in `src/WebViewTypes.ts` in the `react-native-webview` repo. [0]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/RN.2060.20upgrade.3A.20react-native-webview/near/893807 [1]: See, e.g., v10.1.0, at https://github.com/react-native-community/react-native-webview/releases/tag/v10.1.0. [2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/RN.20v0.2E61.3A.20react-native-webview/near/901944
Configuration menu - View commit details
-
Copy full SHA for 41f0b0d - Browse repository at this point
Copy the full SHA 41f0b0dView commit details -
deps: Upgrade
react-native-unimodules
to ^0.9.1, the latest.This is possible, now that the RN v0.60 -> v0.61 upgrade is complete! [1] Change the `@unimodules/core` version, following a valid instruction in bc27f2d: """ So, declare @unimodules/core in our own "dependencies", with the version specified by react-native-unimodules in its active version. This matching should be done each time we change versions of react-native-unimodules. """ Also, remove some now-unnecessary code in `android/build.gradle`, as foreshadowed in cb87f90: """ Second, specify a new dependency for `unimodules-react-native-adapter` in our own `android/build.gradle`, in a fix that is necessary because we're locked on version 0.6.0 of react-native-unimodules. Filed as https://github.com/unimodules/react-native-unimodules/issues/130. """ There are some reasonable, automatic changes to the version-controlled files that ensure that unimodules are automatically linked [2]: `ios/Podfile.lock` and `android/app/src/main/java/com/zulipmobile/generated/BasePackageList.java`. These look like some modules that are part of `react-native-unimodules`' core; these additions are caused by the upgrade and don't stem from running something like `yarn add expo-apple-authentication` to get a particular Unimodule. Also, run `yarn yarn-deduplicate && yarn` as prompted by `tools/test deps`. [1]: See discussion of the various obstacles at https://github.com/unimodules/react-native-unimodules/issues/97#issuecomment-637714639 and https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Podfile.20dependency.20of.20a.20dependency/near/892373. [2]: This is similar to but distinct from `react-native`'s "autolinking" feature that we activated in f460460 and a9a9ac7. See https://github.com/unimodules/react-native-unimodules/issues/75#issuecomment-536517253 for a comment on that. Fixes: zulip#4091
Configuration menu - View commit details
-
Copy full SHA for e116c3f - Browse repository at this point
Copy the full SHA e116c3fView commit details -
jest: Add
jest-expo
preset, to be used in the next commit.A bit frustratingly, we need to install react-native-web, because it's a peer dep of jest-expo, and react-dom, because that's a peer dep of react-native-web. It's likely that we'll never run any code in either module, since we don't care about the web. (Expo seems to be expanding React Native's original claim for multi-platform compatibility, namely iOS and Android, to include the web. `react-native-web` looks like a thing that lets you write React Native...for the web.) At least they don't complain if we add them as dev dependencies instead of regular dependencies. Also, run `yarn yarn-deduplicate && yarn` as prompted by `tools/test deps`.
Configuration menu - View commit details
-
Copy full SHA for 62621ef - Browse repository at this point
Copy the full SHA 62621efView commit details -
jest: Use
jest-expo
as our preset.This calls `react-native`'s Jest setup code before running its own. In particular, it seems to be aware of all the Expo modules we might want to add using `react-native-unimodules`, and mocks them [1]. Since we don't need our own mocks for these, remove them. It seems like we still need to add entries in our `transformModulesWhitelist`, ah, well. But it's good to weed out boring mocks from our `jestSetup.js`, and leave only those that are interesting [2]. Also, it seems like each time we add a module from Expo, there's a debugging process that can be confusing [3]; so, nice to avoid that. It looks like the preset does explicitly consider the bare, non-"managed" Expo workflow [4], which we use. If `jest-expo` turns out to be buggy, or the dependency requirements get even more tangled or burdensome, we should feel free to abandon this effort; it's not terrible to have to add boring mocks. [1]: https://github.com/expo/expo/blob/b8bd30697/packages/jest-expo/src/preset/expoModules.js [2]: Seems like a few remain that aren't related to Expo. Hmm. [3]: https://github.com/zulip/zulip-mobile/pull/4034/files#r445956933 [4]: https://github.com/expo/expo/blob/b8bd30697/packages/jest-expo/src/preset/setup.js#L130
Configuration menu - View commit details
-
Copy full SHA for e40c020 - Browse repository at this point
Copy the full SHA e40c020View commit details -
jest: Comment about boring mocks, remove one that's unnecessary.
I'm not sure when exactly it became unnecessary to mock "Linking", but it's taken care of now in React Native's Jest setup [1]. [1]: https://github.com/facebook/react-native/blob/v0.61.5/jest/setup.js#L153
Configuration menu - View commit details
-
Copy full SHA for c1c9575 - Browse repository at this point
Copy the full SHA c1c9575View commit details -
Revert "ios: Add boilerplate about APIs we don't, in fact, use."
This reverts commit c8d0c8d. As planned [1] [2], now that we've upgraded react-native-unimodules. [1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/apple.20purpose.20strings/near/869928 [2] zulip#4091 (comment)
Configuration menu - View commit details
-
Copy full SHA for e55739e - Browse repository at this point
Copy the full SHA e55739eView commit details