[Dev] RN v0.45 Upgrade #570

sarahscott · 2017-05-26T13:07:19Z

Update transformer
Update various dependencies
Update snapshots to accommodate new RN version
Change all instances of React.PropTypes to PropTypes from the new standalone package
Fix Relay compilation error
Fix RCTAnimation subspec error ([iOS][cocoapods] 'RCTAnimation/RCTValueAnimatedNode.h' file not found facebook/react-native#13198)
Rebase with master
Fix SearchResults test failure in CI
Stress-test in simulator and on-device

Tested on Device?

@sarahscott

How to get set up with this PR?

To run on your computer:

git fetch origin pull/570/head:sarahscott-570-checkout
git checkout sarahscott-570-checkout
yarn install
cd example; pod install; cd ..
open -a Simulator
yarn start

Then run xcrun simctl launch booted net.artsy.Emission once a the simulator has finished booting

To run inside Eigen (prod or beta) or Emission (beta): Shake the phone to get the Admin menu.

If you see "Use Staging React Env" - click that and restart, then follow the next step.

Click on "Choose an RN build" - then pick the one that says: "X,Y,Z"

Note: this is a TODO for PRs, currently you can only do it on master commits.

ArtsyOpenSource · 2017-05-26T13:09:58Z

	Warnings
⚠️	Please assign someone to merge this PR, and optionally include people who should review.
⚠️	Missing Test Files: `src/lib/components/__tests__/load_failure_view-tests.tsx` `src/lib/components/__tests__/opaque_image_view-tests.tsx` `src/lib/components/__tests__/switch_view-tests.tsx` If these files are supposed to not exist, please update your PR body to include "Skip New Tests".
⚠️	Could not find stories using these components: `Biography` `Artworks` `LoadFailureView` `OpaqueImageView` `SwitchView`

Generated by 🚫 dangerJS

sarahscott · 2017-05-26T15:27:46Z

transformer/upstream.js

-const {compactMapping} = require('react-native/packager/src/Bundler/source-map');
+const {compactMapping} = require('./src/Bundler/source-map');
+
+import type {Plugins as BabelPlugins} from 'babel-core';


Here are the offensive imports

Oooh, they are in the updated file. I was going off of the code base in master as-is.

Ok this should be easy. Something like const BabelPlugins = require("babel-core").Plugins

alloy · 2017-05-29T15:32:53Z

@sarahscott I’ve cut a release of Relay with the RN 0.45 compatibility release: https://github.com/alloy/relay/releases/download/v0.9.3%2Breact-native-0.45-fix/react-relay-0.9.3.react-native-0.45-fix.tgz

alloy · 2017-05-31T12:08:20Z

src/lib/components/home/artwork_rails/__tests__/artwork_rail-tests.tsx

+   * back Enzyme or something similar once we figure out compatibility issues with RN 0.45+
+   */
+  rail.state = { didPerformFetch: true }
+


Oh, this is probably due to setState being async, you need to use something like setImmediate to perform assertions on the next runloop tick.

sarahscott · 2017-05-31T16:35:56Z

Example/Podfile

@@ -15,7 +15,7 @@ target 'Emission' do
  pod 'Emission', :path => '../'

  # As this runs dev, we need the developer web socket
-  pod 'React', :path => react_path, :subspecs => %w(RCTWebSocket)
+  pod 'React', :path => react_path, :subspecs => %w(DevSupport BatchedBridge RCTAnimation)


The RCTAnimation subspec causes problems, but I can build Emission if this postinstall script is included in package.json.

Where's the addition to our package.json?

I'll add it now - wasn't sure if we actually wanted to use it or not.

There doesn't appear to be another simple way around that, no?

I haven't found one. The Gene container relies on react-native-parallax-scroll-view, which uses Animated. However, the ArtistCard also uses Animated and works fine without RCTAnimation, so perhaps there is a way around it? With the post-install script and RCTAnimation, though, the Gene error is solved, so maybe it's a good idea to include it if we can.

ArtistCard uses the pure JS animation library, so it’s not using the pod like the Gene one is.

alloy · 2017-05-31T22:09:01Z

I can reproduce the CI test failures by running the tests ‘in band’, i.e. yarn test -- -i.

alloy · 2017-05-31T22:20:27Z

It happens in src/lib/components/consignments/components/__tests__/artist-search-results-tests.tsx, but only when ran with other test files, not in isolation.

alloy · 2017-05-31T22:50:32Z

Commenting out this component makes it go away. The component is essentially a TextView, maybe it needs stubbing? /cc @orta

orta · 2017-06-01T08:24:50Z

That sounds 👍 to me, it's not testing any work from the input itself

into sarah-rn-v45-upgrade

alloy · 2017-06-01T12:51:49Z

Can you add a CHANGELOG entry? I think it should then be green, in which case please go ahead and merge 👍

sarahscott · 2017-06-01T13:04:36Z

merging!

alloy · 2017-06-01T15:48:56Z

🍾

orta · 2017-06-05T11:58:50Z

package.json

    "clean-example": "cd Example && xcodebuild -workspace Emission.xcworkspace -scheme Emission -destination 'platform=iOS Simulator,name=iPhone 6' clean",
    "type-check": "tsc --noEmit --pretty",
    "lint": "tslint 'src/**/*.{ts,tsx}'",
    "lint-fix": "npm run lint -- --fix",
    "ci": "npm run type-check && npm run lint && npm run test -- --runInBand",
    "danger": "danger",
-    "start": "npm run clean-example && concurrently --kill-others 'npm run start-packager' 'npm run storybook'",


Was there a reason that storybooks were removed from the app? I don't see a mention in the PR body

Shipped #585 to fix it 👍

sarahscott commented May 26, 2017

View reviewed changes

sarahscott force-pushed the sarah-rn-v45-upgrade branch from a3e11b1 to c9d6f96 Compare May 30, 2017 17:26

alloy reviewed May 31, 2017

View reviewed changes

sarahscott commented May 31, 2017

View reviewed changes

sarahscott force-pushed the sarah-rn-v45-upgrade branch from a349081 to e0eb33d Compare May 31, 2017 17:40

sarahscott and others added 11 commits May 31, 2017 23:26

[Dev] Updated transformer

0438474

[Dev] Make start script use absolute path for config file

51cc576

[Dev] Added BatchedBridge spec to Podfile

85a68da

[Dev] Updated React to v16.0

f7d8431

[Dev] Updated react-test-renderer

60ad07f

[Dev] Removed Enzyme from ArtworkRail tests

3364879

[Dev] Replaced deprecated React.PropTypes with new prop-types package

c1b7ff0

[Dev] Added RCTAnimation subspec

c98fa9a

[package] Update Relay to have fixed RN 0.45 compat patch.

eb74adb

[package] Update RN to 0.45-rc.2

f2fead8

[ci] Try to skip patching of files that won’t be compiled.

beb89ab

alloy force-pushed the sarah-rn-v45-upgrade branch from c2f4641 to beb89ab Compare May 31, 2017 21:47

[test] Update snapshots

7f338c8

alloy and others added 3 commits June 1, 2017 14:38

[test] Work around crash jestjs/jest#3707

fcbee76

[dev] Fixed bundle script

1327d0f

Merge branch 'sarah-rn-v45-upgrade' of https://github.com/artsy/emission

528cfbf

into sarah-rn-v45-upgrade

sarahscott force-pushed the sarah-rn-v45-upgrade branch from b309860 to 528cfbf Compare June 1, 2017 12:49

[dev] changelog

7a3f28f

sarahscott changed the title ~~[🚨][WIP][Dev] RN v0.45 Upgrade~~ [Dev] RN v0.45 Upgrade Jun 1, 2017

sarahscott merged commit 2ab32d6 into master Jun 1, 2017

sarahscott deleted the sarah-rn-v45-upgrade branch June 1, 2017 13:04

orta reviewed Jun 5, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dev] RN v0.45 Upgrade #570

