Skip to content

Commit

Permalink
Revert "Handle BigNumber conversions in JSON properly (without loss o…
Browse files Browse the repository at this point in the history
…f precision) (#71)" (#126)

* revert: revert "Handle BigNumber conversions in JSON properly (without loss of precision) (#71)"

This reverts commit e386612.

* fix: type errors

* fix: typescript errors in superset-ui-demo
  • Loading branch information
xtinec authored and zhaoyongjie committed Nov 25, 2021
1 parent a24968d commit 7cb307b
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@
"testEnvironment": "node"
}
]
},
"typescript": {
"include": [
"./storybook/**/*"
]
}
},
"workspaces": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ const defaultMockLoadFormData = jest.fn(allProps => Promise.resolve(allProps.for

// coerce here else get: Type 'Mock<Promise<any>, []>' is not assignable to type 'Mock<Promise<any>, any[]>'
let mockLoadFormData = defaultMockLoadFormData as jest.Mock<Promise<any>, any>;
const mockLoadDatasource = jest.fn(datasource => Promise.resolve(datasource));
const mockLoadQueryData = jest.fn(input => Promise.resolve(input));
const mockLoadDatasource = jest.fn(datasource => Promise.resolve(datasource)) as jest.Mock<
Promise<any>,
any
>;
const mockLoadQueryData = jest.fn(input => Promise.resolve(input)) as jest.Mock<Promise<any>, any>;

// ChartClient is now a mock
jest.mock('../../src/clients/ChartClient', () =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
},
"dependencies": {
"@babel/runtime": "^7.1.2",
"json-bigint": "^0.3.0",
"whatwg-fetch": "^2.0.4"
},
"publishConfig": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import JSONbig from 'json-bigint';
import { ParseMethod, SupersetClientResponse } from '../types';

function rejectIfNotOkay(response: Response): Promise<Response> {
Expand All @@ -7,15 +6,6 @@ function rejectIfNotOkay(response: Response): Promise<Response> {
return Promise.resolve<Response>(response);
}

function parseJson(text: string): any {
try {
return JSONbig.parse(text);
} catch (e) {
// if JSONbig.parse fails, it throws an object (not a proper Error), so let's re-wrap the message.
throw new Error(e.message);
}
}

export default function parseResponse(
apiPromise: Promise<Response>,
parseMethod: ParseMethod = 'json',
Expand All @@ -27,9 +17,7 @@ export default function parseResponse(
} else if (parseMethod === 'text') {
return checkedPromise.then(response => response.text().then(text => ({ response, text })));
} else if (parseMethod === 'json') {
return checkedPromise.then(response =>
response.text().then(text => ({ json: parseJson(text), response })),
);
return checkedPromise.then(response => response.json().then(json => ({ json, response })));
}

throw new Error(`Expected parseResponse=null|json|text, got '${parseMethod}'.`);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,8 @@
import fetchMock from 'fetch-mock';

import { BigNumber } from 'bignumber.js';
import { SupersetClientClass, ClientConfig } from '../src';
import throwIfCalled from './utils/throwIfCalled';
import { LOGIN_GLOB } from './fixtures/constants';

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

return new SupersetClientClass({})
Expand Down Expand Up @@ -267,8 +250,7 @@ describe('SupersetClientClass', () => {
const mockTextUrl = `${protocol}//${host}${mockTextEndpoint}`;
const mockPutUrl = `${protocol}//${host}${mockPutEndpoint}`;
const mockDeleteUrl = `${protocol}//${host}${mockDeleteEndpoint}`;
const mockBigNumber = '9223372036854775807';
const mockTextJsonResponse = `{ "value": ${mockBigNumber} }`;
const mockTextJsonResponse = '{ "value": 9223372036854775807 }';

fetchMock.get(mockGetUrl, { json: 'payload' });
fetchMock.post(mockPostUrl, { json: 'payload' });
Expand Down Expand Up @@ -347,21 +329,6 @@ describe('SupersetClientClass', () => {
);
});

it('supports parsing a response as JSON while preserving precision of large numbers', () => {
expect.assertions(2);
const client = new SupersetClientClass({ protocol, host });

return client.init().then(() =>
client.get({ url: mockTextUrl }).then(({ json }) => {
expect(fetchMock.calls(mockTextUrl)).toHaveLength(1);
// @ts-ignore
expect(json.value.toString()).toBe(new BigNumber(mockBigNumber).toString());

return Promise.resolve();
}),
);
});

it('supports parsing a response as text', () => {
expect.assertions(2);
const client = new SupersetClientClass({ protocol, host });
Expand Down Expand Up @@ -471,21 +438,6 @@ describe('SupersetClientClass', () => {
);
});

it('supports parsing a response as JSON while preserving precision of large numbers', () => {
expect.assertions(2);
const client = new SupersetClientClass({ protocol, host });

return client.init().then(() =>
client.post({ url: mockTextUrl }).then(({ json }) => {
expect(fetchMock.calls(mockTextUrl)).toHaveLength(1);
// @ts-ignore
expect(json.value.toString()).toBe(new BigNumber(mockBigNumber).toString());

return Promise.resolve();
}),
);
});

it('supports parsing a response as text', () => {
expect.assertions(2);
const client = new SupersetClientClass({ protocol, host });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('parseResponse()', () => {
.catch(error => {
expect(fetchMock.calls(mockTextUrl)).toHaveLength(1);
expect(error.stack).toBeDefined();
expect(error.message.includes('Unexpected')).toBe(true);
expect(error.message.includes('Unexpected token')).toBe(true);

return Promise.resolve();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,8 @@
"react": "^16.6.0",
"react-dom": "^16.6.0",
"storybook-addon-jsx": "^5.4.0"
},
"devDependencies": {
"@types/storybook__addon-knobs": "^4.0.4"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { SupersetClient } from '@superset-ui/connection';
import ErrorMessage from './ErrorMessage';

export type Props = {
children: ({ payload }: { payload: object }) => ReactNode;
children: ({ payload }: { payload?: object }) => ReactNode;
endpoint?: string;
host: string;
method?: 'POST' | 'GET';
Expand All @@ -16,7 +16,7 @@ type State = {
payload?: object;
};

export const renderError = error => (
export const renderError = (error: Error) => (
<div>
The following error occurred, make sure you have <br />
1) configured CORS in Superset to receive requests from this domain. <br />
Expand All @@ -35,7 +35,7 @@ export default class VerifyCORS extends React.Component<Props, State> {
this.handleVerify = this.handleVerify.bind(this);
}

componentDidUpdate(prevProps) {
componentDidUpdate(prevProps: Props) {
const { endpoint, host, postPayload, method } = this.props;
if (
(this.state.didVerify || this.state.error) &&
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
declare module '@superset-ui/legacy-preset-chart-big-number';
declare module '@superset-ui/legacy-plugin-chart-sankey';
declare module '@superset-ui/legacy-plugin-chart-sunburst';
declare module '@superset-ui/legacy-plugin-chart-word-cloud';

This file was deleted.

0 comments on commit 7cb307b

Please sign in to comment.