-
Notifications
You must be signed in to change notification settings - Fork 905
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: improve upgrade to use rn-diff-purge (#176)
Summary: --------- Upgrading experience is a bit harsh for React Native. We either need to install global `react-native-git-upgrade` and try that, or patch projects manually using tools like [`rn-diff-purge`](https://github.com/pvinis/rn-diff-purge). Since `rn-diff-purge` gets some recognition in RN core and website and it has an automated path of applying patches, I thought that it would be nice to use it in our `upgrade` command. What the command does now: 1. Determine version to upgrade and run some validation checks 1. Fetch patch between current version and specified from [`rn-diff-purge`](https://github.com/pvinis/rn-diff-purge#version-changes) and rename diff app to current project name 1. If the patch is empty, install RN and its peer deps 1. Exit with success 1. Prepare for 3-way merge by creating temporary patch file and adding new git remote 1. Apply patch with a fallback to 3-way merge, exclude `package.json` because it will always be failing 1. Install RN and its peer deps 1. Cleanup temporary files and git remote 1. Exit with success This PR is technically breaking change, so I'm cool with introducing a temporary new command or put old one behind a different name. Test Plan: ---------- Added tests with various scenarios. Successfully upgrading to specified version: <img width="648" alt="screenshot 2019-02-16 at 22 22 24" src="https://user-images.githubusercontent.com/5106466/52905364-75a84f00-3239-11e9-9c3a-d3612d194cbf.png"> Failed to fetch a non-existent diff: <img width="680" alt="screenshot 2019-02-16 at 22 22 49" src="https://user-images.githubusercontent.com/5106466/52905365-75a84f00-3239-11e9-9e92-6fd410271151.png"> No upgrade needed: <img width="904" alt="screenshot 2019-02-16 at 22 33 27" src="https://user-images.githubusercontent.com/5106466/52905501-4abefa80-323b-11e9-9fec-2e6a44ff09a4.png"> Dependency mismatch: <img width="988" alt="screenshot 2019-02-16 at 22 36 04" src="https://user-images.githubusercontent.com/5106466/52905502-4abefa80-323b-11e9-92db-ed0d661e493c.png"> Upgrading to older version: <img width="556" alt="screenshot 2019-02-16 at 22 39 50" src="https://user-images.githubusercontent.com/5106466/52905533-cfaa1400-323b-11e9-9685-d45e24cf0d97.png">
- Loading branch information
Showing
10 changed files
with
717 additions
and
118 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
packages/cli/src/upgrade/__tests__/__snapshots__/upgrade.test.js.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`fetches regular patch, adds remote, applies patch, installs deps, removes remote,: RnDiffApp is replaced with app name (TestApp) 1`] = ` | ||
"Snapshot Diff: | ||
- First value | ||
+ Second value | ||
@@ -1,5 +1,5 @@ | ||
- diff --git a/RnDiffApp/android/build.gradle b/RnDiffApp/android/build.gradle | ||
+ diff --git a/TestApp/android/build.gradle b/TestApp/android/build.gradle | ||
index 85d8f2f8..a1e80854 100644 | ||
- --- a/RnDiffApp/android/build.gradle | ||
- +++ b/RnDiffApp/android/build.gradle | ||
+ --- a/TestApp/android/build.gradle | ||
+ +++ b/TestApp/android/build.gradle | ||
@@ -9,8 +9,8 @@ buildscript { | ||
@@ -28,6 +28,6 @@ | ||
- diff --git a/RnDiffApp/package.json b/RnDiffApp/package.json | ||
+ diff --git a/TestApp/package.json b/TestApp/package.json | ||
index 4e617645..c82829bd 100644 | ||
- --- a/RnDiffApp/package.json | ||
- +++ b/RnDiffApp/package.json | ||
+ --- a/TestApp/package.json | ||
+ +++ b/TestApp/package.json | ||
@@ -7,14 +7,14 @@" | ||
`; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
diff --git a/RnDiffApp/android/build.gradle b/RnDiffApp/android/build.gradle | ||
index 85d8f2f8..a1e80854 100644 | ||
--- a/RnDiffApp/android/build.gradle | ||
+++ b/RnDiffApp/android/build.gradle | ||
@@ -9,8 +9,8 @@ buildscript { | ||
supportLibVersion = "27.1.1" | ||
} | ||
repositories { | ||
- jcenter() | ||
google() | ||
+ jcenter() | ||
} | ||
dependencies { | ||
classpath 'com.android.tools.build:gradle:3.1.4' | ||
@@ -23,12 +23,12 @@ buildscript { | ||
allprojects { | ||
repositories { | ||
mavenLocal() | ||
+ google() | ||
jcenter() | ||
maven { | ||
// All of React Native (JS, Obj-C sources, Android binaries) is installed from npm | ||
url "$rootDir/../node_modules/react-native/android" | ||
} | ||
- google() | ||
} | ||
} | ||
|
||
diff --git a/RnDiffApp/package.json b/RnDiffApp/package.json | ||
index 4e617645..c82829bd 100644 | ||
--- a/RnDiffApp/package.json | ||
+++ b/RnDiffApp/package.json | ||
@@ -7,14 +7,14 @@ | ||
"test": "jest" | ||
}, | ||
"dependencies": { | ||
- "react": "16.5.0", | ||
- "react-native": "0.57.0" | ||
+ "react": "16.6.1", | ||
+ "react-native": "0.57.7" | ||
}, | ||
"devDependencies": { | ||
"babel-jest": "23.6.0", | ||
"jest": "23.6.0", | ||
- "metro-react-native-babel-preset": "0.47.1", | ||
- "react-test-renderer": "16.5.0" | ||
+ "metro-react-native-babel-preset": "0.49.2", | ||
+ "react-test-renderer": "16.6.1" | ||
}, | ||
"jest": { | ||
"preset": "react-native" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,209 @@ | ||
// @flow | ||
import execa from 'execa'; | ||
import path from 'path'; | ||
import fs from 'fs'; | ||
import snapshotDiff from 'snapshot-diff'; | ||
import upgrade from '../upgrade'; | ||
import { fetch } from '../helpers'; | ||
import logger from '../../util/logger'; | ||
|
||
jest.mock('https'); | ||
jest.mock('fs'); | ||
jest.mock('path'); | ||
jest.mock('execa', () => { | ||
const module = jest.fn((command, args) => { | ||
mockPushLog('$', 'execa', command, args); | ||
if (command === 'npm' && args[3] === '--json') { | ||
return Promise.resolve({ | ||
stdout: '{"react": "16.6.3"}', | ||
}); | ||
} | ||
return Promise.resolve({ stdout: '' }); | ||
}); | ||
return module; | ||
}); | ||
jest.mock( | ||
'/project/root/node_modules/react-native/package.json', | ||
() => ({ name: 'react-native', version: '0.57.8' }), | ||
{ virtual: true } | ||
); | ||
jest.mock( | ||
'/project/root/package.json', | ||
() => ({ name: 'TestApp', dependencies: { 'react-native': '^0.57.8' } }), | ||
{ virtual: true } | ||
); | ||
jest.mock('../../util/PackageManager', () => | ||
jest.fn(() => ({ | ||
install: args => { | ||
mockPushLog('$ yarn add', ...args); | ||
}, | ||
})) | ||
); | ||
jest.mock('../helpers', () => ({ | ||
...jest.requireActual('../helpers'), | ||
fetch: jest.fn(() => Promise.resolve('patch')), | ||
})); | ||
jest.mock('../../util/logger', () => ({ | ||
info: jest.fn((...args) => mockPushLog('info', args)), | ||
error: jest.fn((...args) => mockPushLog('error', args)), | ||
warn: jest.fn((...args) => mockPushLog('warn', args)), | ||
success: jest.fn((...args) => mockPushLog('success', args)), | ||
log: jest.fn((...args) => mockPushLog(args)), | ||
})); | ||
|
||
const currentVersion = '0.57.8'; | ||
const newVersion = '0.58.4'; | ||
const olderVersion = '0.56.0'; | ||
const ctx = { | ||
root: '/project/root', | ||
reactNativePath: '', | ||
}; | ||
const opts = { | ||
legacy: false, | ||
}; | ||
|
||
const samplePatch = jest | ||
.requireActual('fs') | ||
.readFileSync(path.join(__dirname, './sample.patch'), 'utf8'); | ||
|
||
let logs = []; | ||
const mockPushLog = (...args) => | ||
logs.push(args.map(x => (Array.isArray(x) ? x.join(' ') : x)).join(' ')); | ||
const flushOutput = () => logs.join('\n'); | ||
|
||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
// $FlowFixMe | ||
fs.writeFileSync = jest.fn(filename => mockPushLog('[fs] write', filename)); | ||
// $FlowFixMe | ||
fs.unlinkSync = jest.fn((...args) => mockPushLog('[fs] unlink', args)); | ||
logs = []; | ||
}); | ||
|
||
test('uses latest version of react-native when none passed', async () => { | ||
await upgrade.func([], ctx, opts); | ||
expect(execa).toBeCalledWith('npm', ['info', 'react-native', 'version']); | ||
}); | ||
|
||
test('errors when invalid version passed', async () => { | ||
await upgrade.func(['next'], ctx, opts); | ||
expect(logger.error).toBeCalledWith( | ||
'Provided version "next" is not allowed. Please pass a valid semver version' | ||
); | ||
}); | ||
|
||
test('errors when older version passed', async () => { | ||
await upgrade.func([olderVersion], ctx, opts); | ||
expect(logger.error).toBeCalledWith( | ||
`Trying to upgrade from newer version "${currentVersion}" to older "${olderVersion}"` | ||
); | ||
}); | ||
|
||
test('warns when dependency upgrade version is in semver range', async () => { | ||
await upgrade.func([currentVersion], ctx, opts); | ||
expect(logger.warn).toBeCalledWith( | ||
`Specified version "${currentVersion}" is already installed in node_modules and it satisfies "^0.57.8" semver range. No need to upgrade` | ||
); | ||
}); | ||
|
||
test('fetches empty patch and installs deps', async () => { | ||
(fetch: any).mockImplementation(() => Promise.resolve('')); | ||
await upgrade.func([newVersion], ctx, opts); | ||
expect(flushOutput()).toMatchInlineSnapshot(` | ||
"info Fetching diff between v0.57.8 and v0.58.4... | ||
info Diff has no changes to apply, proceeding further | ||
warn Continuing after failure. Most of the files are upgraded but you will need to deal with some conflicts manually | ||
info Installing [email protected] and its peer dependencies... | ||
$ execa npm info [email protected] peerDependencies --json | ||
$ yarn add [email protected] [email protected] | ||
$ execa git add package.json | ||
$ execa git add yarn.lock | ||
$ execa git add package-lock.json | ||
success Upgraded React Native to v0.58.4 🎉. Now you can review and commit the changes" | ||
`); | ||
}); | ||
|
||
test('fetches regular patch, adds remote, applies patch, installs deps, removes remote,', async () => { | ||
(fetch: any).mockImplementation(() => Promise.resolve(samplePatch)); | ||
await upgrade.func([newVersion], ctx, opts); | ||
expect(flushOutput()).toMatchInlineSnapshot(` | ||
"info Fetching diff between v0.57.8 and v0.58.4... | ||
[fs] write tmp-upgrade-rn.patch | ||
$ execa git remote add tmp-rn-diff-purge https://github.com/pvinis/rn-diff-purge.git | ||
$ execa git fetch --no-tags tmp-rn-diff-purge | ||
$ execa git apply --check tmp-upgrade-rn.patch --exclude=package.json -p2 --3way | ||
info Applying diff... | ||
$ execa git apply tmp-upgrade-rn.patch --exclude=package.json -p2 --3way | ||
[fs] unlink tmp-upgrade-rn.patch | ||
info Installing [email protected] and its peer dependencies... | ||
$ execa npm info [email protected] peerDependencies --json | ||
$ yarn add [email protected] [email protected] | ||
$ execa git add package.json | ||
$ execa git add yarn.lock | ||
$ execa git add package-lock.json | ||
info Running \\"git status\\" to check what changed... | ||
$ execa git status | ||
$ execa git remote remove tmp-rn-diff-purge | ||
success Upgraded React Native to v0.58.4 🎉. Now you can review and commit the changes" | ||
`); | ||
|
||
expect( | ||
snapshotDiff(samplePatch, fs.writeFileSync.mock.calls[0][1], { | ||
contextLines: 1, | ||
}) | ||
).toMatchSnapshot('RnDiffApp is replaced with app name (TestApp)'); | ||
}); | ||
|
||
test('cleans up if patching fails,', async () => { | ||
(fetch: any).mockImplementation(() => Promise.resolve(samplePatch)); | ||
(execa: any).mockImplementation((command, args) => { | ||
mockPushLog('$', 'execa', command, args); | ||
if (command === 'npm' && args[3] === '--json') { | ||
return Promise.resolve({ | ||
stdout: '{"react": "16.6.3"}', | ||
}); | ||
} | ||
if (command === 'git' && args[0] === 'apply') { | ||
// eslint-disable-next-line prefer-promise-reject-errors | ||
return Promise.reject({ | ||
code: 1, | ||
stderr: 'error: .flowconfig: does not exist in index\n', | ||
}); | ||
} | ||
return Promise.resolve({ stdout: '' }); | ||
}); | ||
|
||
try { | ||
await upgrade.func([newVersion], ctx, opts); | ||
} catch (error) { | ||
expect(error.message).toBe( | ||
'Upgrade failed. Please see the messages above for details' | ||
); | ||
} | ||
|
||
expect(flushOutput()).toMatchInlineSnapshot(` | ||
"info Fetching diff between v0.57.8 and v0.58.4... | ||
[fs] write tmp-upgrade-rn.patch | ||
$ execa git remote add tmp-rn-diff-purge https://github.com/pvinis/rn-diff-purge.git | ||
$ execa git fetch --no-tags tmp-rn-diff-purge | ||
$ execa git apply --check tmp-upgrade-rn.patch --exclude=package.json -p2 --3way | ||
info Applying diff (excluding: package.json, .flowconfig)... | ||
$ execa git apply tmp-upgrade-rn.patch --exclude=package.json --exclude=.flowconfig -p2 --3way | ||
[2merror: .flowconfig: does not exist in index[22m | ||
error Automatically applying diff failed | ||
info Here's the diff we tried to apply: https://github.com/pvinis/rn-diff-purge/compare/version/0.57.8...version/0.58.4 | ||
info You may find release notes helpful: https://github.com/facebook/react-native/releases/tag/v0.58.4 | ||
[fs] unlink tmp-upgrade-rn.patch | ||
warn Continuing after failure. Most of the files are upgraded but you will need to deal with some conflicts manually | ||
info Installing [email protected] and its peer dependencies... | ||
$ execa npm info [email protected] peerDependencies --json | ||
$ yarn add [email protected] [email protected] | ||
$ execa git add package.json | ||
$ execa git add yarn.lock | ||
$ execa git add package-lock.json | ||
info Running \\"git status\\" to check what changed... | ||
$ execa git status | ||
$ execa git remote remove tmp-rn-diff-purge | ||
warn Please run \\"git diff\\" to review the conflicts and resolve them" | ||
`); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// @flow | ||
import https from 'https'; | ||
|
||
export const fetch = (url: string) => | ||
new Promise<string>((resolve, reject) => { | ||
const request = https.get(url, response => { | ||
if (response.statusCode < 200 || response.statusCode > 299) { | ||
reject( | ||
new Error(`Failed to load page, status code: ${response.statusCode}`) | ||
); | ||
} | ||
const body = []; | ||
response.on('data', chunk => body.push(chunk)); | ||
response.on('end', () => resolve(body.join(''))); | ||
}); | ||
request.on('error', err => reject(err)); | ||
}); |
Oops, something went wrong.