[Dev] RN v0.45 Upgrade #570

sarahscott commented May 26, 2017 •

edited

Loading

ArtsyOpenSource commented May 26, 2017 •

edited

Loading

sarahscott May 26, 2017

alloy May 26, 2017

alloy commented May 29, 2017

alloy May 31, 2017

sarahscott May 31, 2017

alloy May 31, 2017

sarahscott May 31, 2017

alloy May 31, 2017 •

edited

Loading

sarahscott May 31, 2017

alloy May 31, 2017

alloy commented May 31, 2017

alloy commented May 31, 2017 •

edited

Loading

alloy commented May 31, 2017

orta commented Jun 1, 2017

alloy commented Jun 1, 2017

sarahscott commented Jun 1, 2017

alloy commented Jun 1, 2017

orta Jun 5, 2017

orta Jun 5, 2017

alloy Jun 7, 2017

[Dev] RN v0.45 Upgrade #570

[Dev] RN v0.45 Upgrade #570

Conversation

sarahscott commented May 26, 2017 • edited Loading

Tested on Device?

ArtsyOpenSource commented May 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alloy commented May 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alloy May 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alloy commented May 31, 2017

alloy commented May 31, 2017 • edited Loading

alloy commented May 31, 2017

orta commented Jun 1, 2017

alloy commented Jun 1, 2017

sarahscott commented Jun 1, 2017

alloy commented Jun 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarahscott commented May 26, 2017 •

edited

Loading

ArtsyOpenSource commented May 26, 2017 •

edited

Loading

alloy May 31, 2017 •

edited

Loading

alloy commented May 31, 2017 •

edited

Loading