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

Commit 7f49eb2

Browse files
author
Christine Chambers
committed
Revert "Handle BigNumber conversions in JSON properly (without loss of precision) (#71)"
This reverts commit e386612.
1 parent d1d1e34 commit 7f49eb2

File tree

7 files changed

+25
-111
lines changed

7 files changed

+25
-111
lines changed

packages/superset-ui-chart/src/components/SuperChart.tsx

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,21 @@ export default class SuperChart extends React.PureComponent<SuperChartProps, {}>
116116
);
117117
}
118118

119-
processChartProps: (input: {
120-
chartProps: ChartProps;
121-
preTransformProps?: PreTransformProps;
122-
transformProps?: TransformProps;
123-
postTransformProps?: PostTransformProps;
124-
}) => any;
125-
126-
createLoadableRenderer: (input: {
127-
chartType: string;
128-
overrideTransformProps?: TransformProps;
129-
}) => LoadableRenderer<RenderProps, LoadedModules> | (() => null);
119+
processChartProps: (
120+
input: {
121+
chartProps: ChartProps;
122+
preTransformProps?: PreTransformProps;
123+
transformProps?: TransformProps;
124+
postTransformProps?: PostTransformProps;
125+
},
126+
) => any;
127+
128+
createLoadableRenderer: (
129+
input: {
130+
chartType: string;
131+
overrideTransformProps?: TransformProps;
132+
},
133+
) => LoadableRenderer<RenderProps, LoadedModules> | (() => null);
130134

131135
renderChart(loaded: LoadedModules, props: RenderProps) {
132136
const { Chart, transformProps } = loaded;

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: 3 additions & 50 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 });
@@ -508,7 +461,7 @@ describe('SupersetClientClass', () => {
508461

509462
return client.init().then(() =>
510463
client.post({ url: mockPostUrl, postPayload }).then(() => {
511-
const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData;
464+
const formData = fetchMock.calls(mockPostUrl)[0][1].body;
512465
expect(fetchMock.calls(mockPostUrl)).toHaveLength(1);
513466
Object.keys(postPayload).forEach(key => {
514467
expect(formData.get(key)).toBe(JSON.stringify(postPayload[key]));
@@ -526,7 +479,7 @@ describe('SupersetClientClass', () => {
526479

527480
return client.init().then(() =>
528481
client.post({ url: mockPostUrl, postPayload, stringify: false }).then(() => {
529-
const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData;
482+
const formData = fetchMock.calls(mockPostUrl)[0][1].body;
530483
expect(fetchMock.calls(mockPostUrl)).toHaveLength(1);
531484
Object.keys(postPayload).forEach(key => {
532485
expect(formData.get(key)).toBe(String(postPayload[key]));

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ describe('callApi()', () => {
9292
expect(calls).toHaveLength(1);
9393

9494
const fetchParams = calls[0][1];
95-
const body = fetchParams.body as FormData;
95+
const { body } = fetchParams;
9696

9797
Object.keys(postPayload).forEach(key => {
9898
expect(body.get(key)).toBe(JSON.stringify(postPayload[key]));
@@ -112,7 +112,7 @@ describe('callApi()', () => {
112112
expect(calls).toHaveLength(1);
113113

114114
const fetchParams = calls[0][1];
115-
const body = fetchParams.body as FormData;
115+
const { body } = fetchParams;
116116
expect(body.get('key')).toBe(JSON.stringify(postPayload.key));
117117
expect(body.get('noValue')).toBeNull();
118118

@@ -139,8 +139,8 @@ describe('callApi()', () => {
139139
const calls = fetchMock.calls(mockPostUrl);
140140
expect(calls).toHaveLength(2);
141141

142-
const stringified = calls[0][1].body as FormData;
143-
const unstringified = calls[1][1].body as FormData;
142+
const stringified = calls[0][1].body;
143+
const unstringified = calls[1][1].body;
144144

145145
Object.keys(postPayload).forEach(key => {
146146
expect(stringified.get(key)).toBe(JSON.stringify(postPayload[key]));

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)