Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend/typing regression in v11 #1178

Closed
2 tasks done
pasieronen opened this issue Apr 23, 2020 · 6 comments
Closed
2 tasks done

Extend/typing regression in v11 #1178

pasieronen opened this issue Apr 23, 2020 · 6 comments
Labels
bug Something does not work as it should ✭ help wanted ✭

Comments

@pasieronen
Copy link

Describe the bug

  • Node.js version: 12.16.2
  • OS & version: MacOS 10.15.4
  • Typescript version: 3.8.3

Actual behavior

With got 11.0.2, this file results in TypeScript compiler error:

import got from 'got'

interface ServerResponse { foo: number }

const jsonClient = got.extend({ responseType: 'json' });

jsonClient.post<ServerResponse>('http://localhost:3030/', { json: { bar: 1 } }).then(r => console.log(r.body.foo))

The error is:

test.ts:7:59 - error TS2769: No overload matches this call.
  Overload 1 of 16, '(url: string | URL, options?: Merge<Options, { isStream?: false; resolveBodyOnly?: false; responseType: "json"; }>): CancelableRequest<Response<ServerResponse>>', gave the following error.
    Argument of type '{ json: { bar: number; }; }' is not assignable to parameter of type 'Merge<Options, { isStream?: false; resolveBodyOnly?: false; responseType: "json"; }>'.
      Property 'responseType' is missing in type '{ json: { bar: number; }; }' but required in type '{ isStream?: false; resolveBodyOnly?: false; responseType: "json"; }'.
  Overload 2 of 16, '(url: string | URL, options?: Merge<Merge<Options, { isStream?: false; resolveBodyOnly?: false; responseType: "json"; }>, ResponseBodyOnly>): CancelableRequest<...>', gave the following error.
    Argument of type '{ json: { bar: number; }; }' is not assignable to parameter of type 'Merge<Merge<Options, { isStream?: false; resolveBodyOnly?: false; responseType: "json"; }>, ResponseBodyOnly>'.
      Property 'responseType' is missing in type '{ json: { bar: number; }; }' but required in type 'Pick<Merge<Options, { isStream?: false; resolveBodyOnly?: false; responseType: "json"; }>, "search" | "host" | "password" | "body" | "form" | "path" | "json" | "timeout" | ... 54 more ... | "sessionIdContext">'.

7 jsonClient.post<ServerResponse>('http://localhost:3030/', { json: { bar: 1 } }).then(r => console.log(r.body.foo))
                                                            ~~~~~~~~~~~~~~~~~~~~

  node_modules/got/dist/source/types.d.ts:28:5
    28     responseType: 'json';
           ~~~~~~~~~~~~
    'responseType' is declared here.
  node_modules/got/dist/source/types.d.ts:28:5
    28     responseType: 'json';
           ~~~~~~~~~~~~
    'responseType' is declared here.


Found 1 error.

Expected behavior

With got 10.7.0, the code compiles fine.

Code to reproduce

See above.

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@dscalzi
Copy link

dscalzi commented May 2, 2020

Can confirm issue persists on v11.1.0.

@szmarczak
Copy link
Collaborator

I think this d3b972e may be the culprit, but haven't tried your example yet.

@dscalzi
Copy link

dscalzi commented May 2, 2020

Here's a sample test that should work, yet fails with this regression (and should be added to the test suite).

test('sends plain objects as JSON using extended client', withServer, async (t, server, got) => {
	server.post('/', defaultEndpoint);

	const extendedClient = got.extend({ responseType: 'json' })

	const {body} = await extendedClient.post<{such: 'wow'}>({
		json: {such: 'wow'}
	});
	t.deepEqual(body, {such: 'wow'});
});

I don't think the <{such: 'wow'}> would be needed when this is fixed.

@dscalzi
Copy link

dscalzi commented May 2, 2020

Just checked back to v10.7.0, this wasnt necessarily working as intended their either. It was using the responseType?: 'default' signature (defined here https://github.com/sindresorhus/got/blob/v10.7.0/source/create.ts#L49, removed in v11). In order for this to be properly implemented, there would have to be some kind of relationship between the InstanceDefaults and the options parameter of the GotRequestFunction.

@szmarczak szmarczak added bug Something does not work as it should ✭ help wanted ✭ enhancement This change will extend Got features and removed bug Something does not work as it should labels May 3, 2020
@szmarczak
Copy link
Collaborator

This is actually a duplicate of #1117, closing.

@szmarczak szmarczak reopened this May 3, 2020
@szmarczak
Copy link
Collaborator

Actually this shouldn't error, sorry.

@szmarczak szmarczak added bug Something does not work as it should and removed enhancement This change will extend Got features labels May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should ✭ help wanted ✭
Projects
None yet
Development

No branches or pull requests

3 participants