Skip to content
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

[BPK-1754] Upgrade React Native to 0.57.3 #1114

Closed
wants to merge 17 commits into from
Closed

Conversation

shaundon
Copy link
Contributor

@shaundon shaundon commented Oct 15, 2018

Notes

We got Jest to run with the following things:

  • Change .babelrc to babel.config.js (weird I know).
  • Add @babel/core and babel-core and babel-jest to deps, based on the RN upgrade guide.
  • Added Jest as a dependency in native. This isn't desirable long term as it diverges from the version of Jest used in the root. However I think this is better for now to keep this PR small. After this I plan to do another PR to upgrade Jest for web, then I can remove Jest from native.
  • Upgrade React to 16.6. I wanted to avoid this but it's unavoidable, the tests won't work without it. It's specified in the upgrade guide here: https://github.com/react-native-community/react-native-releases/blob/master/CHANGELOG.md#0573.
  • I had to add jest-cli to deps to get rid of environment.dispose is not a function errors. How is this even a thing I really don't know

@shaundon shaundon force-pushed the BPK-1754-upgrade-rn branch 2 times, most recently from 3b33672 to 4e4154b Compare October 17, 2018 11:41
native/.babelrc Outdated
@@ -1,3 +1,3 @@
{
"presets": ["react-native"]
"presets": ["module:metro-react-native-babel-preset"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -2,52 +2,52 @@ PODS:
- boost-for-react-native (1.63.0)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update is because I did pod update to bump RN. I presume the other changes are ok though, seems to work fine.

@@ -11,20 +11,21 @@
"android:build": "react-native run-android",
"android": "npm run android:emulator & npm run android:build",
"test": "jest --coverage",
"storybook": "storybook start -p 7007 --root ../"
"storybook": "storybook start -p 7007 --skip-packager | react-native start --watchFolders $PWD/.."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storybook used to run the RN packager for us but their usage doesn't work with the new RN (see storybookjs/storybook#4179).

So, now we run the packager ourselves. $PWD/.. is because Watchman won't accept a relative path (i.e. ..), so this was my hack workaround.

"react": "16.5.1",
"react-dom": "16.5.1",
"react-native": "0.57.1",
"react-test-renderer": "16.5.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmikulka
Copy link

@shaundon just by comparing our package.json and yours, I can see few differences:

  • you need both "@babel/core": "^7.0.0" and "babel-core": "7.0.0-bridge.0"
  • "metro-react-native-babel-preset": "^0.45.6"
  • "jest": "23.6.0"

In jest config, we have

    "transform": {
      "^.+\\.jsx?$": "<rootDir>/node_modules/react-native/jest/preprocessor.js"
    },

.babelrc:

{
  "plugins": ["codegen"]
}

skyscanner-app/react-native/scripts/metro.config.js

const path = require('path');

module.exports = {
  transformer: {
    babelTransformerPath: require.resolve('./transformer'), // NOTE: we use react-native-obfuscating-transformer
  },
  projectRoot: path.resolve(__dirname, '..'),
};

@shaundon shaundon force-pushed the BPK-1754-upgrade-rn branch from 4a48df0 to 1468bb5 Compare October 25, 2018 15:36
@backpackbot
Copy link

backpackbot commented Oct 25, 2018

Warnings
⚠️

One or more packages have changed, but UNRELEASED.md wasn't updated.

⚠️

package-lock.json was updated. Ensure that this was intentional.

Generated by 🚫 dangerJS

@shaundon shaundon force-pushed the BPK-1754-upgrade-rn branch 5 times, most recently from 769dcaa to 3a2c989 Compare October 26, 2018 16:36
@shaundon shaundon force-pushed the BPK-1754-upgrade-rn branch from 3a2c989 to a46919c Compare October 26, 2018 16:37
@shaundon shaundon force-pushed the BPK-1754-upgrade-rn branch from a46919c to 9b05a5e Compare October 26, 2018 16:39
@shaundon
Copy link
Contributor Author

@shaundon
Copy link
Contributor Author

@shaundon shaundon force-pushed the BPK-1754-upgrade-rn branch 5 times, most recently from 0f77f6e to bd7f9bf Compare October 29, 2018 14:35
@shaundon shaundon changed the title [BPK-1754] Upgrade React Native to 0.57.x [BPK-1754] Upgrade React Native to 0.57.3 Oct 29, 2018
@shaundon shaundon force-pushed the BPK-1754-upgrade-rn branch from bd7f9bf to e6f6bd7 Compare October 29, 2018 15:01
@matthewdavidson
Copy link
Contributor

Closing since we have moved the react-native code to https://github.com/Skyscanner/backpack-react-native

@matthewdavidson matthewdavidson deleted the BPK-1754-upgrade-rn branch December 18, 2018 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants