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) (apache#71)" (apache#126)

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

This reverts commit e386612.

* fix: type errors

* fix: typescript errors in superset-ui-demo
  • Loading branch information
xtinec committed Apr 2, 2019
1 parent 20cf61b commit a53b2af
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 99 deletions.
5 changes: 5 additions & 0 deletions package.json
Expand Up @@ -86,6 +86,11 @@
"testEnvironment": "node"
}
]
},
"typescript": {
"include": [
"./storybook/**/*"
]
}
},
"workspaces": [
Expand Down
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
1 change: 0 additions & 1 deletion packages/superset-ui-connection/package.json
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
14 changes: 1 addition & 13 deletions packages/superset-ui-connection/src/callApi/parseResponse.ts
@@ -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
50 changes: 1 addition & 49 deletions packages/superset-ui-connection/test/SupersetClientClass.test.ts
@@ -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
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
3 changes: 3 additions & 0 deletions packages/superset-ui-demo/package.json
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"
}
}
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
4 changes: 4 additions & 0 deletions packages/superset-ui-demo/types/external.d.ts
@@ -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';
30 changes: 0 additions & 30 deletions types/external.d.ts

This file was deleted.

0 comments on commit a53b2af

Please sign in to comment.