-
-
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 v61 upgrade #4151
RN v61 upgrade #4151
Conversation
Hmm, actually, there's a part I haven't investigated yet—the various changes to [...] Hmm, so there weren't actually any changes to the git lsp v0.60.6..v0.61.5 -- template/ios/HelloWorld.xcodeproj/project.pbxproj I'm guessing the changes showing up in the upgrade helper are NFC and incidental, and caused by some changes in the environment in which |
edit: One way of doing this is at e956f99 from my current revision of #4183 (not merged yet). Bookmarking this, as something we might (?) have to think about, for mocking native modules now that Haste isn't being used: https://altany.github.io/react-native/0.61/jest/mocking/upgrade/2020/01/25/mocking-react-native-0.61-modules-with-jest.html. It links to facebook/react-native#26579 (comment). |
c0dc963
to
c323ea1
Compare
Thanks @chrisbobbe ! Chat discussion starting here. Looks good, with some comments in that thread:
|
jest/jestSetup.js
Outdated
// modules being available after the bridge initializes. | ||
// | ||
// The 'react-native' we import here is already mocked to a great | ||
// extent by `node_modules/react-native-jest/setup.js`, thanks to our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// extent by `node_modules/react-native-jest/setup.js`, thanks to our | |
// extent by `node_modules/react-native/jest/setup.js`, thanks to our |
f3ff7a3
to
eea6501
Compare
Thanks for the review! Just pushed these changes to this branch; also propagated them to my branch rn-61-followup-1, for #4152. |
With something called Haste, we were allowed to just pass "Linking" to `jest.mock`, and it would automagically be known that we want something called "Linking" from React Native. With RN v0.61, Haste is no longer used, and that way of mocking breaks. One possible response is to spell out the entire path to "Linking" within `react-native`: jest.mock('react-native/Libraries/Linking/Linking') But that's brittle: that path may change with new React Native versions, and it'll be unpleasant to have to adapt. The recommended solution [1] is to mock the `react-native` module ourselves, on top of the mocking that React Native's Jest setup does for us. And to put our "Linking" mock there. So, do. The *exact* recommendation is something that uses `Object.setPrototypeOf`. We don't do that. Instead, Greg found an earlier revision of the comment where that recommendation appears, and we go from there. This way, we avoid an awkward problem with react-native-vector-icons. That library re-exports `react-native` in its `lib/react-native.js`, and imports that when they want properties from the `react-native` module. Errors ensue; it appears that their strategy cuts off access to properties we'd intended to make available by using ReactNative as a prototype. So, don't mess around with prototypes. [1] facebook/react-native#26579 (comment)
Part of the RN v0.60 -> v0.61 upgrade [1]. Corresponds to facebook/react-native@9127fb51f. [1]: https://react-native-community.github.io/upgrade-helper/?from=0.60.6&to=0.61.5
Part of the RN v0.60 -> v0.61 upgrade [1]. Corresponds to facebook/react-native@ba8f88d1a. Before the upgrade commit, we add the (temporarily unnecessary) new name. After the upgrade, we'll remove the (then-unnecessary) old name. [1]: https://react-native-community.github.io/upgrade-helper/?from=0.60.6&to=0.61.5
Using the RN Upgrade Helper, a web app showing the diff from the release/0.60.6 and the release/0.61.5 branches of the `react-native-community/rn-diff-purge` repo, at https://react-native-community.github.io/upgrade-helper/?from=0.60.6&to=0.61.5. In this commit: - Upgrade `react-native`, `react`, and `flow-bin` following the template - Change our Podfile to reflect new/changed iOS dependencies in RN, following the template - Make changes to adapt to multiple upgrades of RN's Hermes dependency (details below) - Run `yarn yarn-deduplicate && yarn` as prompted by `tools/test deps` See (on the issue) a list of changes from the upgrade helper that we don't do in this series [1]. Hermes details: In facebook/react-native@c21e36db4, React Native started using v0.1.1 of Hermes [2], which includes a rename of the NPM package (facebook/hermes@c74842e) from 'hermesvm' to 'hermes-engine'. So, use the new path in the one place where its path occurs in our code, following the changes to the template in that commit. In facebook/react-native@06c64f5f1, React Native started using v0.2.1 of Hermes [3], which was came with the announcement, "The C++ runtime library is now packaged in a separate AAR to avoid `pickFirst` nondeterminisms". Follow the changes to the template in that commit, where several `pickFirst` lines are removed. [1]: zulip#3781 (comment) [2]: https://github.com/facebook/hermes/releases/tag/v0.1.1 [3]: https://github.com/facebook/hermes/releases/tag/v0.2.1 Fixes: zulip#3781
Part of the RN v0.60 -> v0.61 upgrade [1]. Corresponds to facebook/react-native@ba8f88d1a. Before the upgrade commit, we added the (then-unnecessary) new name. Now that the upgrade is done, we remove the (now-unnecessary) old name. [1]: https://react-native-community.github.io/upgrade-helper/?from=0.60.6&to=0.61.5
Part of the RN v0.60 -> v0.61 changes to the template app [1], corresponding to facebook/react-native@bf8d91868 and a correction in facebook/react-native@732ded7f7. This must happen at or after the main upgrade commit because the former declares that it depends on facebook/react-native#25100, which landed in facebook/react-native@69d1ed731 and was released in v0.61.0. [1]: https://react-native-community.github.io/upgrade-helper/?from=0.60.6&to=0.61.5
Merged! 🎉 Thanks for pushing this through. |
Awaiting #4150.And, it's merged! Thanks, @gnprice !Fixes: #3781