From 834be962c5b6b1876e8984db7115fe4eceab240d Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Wed, 3 Jul 2019 16:30:23 +0200 Subject: [PATCH 01/11] imp: standarize fetch in CLI --- .../init/__tests__/templateName.test.js | 18 +++--- .../cli/src/commands/init/templateName.js | 10 +++- .../upgrade/__tests__/upgrade.test.js | 28 +++++---- packages/cli/src/commands/upgrade/upgrade.js | 12 +++- .../tools/releaseChecker/getLatestRelease.js | 57 ++++++++++--------- packages/tools/src/fetch.ts | 12 ++++ packages/tools/src/index.ts | 1 + packages/tools/src/isPackagerRunning.ts | 2 +- 8 files changed, 88 insertions(+), 52 deletions(-) create mode 100644 packages/tools/src/fetch.ts diff --git a/packages/cli/src/commands/init/__tests__/templateName.test.js b/packages/cli/src/commands/init/__tests__/templateName.test.js index db972581b..34dc7fa35 100644 --- a/packages/cli/src/commands/init/__tests__/templateName.test.js +++ b/packages/cli/src/commands/init/__tests__/templateName.test.js @@ -1,12 +1,18 @@ // @flow import {processTemplateName} from '../templateName'; -import {fetch} from '../../../tools/fetch'; +import {fetch} from '@react-native-community/cli-tools'; -jest.mock('../../../tools/fetch', () => ({fetch: jest.fn()})); +jest.mock('@react-native-community/cli-tools', () => ({fetch: jest.fn()})); const RN_NPM_PACKAGE = 'react-native'; const ABS_RN_PATH = '/path/to/react-native'; +const mockFetch = (value: any, status: number) => { + (fetch: any).mockImplementation(() => + Promise.resolve({json: () => Promise.resolve(value), status}), + ); +}; + test('supports file protocol with absolute path', async () => { jest.mock( `${ABS_RN_PATH}/package.json`, @@ -23,9 +29,7 @@ test('supports file protocol with absolute path', async () => { test('supports shorthand templates', async () => { const templateName = 'typescript'; - (fetch: any).mockImplementationOnce(() => { - return Promise.resolve(`{"name": "react-native-template-${templateName}"}`); - }); + mockFetch({name: 'react-native-template-${templateName}'}, 200); expect(await processTemplateName(templateName)).toEqual({ uri: `react-native-template-${templateName}`, name: `react-native-template-${templateName}`, @@ -34,9 +38,7 @@ test('supports shorthand templates', async () => { test('supports not-found shorthand templates', async () => { const templateName = 'typescriptz'; - (fetch: any).mockImplementationOnce(() => { - return Promise.resolve('Not found'); - }); + mockFetch('Not found', 200); expect(await processTemplateName(templateName)).toEqual({ uri: templateName, name: templateName, diff --git a/packages/cli/src/commands/init/templateName.js b/packages/cli/src/commands/init/templateName.js index d2b8b7e98..901f6d05a 100644 --- a/packages/cli/src/commands/init/templateName.js +++ b/packages/cli/src/commands/init/templateName.js @@ -1,7 +1,7 @@ // @flow import path from 'path'; import {URL} from 'url'; -import {fetch} from '../../tools/fetch'; +import {fetch} from '@react-native-community/cli-tools'; const FILE_PROTOCOL = /file:/; const HTTP_PROTOCOL = /https?:/; @@ -81,7 +81,13 @@ async function tryTemplateShorthand(templateName: string) { `https://registry.yarnpkg.com/${reactNativeTemplatePackage}/latest`, ); - if (JSON.parse(response).name) { + if (response.status < 200 || response.status > 299) { + throw new Error(`Failed to load page, status code: ${response.status}`); + } + + const body = await response.json(); + + if (body.name) { return reactNativeTemplatePackage; } } catch (e) { diff --git a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js index 9f5e11826..4a4857722 100644 --- a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js +++ b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js @@ -5,7 +5,7 @@ import fs from 'fs'; import snapshotDiff from 'snapshot-diff'; import stripAnsi from 'strip-ansi'; import upgrade from '../upgrade'; -import {fetch} from '../../../tools/fetch'; +import {fetch} from '@react-native-community/cli-tools'; import logger from '../../../tools/logger'; import loadConfig from '../../../tools/config'; import merge from '../../../tools/merge'; @@ -43,11 +43,11 @@ jest.mock('../../../tools/packageManager', () => ({ mockPushLog('$ yarn add', ...args); }, })); -jest.mock('../../../tools/fetch', () => ({ - fetch: jest.fn(() => Promise.resolve('patch')), -})); jest.mock('@react-native-community/cli-tools', () => ({ ...jest.requireActual('@react-native-community/cli-tools'), + fetch: jest.fn(() => + Promise.resolve({json: () => Promise.resolve('patch'), status: 200}), + ), logger: { info: jest.fn((...args) => mockPushLog('info', args)), error: jest.fn((...args) => mockPushLog('error', args)), @@ -58,6 +58,12 @@ jest.mock('@react-native-community/cli-tools', () => ({ }, })); +const mockFetch = (value, status) => { + (fetch: any).mockImplementation(() => + Promise.resolve({json: () => Promise.resolve(value), status}), + ); +}; + const mockExecaDefault = (command, args) => { mockPushLog('$', 'execa', command, args); if (command === 'npm' && args[3] === '--json') { @@ -121,7 +127,7 @@ test('uses latest version of react-native when none passed', async () => { }, 60000); test('applies patch in current working directory when nested', async () => { - (fetch: any).mockImplementation(() => Promise.resolve(samplePatch)); + mockFetch(samplePatch, 200); (execa: any).mockImplementation(mockExecaNested); const config = {...ctx, root: '/project/root/NestedApp'}; await upgrade.func([newVersion], config, opts); @@ -162,7 +168,7 @@ test('warns when dependency upgrade version is in semver range', async () => { }, 60000); test('fetches empty patch and installs deps', async () => { - (fetch: any).mockImplementation(() => Promise.resolve('')); + mockFetch('', 200); await upgrade.func([newVersion], ctx, opts); expect(flushOutput()).toMatchInlineSnapshot(` "info Fetching diff between v0.57.8 and v0.58.4... @@ -178,7 +184,9 @@ test('fetches empty patch and installs deps', async () => { }, 60000); test('fetches regular patch, adds remote, applies patch, installs deps, removes remote,', async () => { - (fetch: any).mockImplementation(() => Promise.resolve(samplePatch)); + (fetch: any).mockImplementation(() => + Promise.resolve({json: () => Promise.resolve(samplePatch), status: 200}), + ); await upgrade.func( [newVersion], merge(ctx, { @@ -217,7 +225,7 @@ test('fetches regular patch, adds remote, applies patch, installs deps, removes ); }, 60000); test('fetches regular patch, adds remote, applies patch, installs deps, removes remote when updated from nested directory', async () => { - (fetch: any).mockImplementation(() => Promise.resolve(samplePatch)); + mockFetch(samplePatch, 200); (execa: any).mockImplementation(mockExecaNested); const config = {...ctx, root: '/project/root/NestedApp'}; await upgrade.func([newVersion], config, opts); @@ -242,7 +250,7 @@ test('fetches regular patch, adds remote, applies patch, installs deps, removes `); }, 60000); test('cleans up if patching fails,', async () => { - (fetch: any).mockImplementation(() => Promise.resolve(samplePatch)); + mockFetch(samplePatch, 200); (execa: any).mockImplementation((command, args) => { mockPushLog('$', 'execa', command, args); if (command === 'npm' && args[3] === '--json') { @@ -296,7 +304,7 @@ test('cleans up if patching fails,', async () => { `); }, 60000); test('works with --name-ios and --name-android', async () => { - (fetch: any).mockImplementation(() => Promise.resolve(samplePatch)); + mockFetch(samplePatch, 200); await upgrade.func( [newVersion], merge(ctx, { diff --git a/packages/cli/src/commands/upgrade/upgrade.js b/packages/cli/src/commands/upgrade/upgrade.js index d3cbb5085..4e0818d8c 100644 --- a/packages/cli/src/commands/upgrade/upgrade.js +++ b/packages/cli/src/commands/upgrade/upgrade.js @@ -5,9 +5,8 @@ import chalk from 'chalk'; import semver from 'semver'; import execa from 'execa'; import type {ConfigT} from 'types'; -import {logger, CLIError} from '@react-native-community/cli-tools'; +import {logger, CLIError, fetch} from '@react-native-community/cli-tools'; import * as PackageManager from '../../tools/packageManager'; -import {fetch} from '../../tools/fetch'; import legacyUpgrade from './legacyUpgrade'; type FlagsT = { @@ -46,7 +45,14 @@ const getPatch = async (currentVersion, newVersion, config) => { logger.info(`Fetching diff between v${currentVersion} and v${newVersion}...`); try { - patch = await fetch(`${rawDiffUrl}/${currentVersion}..${newVersion}.diff`); + const response = await fetch( + `${rawDiffUrl}/${currentVersion}..${newVersion}.diff`, + ); + + if (response.status < 200 || response.status > 299) { + throw new Error(`Failed to load page, status code: ${response.status}`); + } + patch = await response.json(); } catch (error) { logger.error( `Failed to fetch diff for react-native@${newVersion}. Maybe it's not released yet?`, diff --git a/packages/cli/src/tools/releaseChecker/getLatestRelease.js b/packages/cli/src/tools/releaseChecker/getLatestRelease.js index 7f41a2304..03b4d996e 100644 --- a/packages/cli/src/tools/releaseChecker/getLatestRelease.js +++ b/packages/cli/src/tools/releaseChecker/getLatestRelease.js @@ -1,10 +1,10 @@ /** * @flow */ -import https from 'https'; import semver from 'semver'; import logger from '../logger'; import cacheManager from './releaseCacheManager'; +import {fetch} from '@react-native-community/cli-tools'; export type Release = { version: string, @@ -81,11 +81,12 @@ async function getLatestRnDiffPurgeVersion(name: string, eTag: ?string) { options.headers['If-None-Match'] = eTag; } - const response = await httpsGet(options); + const response = await fetch(options); // Remote is newer. - if (response.statusCode === 200) { - const latestVersion = JSON.parse(response.body)[0].name.substring(8); + if (response.status === 200) { + const body: Array = await response.json(); + const latestVersion = body[0].name.substring(8); // Update cache only if newer release is stable. if (!semver.prerelease(latestVersion)) { @@ -114,27 +115,27 @@ type Headers = { [header: string]: mixed, }; -function httpsGet(options) { - return new Promise((resolve, reject) => { - https - .get(options, result => { - let body = ''; - - result.setEncoding('utf8'); - result.on('data', data => { - body += data; - }); - - result.on('end', () => { - resolve({ - body, - eTag: result.headers.etag, - statusCode: result.statusCode, - }); - }); - - result.on('error', error => reject(error)); - }) - .on('error', error => reject(error)); - }); -} +// function httpsGet(options) { +// return new Promise((resolve, reject) => { +// https +// .get(options, result => { +// let body = ''; + +// result.setEncoding('utf8'); +// result.on('data', data => { +// body += data; +// }); + +// result.on('end', () => { +// resolve({ +// body, +// eTag: result.headers.etag, +// statusCode: result.statusCode, +// }); +// }); + +// result.on('error', error => reject(error)); +// }) +// .on('error', error => reject(error)); +// }); +// } diff --git a/packages/tools/src/fetch.ts b/packages/tools/src/fetch.ts new file mode 100644 index 000000000..1f9e7bc56 --- /dev/null +++ b/packages/tools/src/fetch.ts @@ -0,0 +1,12 @@ +import nodeFetch, { + RequestInit as FetchOptions, + Response, + Request, +} from 'node-fetch'; + +export default function fetch( + url: string | Request, + options?: FetchOptions, +): Promise { + return nodeFetch(url, options); +} diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index d99e20bf0..0a6f4fafa 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -2,5 +2,6 @@ export {default as logger} from './logger'; export {default as groupFilesByType} from './groupFilesByType'; export {default as isPackagerRunning} from './isPackagerRunning'; export {default as getDefaultUserTerminal} from './getDefaultUserTerminal'; +export {default as fetch} from './fetch'; export * from './errors'; diff --git a/packages/tools/src/isPackagerRunning.ts b/packages/tools/src/isPackagerRunning.ts index 11b276368..b2b888005 100644 --- a/packages/tools/src/isPackagerRunning.ts +++ b/packages/tools/src/isPackagerRunning.ts @@ -6,7 +6,7 @@ * */ -import fetch from 'node-fetch'; +import fetch from './fetch'; /** * Indicates whether or not the packager is running. It returns a promise that From 693573c2a67e0fe7565e2a221a5f6333f65958f4 Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Wed, 3 Jul 2019 16:34:47 +0200 Subject: [PATCH 02/11] imp: remove commented out function --- .../tools/releaseChecker/getLatestRelease.js | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/packages/cli/src/tools/releaseChecker/getLatestRelease.js b/packages/cli/src/tools/releaseChecker/getLatestRelease.js index 03b4d996e..d4cf005b7 100644 --- a/packages/cli/src/tools/releaseChecker/getLatestRelease.js +++ b/packages/cli/src/tools/releaseChecker/getLatestRelease.js @@ -114,28 +114,3 @@ type Headers = { 'User-Agent': mixed, [header: string]: mixed, }; - -// function httpsGet(options) { -// return new Promise((resolve, reject) => { -// https -// .get(options, result => { -// let body = ''; - -// result.setEncoding('utf8'); -// result.on('data', data => { -// body += data; -// }); - -// result.on('end', () => { -// resolve({ -// body, -// eTag: result.headers.etag, -// statusCode: result.statusCode, -// }); -// }); - -// result.on('error', error => reject(error)); -// }) -// .on('error', error => reject(error)); -// }); -// } From 4550995eaeed5750fef28f568fe797ff5285a03d Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Tue, 16 Jul 2019 17:36:04 +0200 Subject: [PATCH 03/11] Change fetch a little bit --- .../init/__tests__/templateName.test.js | 2 -- .../upgrade/__tests__/upgrade.test.js | 12 +++-------- packages/cli/src/commands/upgrade/upgrade.js | 8 +++---- .../tools/releaseChecker/getLatestRelease.js | 14 +++++++------ packages/tools/src/fetch.ts | 21 ++++++++++++++++--- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/packages/cli/src/commands/init/__tests__/templateName.test.js b/packages/cli/src/commands/init/__tests__/templateName.test.js index 748849e4f..d374637dc 100644 --- a/packages/cli/src/commands/init/__tests__/templateName.test.js +++ b/packages/cli/src/commands/init/__tests__/templateName.test.js @@ -1,8 +1,6 @@ // @flow import {processTemplateName} from '../templateName'; -jest.mock('@react-native-community/cli-tools', () => ({fetch: jest.fn()})); - const RN_NPM_PACKAGE = 'react-native'; const ABS_RN_PATH = '/path/to/react-native'; diff --git a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js index 4a4857722..f55215f7a 100644 --- a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js +++ b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js @@ -45,9 +45,7 @@ jest.mock('../../../tools/packageManager', () => ({ })); jest.mock('@react-native-community/cli-tools', () => ({ ...jest.requireActual('@react-native-community/cli-tools'), - fetch: jest.fn(() => - Promise.resolve({json: () => Promise.resolve('patch'), status: 200}), - ), + fetch: jest.fn(() => Promise.resolve({data: 'patch', status: 200})), logger: { info: jest.fn((...args) => mockPushLog('info', args)), error: jest.fn((...args) => mockPushLog('error', args)), @@ -59,9 +57,7 @@ jest.mock('@react-native-community/cli-tools', () => ({ })); const mockFetch = (value, status) => { - (fetch: any).mockImplementation(() => - Promise.resolve({json: () => Promise.resolve(value), status}), - ); + (fetch: any).mockImplementation(() => Promise.resolve({data: value, status})); }; const mockExecaDefault = (command, args) => { @@ -184,9 +180,7 @@ test('fetches empty patch and installs deps', async () => { }, 60000); test('fetches regular patch, adds remote, applies patch, installs deps, removes remote,', async () => { - (fetch: any).mockImplementation(() => - Promise.resolve({json: () => Promise.resolve(samplePatch), status: 200}), - ); + mockFetch(samplePatch, 200); await upgrade.func( [newVersion], merge(ctx, { diff --git a/packages/cli/src/commands/upgrade/upgrade.js b/packages/cli/src/commands/upgrade/upgrade.js index 4e0818d8c..3beee1522 100644 --- a/packages/cli/src/commands/upgrade/upgrade.js +++ b/packages/cli/src/commands/upgrade/upgrade.js @@ -45,14 +45,14 @@ const getPatch = async (currentVersion, newVersion, config) => { logger.info(`Fetching diff between v${currentVersion} and v${newVersion}...`); try { - const response = await fetch( + const {status, data} = await fetch( `${rawDiffUrl}/${currentVersion}..${newVersion}.diff`, ); - if (response.status < 200 || response.status > 299) { - throw new Error(`Failed to load page, status code: ${response.status}`); + if (status < 200 || status > 299) { + throw new Error(`Failed to load page, status code: ${status}`); } - patch = await response.json(); + patch = data; } catch (error) { logger.error( `Failed to fetch diff for react-native@${newVersion}. Maybe it's not released yet?`, diff --git a/packages/cli/src/tools/releaseChecker/getLatestRelease.js b/packages/cli/src/tools/releaseChecker/getLatestRelease.js index d4cf005b7..e13bae679 100644 --- a/packages/cli/src/tools/releaseChecker/getLatestRelease.js +++ b/packages/cli/src/tools/releaseChecker/getLatestRelease.js @@ -81,17 +81,19 @@ async function getLatestRnDiffPurgeVersion(name: string, eTag: ?string) { options.headers['If-None-Match'] = eTag; } - const response = await fetch(options); + const {data, status, headers} = await fetch(options); // Remote is newer. - if (response.status === 200) { - const body: Array = await response.json(); + if (status === 200) { + const body: Array = data; const latestVersion = body[0].name.substring(8); // Update cache only if newer release is stable. if (!semver.prerelease(latestVersion)) { - logger.debug(`Saving ${response.eTag} to cache`); - cacheManager.set(name, 'eTag', response.eTag); + const eTagHeader = headers.get('eTag'); + + logger.debug(`Saving ${eTagHeader} to cache`); + cacheManager.set(name, 'eTag', eTagHeader); cacheManager.set(name, 'latestVersion', latestVersion); } @@ -99,7 +101,7 @@ async function getLatestRnDiffPurgeVersion(name: string, eTag: ?string) { } // Cache is still valid. - if (response.statusCode === 304) { + if (status === 304) { const latestVersion = cacheManager.get(name, 'latestVersion'); if (latestVersion) { return latestVersion; diff --git a/packages/tools/src/fetch.ts b/packages/tools/src/fetch.ts index 1f9e7bc56..9b793d304 100644 --- a/packages/tools/src/fetch.ts +++ b/packages/tools/src/fetch.ts @@ -2,11 +2,26 @@ import nodeFetch, { RequestInit as FetchOptions, Response, Request, + Headers, } from 'node-fetch'; -export default function fetch( +async function unwrapFetchResult(response: Response) { + try { + return response.json(); + } catch (e) { + return response.text(); + } +} + +export default async function fetch( url: string | Request, options?: FetchOptions, -): Promise { - return nodeFetch(url, options); +): Promise<{status: number, data: any, headers: Headers}> { + const result = await nodeFetch(url, options); + + return { + status: result.status, + headers: result.headers, + data: await unwrapFetchResult(result), + }; } From 911c986b34f113c7e770cfdfe1e2dde30c2f05a7 Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Tue, 16 Jul 2019 17:44:32 +0200 Subject: [PATCH 04/11] Fix isPackagerRunning --- packages/tools/src/isPackagerRunning.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/tools/src/isPackagerRunning.ts b/packages/tools/src/isPackagerRunning.ts index b2b888005..ad97fb9ca 100644 --- a/packages/tools/src/isPackagerRunning.ts +++ b/packages/tools/src/isPackagerRunning.ts @@ -19,10 +19,9 @@ async function isPackagerRunning( packagerPort: string = process.env.RCT_METRO_PORT || '8081', ): Promise<'running' | 'not_running' | 'unrecognized'> { try { - const result = await fetch(`http://localhost:${packagerPort}/status`); - const body = await result.text(); + const {data} = await fetch(`http://localhost:${packagerPort}/status`); - return body === 'packager-status:running' ? 'running' : 'unrecognized'; + return data === 'packager-status:running' ? 'running' : 'unrecognized'; } catch (_error) { return 'not_running'; } From 2bf2da520ab706bace120959edc51e246448ec89 Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Tue, 16 Jul 2019 17:54:43 +0200 Subject: [PATCH 05/11] Improve mocking --- jest/setupUnitTests.js | 1 + packages/cli/src/commands/upgrade/__tests__/upgrade.test.js | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/jest/setupUnitTests.js b/jest/setupUnitTests.js index a6ea7c372..a48825526 100644 --- a/jest/setupUnitTests.js +++ b/jest/setupUnitTests.js @@ -5,6 +5,7 @@ jest.mock('@react-native-community/cli-tools', () => { return { ...jest.requireActual('@react-native-community/cli-tools'), + fetch: jest.fn(), logger: { success: jest.fn(), info: jest.fn(), diff --git a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js index f55215f7a..064a7669a 100644 --- a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js +++ b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js @@ -45,7 +45,7 @@ jest.mock('../../../tools/packageManager', () => ({ })); jest.mock('@react-native-community/cli-tools', () => ({ ...jest.requireActual('@react-native-community/cli-tools'), - fetch: jest.fn(() => Promise.resolve({data: 'patch', status: 200})), + fetch: jest.fn(), logger: { info: jest.fn((...args) => mockPushLog('info', args)), error: jest.fn((...args) => mockPushLog('error', args)), @@ -56,7 +56,7 @@ jest.mock('@react-native-community/cli-tools', () => ({ }, })); -const mockFetch = (value, status) => { +const mockFetch = (value = '', status = 200) => { (fetch: any).mockImplementation(() => Promise.resolve({data: value, status})); }; @@ -164,7 +164,7 @@ test('warns when dependency upgrade version is in semver range', async () => { }, 60000); test('fetches empty patch and installs deps', async () => { - mockFetch('', 200); + mockFetch(); await upgrade.func([newVersion], ctx, opts); expect(flushOutput()).toMatchInlineSnapshot(` "info Fetching diff between v0.57.8 and v0.58.4... From 834c15c72d35de1e1e52bab7db36faf8435dc2f7 Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Tue, 16 Jul 2019 18:03:56 +0200 Subject: [PATCH 06/11] Fix release checker --- packages/cli/src/tools/releaseChecker/getLatestRelease.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/tools/releaseChecker/getLatestRelease.js b/packages/cli/src/tools/releaseChecker/getLatestRelease.js index e13bae679..d3980e8a2 100644 --- a/packages/cli/src/tools/releaseChecker/getLatestRelease.js +++ b/packages/cli/src/tools/releaseChecker/getLatestRelease.js @@ -71,8 +71,6 @@ function buildChangelogUrl(version: string) { */ async function getLatestRnDiffPurgeVersion(name: string, eTag: ?string) { const options = { - hostname: 'api.github.com', - path: '/repos/react-native-community/rn-diff-purge/tags', // https://developer.github.com/v3/#user-agent-required headers: ({'User-Agent': 'React-Native-CLI'}: Headers), }; @@ -81,7 +79,10 @@ async function getLatestRnDiffPurgeVersion(name: string, eTag: ?string) { options.headers['If-None-Match'] = eTag; } - const {data, status, headers} = await fetch(options); + const {data, status, headers} = await fetch( + 'https://api.github.com/repos/react-native-community/rn-diff-purge/tags', + options, + ); // Remote is newer. if (status === 200) { From 18900546368583bf526ebedb1264d27a15517018 Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Tue, 16 Jul 2019 18:32:05 +0200 Subject: [PATCH 07/11] Fix upgrade --- packages/cli/src/commands/upgrade/upgrade.js | 3 ++- packages/cli/src/tools/fetch.js | 17 ----------------- packages/tools/src/fetch.ts | 9 ++++++--- 3 files changed, 8 insertions(+), 21 deletions(-) delete mode 100644 packages/cli/src/tools/fetch.js diff --git a/packages/cli/src/commands/upgrade/upgrade.js b/packages/cli/src/commands/upgrade/upgrade.js index 3beee1522..609c423ec 100644 --- a/packages/cli/src/commands/upgrade/upgrade.js +++ b/packages/cli/src/commands/upgrade/upgrade.js @@ -50,10 +50,11 @@ const getPatch = async (currentVersion, newVersion, config) => { ); if (status < 200 || status > 299) { - throw new Error(`Failed to load page, status code: ${status}`); + throw new CLIError(`Failed to get diff, status code: ${status}`); } patch = data; } catch (error) { + logger.error(error); logger.error( `Failed to fetch diff for react-native@${newVersion}. Maybe it's not released yet?`, ); diff --git a/packages/cli/src/tools/fetch.js b/packages/cli/src/tools/fetch.js deleted file mode 100644 index 5c9ad2fc2..000000000 --- a/packages/cli/src/tools/fetch.js +++ /dev/null @@ -1,17 +0,0 @@ -// @flow -import https from 'https'; - -export const fetch = (url: string) => - new Promise((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)); - }); diff --git a/packages/tools/src/fetch.ts b/packages/tools/src/fetch.ts index 9b793d304..d2afbbbbc 100644 --- a/packages/tools/src/fetch.ts +++ b/packages/tools/src/fetch.ts @@ -6,10 +6,12 @@ import nodeFetch, { } from 'node-fetch'; async function unwrapFetchResult(response: Response) { + const data = await response.text(); + try { - return response.json(); + return JSON.stringify(data); } catch (e) { - return response.text(); + return data; } } @@ -18,10 +20,11 @@ export default async function fetch( options?: FetchOptions, ): Promise<{status: number, data: any, headers: Headers}> { const result = await nodeFetch(url, options); + const data = await unwrapFetchResult(result); return { status: result.status, headers: result.headers, - data: await unwrapFetchResult(result), + data, }; } From da3e461b7f0d3b493bc98fed6df3feb65fd441b4 Mon Sep 17 00:00:00 2001 From: Kacper Wiszczuk Date: Tue, 16 Jul 2019 18:42:27 +0200 Subject: [PATCH 08/11] Handle status codes inside fetch --- packages/cli/src/commands/upgrade/upgrade.js | 5 +---- packages/tools/src/fetch.ts | 5 +++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/upgrade/upgrade.js b/packages/cli/src/commands/upgrade/upgrade.js index 609c423ec..15afbf3a1 100644 --- a/packages/cli/src/commands/upgrade/upgrade.js +++ b/packages/cli/src/commands/upgrade/upgrade.js @@ -45,13 +45,10 @@ const getPatch = async (currentVersion, newVersion, config) => { logger.info(`Fetching diff between v${currentVersion} and v${newVersion}...`); try { - const {status, data} = await fetch( + const {data} = await fetch( `${rawDiffUrl}/${currentVersion}..${newVersion}.diff`, ); - if (status < 200 || status > 299) { - throw new CLIError(`Failed to get diff, status code: ${status}`); - } patch = data; } catch (error) { logger.error(error); diff --git a/packages/tools/src/fetch.ts b/packages/tools/src/fetch.ts index d2afbbbbc..c8cc62c95 100644 --- a/packages/tools/src/fetch.ts +++ b/packages/tools/src/fetch.ts @@ -4,6 +4,7 @@ import nodeFetch, { Request, Headers, } from 'node-fetch'; +import {CLIError} from './errors'; async function unwrapFetchResult(response: Response) { const data = await response.text(); @@ -22,6 +23,10 @@ export default async function fetch( const result = await nodeFetch(url, options); const data = await unwrapFetchResult(result); + if (result.status >= 400) { + throw new CLIError(`Fetch request failed with status ${result.status}`); + } + return { status: result.status, headers: result.headers, From 704920f6b99d1a19aab19fe67dec7a232b0c9f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 16 Jul 2019 20:34:48 +0200 Subject: [PATCH 09/11] use logger from cli-tools --- packages/cli/src/commands/upgrade/__tests__/upgrade.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js index 064a7669a..af689b03e 100644 --- a/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js +++ b/packages/cli/src/commands/upgrade/__tests__/upgrade.test.js @@ -5,8 +5,7 @@ import fs from 'fs'; import snapshotDiff from 'snapshot-diff'; import stripAnsi from 'strip-ansi'; import upgrade from '../upgrade'; -import {fetch} from '@react-native-community/cli-tools'; -import logger from '../../../tools/logger'; +import {fetch, logger} from '@react-native-community/cli-tools'; import loadConfig from '../../../tools/config'; import merge from '../../../tools/merge'; From 8296696405d861b02d1626f06422467bd4e9109f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 16 Jul 2019 20:45:51 +0200 Subject: [PATCH 10/11] add more details to fetch failure --- packages/cli/src/commands/upgrade/upgrade.js | 2 +- packages/tools/src/fetch.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/upgrade/upgrade.js b/packages/cli/src/commands/upgrade/upgrade.js index 15afbf3a1..b20b788e9 100644 --- a/packages/cli/src/commands/upgrade/upgrade.js +++ b/packages/cli/src/commands/upgrade/upgrade.js @@ -51,7 +51,7 @@ const getPatch = async (currentVersion, newVersion, config) => { patch = data; } catch (error) { - logger.error(error); + logger.error(error.message); logger.error( `Failed to fetch diff for react-native@${newVersion}. Maybe it's not released yet?`, ); diff --git a/packages/tools/src/fetch.ts b/packages/tools/src/fetch.ts index c8cc62c95..098ab0d6d 100644 --- a/packages/tools/src/fetch.ts +++ b/packages/tools/src/fetch.ts @@ -24,7 +24,9 @@ export default async function fetch( const data = await unwrapFetchResult(result); if (result.status >= 400) { - throw new CLIError(`Fetch request failed with status ${result.status}`); + throw new CLIError( + `Fetch request failed with status ${result.status}: ${data}.`, + ); } return { From 6a2331498ac98a02263ac58b96beff65f86acbe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 16 Jul 2019 20:48:06 +0200 Subject: [PATCH 11/11] parse text repsonse --- packages/tools/src/fetch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tools/src/fetch.ts b/packages/tools/src/fetch.ts index 098ab0d6d..73cc8cdd5 100644 --- a/packages/tools/src/fetch.ts +++ b/packages/tools/src/fetch.ts @@ -10,7 +10,7 @@ async function unwrapFetchResult(response: Response) { const data = await response.text(); try { - return JSON.stringify(data); + return JSON.parse(data); } catch (e) { return data; }