-
Notifications
You must be signed in to change notification settings - Fork 906
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
chore: migrate upgrade command to TS and remove legacy implementation #684
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
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
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
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
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
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
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
This file was deleted.
Oops, something went wrong.
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,40 @@ | ||
import {Command} from '@react-native-community/cli-types'; | ||
|
||
// @ts-ignore - JS file | ||
import server from './server/server'; | ||
// @ts-ignore - JS file | ||
import bundle from './bundle/bundle'; | ||
// @ts-ignore - JS file | ||
import ramBundle from './bundle/ramBundle'; | ||
// @ts-ignore - JS file | ||
import link from './link/link'; // eslint-disable-line | ||
// @ts-ignore - JS file | ||
import unlink from './link/unlink'; // eslint-disable-line | ||
// @ts-ignore - JS file | ||
import install from './install/install'; // eslint-disable-line | ||
// @ts-ignore - JS file | ||
import uninstall from './install/uninstall'; // eslint-disable-line | ||
import upgrade from './upgrade/upgrade'; | ||
// @ts-ignore - JS file | ||
import info from './info/info'; // eslint-disable-line | ||
// @ts-ignore - JS file | ||
import config from './config/config'; // eslint-disable-line | ||
// @ts-ignore - JS file | ||
import init from './init'; | ||
// @ts-ignore - JS file | ||
import doctor from './doctor'; | ||
|
||
export default [ | ||
server, | ||
bundle, | ||
ramBundle, | ||
link, | ||
unlink, | ||
install, | ||
uninstall, | ||
upgrade, | ||
info, | ||
config, | ||
init, | ||
doctor, | ||
] as Command[]; |
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
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
File renamed without changes.
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 |
---|---|---|
@@ -1,12 +1,11 @@ | ||
// @flow | ||
import execa from 'execa'; | ||
import path from 'path'; | ||
import fs from 'fs'; | ||
import snapshotDiff from 'snapshot-diff'; | ||
import stripAnsi from 'strip-ansi'; | ||
import upgrade from '../upgrade'; | ||
import {fetch, logger} from '@react-native-community/cli-tools'; | ||
import loadConfig from '../../../tools/config'; | ||
import loadConfig from '../../../tools/config'; // eslint-disable-line | ||
import merge from '../../../tools/merge'; | ||
|
||
jest.mock('https'); | ||
|
@@ -56,7 +55,9 @@ jest.mock('@react-native-community/cli-tools', () => ({ | |
})); | ||
|
||
const mockFetch = (value = '', status = 200) => { | ||
(fetch: any).mockImplementation(() => Promise.resolve({data: value, status})); | ||
(fetch as jest.Mock).mockImplementation(() => | ||
Promise.resolve({data: value, status}), | ||
); | ||
}; | ||
|
||
const mockExecaDefault = (command, args) => { | ||
|
@@ -85,9 +86,6 @@ const currentVersion = '0.57.8'; | |
const newVersion = '0.58.4'; | ||
const olderVersion = '0.56.0'; | ||
const ctx = loadConfig(); | ||
const opts = { | ||
legacy: false, | ||
}; | ||
|
||
const samplePatch = jest | ||
.requireActual('fs') | ||
|
@@ -101,34 +99,30 @@ const flushOutput = () => stripAnsi(logs.join('\n')); | |
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
jest.restoreAllMocks(); | ||
// $FlowFixMe | ||
fs.writeFileSync = jest.fn(filename => mockPushLog('[fs] write', filename)); | ||
// $FlowFixMe | ||
fs.unlinkSync = jest.fn((...args) => mockPushLog('[fs] unlink', args)); | ||
logs = []; | ||
(execa: any).mockImplementation(mockExecaDefault); | ||
((execa as unknown) as jest.Mock).mockImplementation(mockExecaDefault); | ||
Object.defineProperty(process, 'platform', { | ||
value: 'darwin', | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
// $FlowFixMe | ||
fs.writeFileSync = jest.requireMock('fs').writeFileSync; | ||
// $FlowFixMe | ||
fs.unlinkSync = jest.requireMock('fs').unlinkSync; | ||
}); | ||
|
||
test('uses latest version of react-native when none passed', async () => { | ||
await upgrade.func([], ctx, opts); | ||
await upgrade.func([], ctx); | ||
expect(execa).toBeCalledWith('npm', ['info', 'react-native', 'version']); | ||
}, 60000); | ||
|
||
test('applies patch in current working directory when nested', async () => { | ||
mockFetch(samplePatch, 200); | ||
(execa: any).mockImplementation(mockExecaNested); | ||
((execa as unknown) as jest.Mock).mockImplementation(mockExecaNested); | ||
const config = {...ctx, root: '/project/root/NestedApp'}; | ||
await upgrade.func([newVersion], config, opts); | ||
await upgrade.func([newVersion], config); | ||
|
||
expect(execa).toBeCalledWith('git', [ | ||
'apply', | ||
|
@@ -141,33 +135,33 @@ test('applies patch in current working directory when nested', async () => { | |
}); | ||
|
||
test('errors when invalid version passed', async () => { | ||
await upgrade.func(['next'], ctx, opts); | ||
await upgrade.func(['next'], ctx); | ||
expect(logger.error).toBeCalledWith( | ||
'Provided version "next" is not allowed. Please pass a valid semver version', | ||
); | ||
}, 60000); | ||
|
||
test('errors when older version passed', async () => { | ||
await upgrade.func([olderVersion], ctx, opts); | ||
await upgrade.func([olderVersion], ctx); | ||
expect(logger.error).toBeCalledWith( | ||
`Trying to upgrade from newer version "${currentVersion}" to older "${olderVersion}"`, | ||
); | ||
await upgrade.func(['0.57.10'], ctx, opts); | ||
await upgrade.func(['0.57.10'], ctx); | ||
expect(logger.error).not.toBeCalledWith( | ||
`Trying to upgrade from newer version "${currentVersion}" to older "0.57.10"`, | ||
); | ||
}, 60000); | ||
|
||
test('warns when dependency upgrade version is in semver range', async () => { | ||
await upgrade.func([currentVersion], ctx, opts); | ||
await upgrade.func([currentVersion], ctx); | ||
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`, | ||
); | ||
}, 60000); | ||
|
||
test('fetches empty patch and installs deps', async () => { | ||
mockFetch(); | ||
await upgrade.func([newVersion], ctx, opts); | ||
await upgrade.func([newVersion], ctx); | ||
expect(flushOutput()).toMatchInlineSnapshot(` | ||
"info Fetching diff between v0.57.8 and v0.58.4... | ||
info Diff has no changes to apply, proceeding further | ||
|
@@ -192,7 +186,6 @@ test('fetches regular patch, adds remote, applies patch, installs deps, removes | |
android: {packageName: 'com.testapp'}, | ||
}, | ||
}), | ||
opts, | ||
); | ||
expect(flushOutput()).toMatchInlineSnapshot(` | ||
"info Fetching diff between v0.57.8 and v0.58.4... | ||
|
@@ -215,18 +208,22 @@ test('fetches regular patch, adds remote, applies patch, installs deps, removes | |
success Upgraded React Native to v0.58.4 🎉. Now you can review and commit the changes" | ||
`); | ||
expect( | ||
snapshotDiff(samplePatch, (fs.writeFileSync: any).mock.calls[0][1], { | ||
contextLines: 1, | ||
}), | ||
snapshotDiff( | ||
samplePatch, | ||
(fs.writeFileSync as jest.Mock).mock.calls[0][1], | ||
{ | ||
contextLines: 1, | ||
}, | ||
), | ||
).toMatchSnapshot( | ||
'RnDiffApp is replaced with app name (TestApp and com.testapp)', | ||
); | ||
}, 60000); | ||
test('fetches regular patch, adds remote, applies patch, installs deps, removes remote when updated from nested directory', async () => { | ||
mockFetch(samplePatch, 200); | ||
(execa: any).mockImplementation(mockExecaNested); | ||
((execa as unknown) as jest.Mock).mockImplementation(mockExecaNested); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I usually make it |
||
const config = {...ctx, root: '/project/root/NestedApp'}; | ||
await upgrade.func([newVersion], config, opts); | ||
await upgrade.func([newVersion], config); | ||
expect(flushOutput()).toMatchInlineSnapshot(` | ||
"info Fetching diff between v0.57.8 and v0.58.4... | ||
[fs] write tmp-upgrade-rn.patch | ||
|
@@ -250,7 +247,7 @@ test('fetches regular patch, adds remote, applies patch, installs deps, removes | |
}, 60000); | ||
test('cleans up if patching fails,', async () => { | ||
mockFetch(samplePatch, 200); | ||
(execa: any).mockImplementation((command, args) => { | ||
((execa as unknown) as jest.Mock).mockImplementation((command, args) => { | ||
mockPushLog('$', 'execa', command, args); | ||
if (command === 'npm' && args[3] === '--json') { | ||
return Promise.resolve({ | ||
|
@@ -270,7 +267,7 @@ test('cleans up if patching fails,', async () => { | |
return Promise.resolve({stdout: ''}); | ||
}); | ||
try { | ||
await upgrade.func([newVersion], ctx, opts); | ||
await upgrade.func([newVersion], ctx); | ||
} catch (error) { | ||
expect(error.message).toBe( | ||
'Upgrade failed. Please see the messages above for details', | ||
|
@@ -313,12 +310,15 @@ test('works with --name-ios and --name-android', async () => { | |
android: {packageName: 'co.uk.customandroid.app'}, | ||
}, | ||
}), | ||
opts, | ||
); | ||
expect( | ||
snapshotDiff(samplePatch, (fs.writeFileSync: any).mock.calls[0][1], { | ||
contextLines: 1, | ||
}), | ||
snapshotDiff( | ||
samplePatch, | ||
(fs.writeFileSync as jest.Mock).mock.calls[0][1], | ||
{ | ||
contextLines: 1, | ||
}, | ||
), | ||
).toMatchSnapshot( | ||
'RnDiffApp is replaced with app name (CustomIos and co.uk.customandroid.app)', | ||
); | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why we need it?
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.
tests are failing without this transform, not yet sure why