Skip to content
This repository was archived by the owner on Dec 10, 2021. It is now read-only.

Commit f02d176

Browse files
author
Christine Chambers
committed
fix: revert "Handle BigNumber conversions in JSON properly (without loss of precision) (#71)"
This reverts commit e386612.
1 parent 5aa383e commit f02d176

File tree

5 files changed

+4
-94
lines changed

5 files changed

+4
-94
lines changed

packages/superset-ui-connection/package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,11 @@
2626
},
2727
"homepage": "https://github.com/apache-superset/superset-ui#readme",
2828
"devDependencies": {
29-
"@types/fetch-mock": "^6.0.0",
3029
"fetch-mock": "^6.5.2",
3130
"node-fetch": "^2.2.0"
3231
},
3332
"dependencies": {
3433
"@babel/runtime": "^7.1.2",
35-
"json-bigint": "^0.3.0",
3634
"whatwg-fetch": "^2.0.4"
3735
},
3836
"publishConfig": {

packages/superset-ui-connection/src/callApi/parseResponse.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import JSONbig from 'json-bigint';
21
import { ParseMethod, SupersetClientResponse } from '../types';
32

43
function rejectIfNotOkay(response: Response): Promise<Response> {
@@ -7,15 +6,6 @@ function rejectIfNotOkay(response: Response): Promise<Response> {
76
return Promise.resolve<Response>(response);
87
}
98

10-
function parseJson(text: string): any {
11-
try {
12-
return JSONbig.parse(text);
13-
} catch (e) {
14-
// if JSONbig.parse fails, it throws an object (not a proper Error), so let's re-wrap the message.
15-
throw new Error(e.message);
16-
}
17-
}
18-
199
export default function parseResponse(
2010
apiPromise: Promise<Response>,
2111
parseMethod: ParseMethod = 'json',
@@ -27,9 +17,7 @@ export default function parseResponse(
2717
} else if (parseMethod === 'text') {
2818
return checkedPromise.then(response => response.text().then(text => ({ response, text })));
2919
} else if (parseMethod === 'json') {
30-
return checkedPromise.then(response =>
31-
response.text().then(text => ({ json: parseJson(text), response })),
32-
);
20+
return checkedPromise.then(response => response.json().then(json => ({ json, response })));
3321
}
3422

3523
throw new Error(`Expected parseResponse=null|json|text, got '${parseMethod}'.`);

packages/superset-ui-connection/test/SupersetClientClass.test.ts

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,8 @@
11
import fetchMock from 'fetch-mock';
2-
3-
import { BigNumber } from 'bignumber.js';
42
import { SupersetClientClass, ClientConfig } from '../src';
53
import throwIfCalled from './utils/throwIfCalled';
64
import { LOGIN_GLOB } from './fixtures/constants';
75

8-
/* NOTE: We're using fetchMock v6.5.2, but corresponding fetchMock type declaration files are only available for v6.0.2
9-
* and v7+. It looks like there are behavior changes between v6 and v7 that break our tests, so upgrading to v7 is
10-
* probably some work.
11-
*
12-
* To avoid this, we're using the type declarations for v6.0.2, but there is at least one API inconsistency between that
13-
* type declaration file and the actual library we're using. It looks like `sendAsJson` was added sometime after that
14-
* release, or else the type declaration file isn't completely accurate. To get around this, it's necessary to add
15-
* a `@ts-ignore` decorator before references to `sendAsJson` (there's one instance of that in this file).
16-
*
17-
* The **right** solution is probably to upgrade to fetchMock v7 (and the latest type declaration) and fix the tests
18-
* that become broken as a result.
19-
*/
206
describe('SupersetClientClass', () => {
217
beforeAll(() => {
228
fetchMock.get(LOGIN_GLOB, { csrf_token: '' });
@@ -138,8 +124,6 @@ describe('SupersetClientClass', () => {
138124
it('does not set csrfToken if response is not json', () => {
139125
fetchMock.get(LOGIN_GLOB, '123', {
140126
overwriteRoutes: true,
141-
// @TODO remove once fetchMock is upgraded to 7+, see note at top of this file
142-
// @ts-ignore
143127
sendAsJson: false,
144128
});
145129

@@ -267,8 +251,7 @@ describe('SupersetClientClass', () => {
267251
const mockTextUrl = `${protocol}//${host}${mockTextEndpoint}`;
268252
const mockPutUrl = `${protocol}//${host}${mockPutEndpoint}`;
269253
const mockDeleteUrl = `${protocol}//${host}${mockDeleteEndpoint}`;
270-
const mockBigNumber = '9223372036854775807';
271-
const mockTextJsonResponse = `{ "value": ${mockBigNumber} }`;
254+
const mockTextJsonResponse = '{ "value": 9223372036854775807 }';
272255

273256
fetchMock.get(mockGetUrl, { json: 'payload' });
274257
fetchMock.post(mockPostUrl, { json: 'payload' });
@@ -347,21 +330,6 @@ describe('SupersetClientClass', () => {
347330
);
348331
});
349332

350-
it('supports parsing a response as JSON while preserving precision of large numbers', () => {
351-
expect.assertions(2);
352-
const client = new SupersetClientClass({ protocol, host });
353-
354-
return client.init().then(() =>
355-
client.get({ url: mockTextUrl }).then(({ json }) => {
356-
expect(fetchMock.calls(mockTextUrl)).toHaveLength(1);
357-
// @ts-ignore
358-
expect(json.value.toString()).toBe(new BigNumber(mockBigNumber).toString());
359-
360-
return Promise.resolve();
361-
}),
362-
);
363-
});
364-
365333
it('supports parsing a response as text', () => {
366334
expect.assertions(2);
367335
const client = new SupersetClientClass({ protocol, host });
@@ -471,21 +439,6 @@ describe('SupersetClientClass', () => {
471439
);
472440
});
473441

474-
it('supports parsing a response as JSON while preserving precision of large numbers', () => {
475-
expect.assertions(2);
476-
const client = new SupersetClientClass({ protocol, host });
477-
478-
return client.init().then(() =>
479-
client.post({ url: mockTextUrl }).then(({ json }) => {
480-
expect(fetchMock.calls(mockTextUrl)).toHaveLength(1);
481-
// @ts-ignore
482-
expect(json.value.toString()).toBe(new BigNumber(mockBigNumber).toString());
483-
484-
return Promise.resolve();
485-
}),
486-
);
487-
});
488-
489442
it('supports parsing a response as text', () => {
490443
expect.assertions(2);
491444
const client = new SupersetClientClass({ protocol, host });

packages/superset-ui-connection/test/callApi/parseResponse.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ describe('parseResponse()', () => {
6363
.catch(error => {
6464
expect(fetchMock.calls(mockTextUrl)).toHaveLength(1);
6565
expect(error.stack).toBeDefined();
66-
expect(error.message.includes('Unexpected')).toBe(true);
66+
expect(error.message.includes('Unexpected token')).toBe(true);
6767

6868
return Promise.resolve();
6969
});

types/external.d.ts

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1 @@
1-
declare module 'json-bigint' {
2-
interface JSONbig {
3-
/**
4-
* Converts a JavaScript Object Notation (JSON) string into an object, preserving precision for numeric values.
5-
* @param text A valid JSON string.
6-
* @param reviver A function that transforms the results. This function is called for each member of the object.
7-
* If a member contains nested objects, the nested objects are transformed before the parent object is.
8-
*/
9-
parse(text: string, reviver?: (key: any, value: any) => any): any;
10-
11-
/**
12-
* Converts a JavaScript value to a JavaScript Object Notation (JSON) string, preserving precision for numeric values.
13-
* @param value A JavaScript value, usually an object or array, to be converted.
14-
* @param replacer A function that transforms the results, or an array of strings and numbers that acts
15-
* as a approved list for selecting the object properties that will be stringified.
16-
* @param space Adds indentation, white space, and line break characters to the return-value JSON text to make it easier to read.
17-
*/
18-
stringify(
19-
value: any,
20-
replacer?: (number | string)[] | null | ((key: string, value: any) => any),
21-
space?: string | number,
22-
): string;
23-
}
24-
25-
/**
26-
* An intrinsic object that provides functions to convert JavaScript values to and from the JavaScript Object Notation (JSON) format.
27-
*/
28-
const JSONbig: JSONbig;
29-
export = JSONbig;
30-
}
1+
declare module 'fetch-mock';

0 commit comments

Comments
 (0)