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

v6.0.0 to v6.0.1 introduces TypeScript type incompatibilities #433

Closed
guyellis opened this issue Dec 7, 2021 · 7 comments · Fixed by #435 or #437
Closed

v6.0.0 to v6.0.1 introduces TypeScript type incompatibilities #433

guyellis opened this issue Dec 7, 2021 · 7 comments · Fixed by #435 or #437

Comments

@guyellis
Copy link

guyellis commented Dec 7, 2021

Description

In this change: v6.0.0...v6.0.1

The NodeHttpAdapter and FSPersister types are no longer compatible.

import NodeHttpAdapter from '@pollyjs/adapter-node-http';
import FSPersister from '@pollyjs/persister-fs';

  setupPolly({
    adapters: [NodeHttpAdapter],
    persister: FSPersister,
   ...
  });

Error Message & Stack Trace

Type 'typeof NodeHttpAdapter' is not assignable to type 'string | typeof Adapter'.
  Type 'typeof NodeHttpAdapter' is not assignable to type 'typeof Adapter'.
    Construct signature return types 'NodeHttpAdapter' and 'Adapter<TOptions, TRequest>' are incompatible.
      The types of 'defaultOptions' are incompatible between these types.
        Type '{}' is not assignable to type 'TOptions'.
          '{}' is assignable to the constraint of type 'TOptions', but 'TOptions' could be instantiated with a different subtype of constraint '{}'.ts(2322)

Environment

Node.js v14.18.1
linux 5.15.5-76051505-generic
npm: 6.14.15
@guyellis
Copy link
Author

guyellis commented Dec 7, 2021

I just realized that the types come from:

    "@types/pollyjs__adapter-node-http": "2.0.1",
    "@types/pollyjs__core": "4.3.3",
    "@types/pollyjs__persister-fs": "2.0.1",

And that PollyJS is a JS project.

Feel free to close this issue if you feel that this should be handled elsewhere. I'm leaving it open for now in case others come across this when trying to upgrade and may be able to help provide direction here.

@Jason3S
Copy link

Jason3S commented Dec 7, 2021

I ran into the same problem.

I ended up doing the following as a workaround.

import Adapter from '@pollyjs/adapter';
import Persister from '@pollyjs/persister';
import NodeHttpAdapter from '@pollyjs/adapter-node-http';
import FSPersister from '@pollyjs/persister-fs';

Polly.register(NodeHttpAdapter as typeof Adapter);
Polly.register(FSPersister as typeof Persister);

It was not clear to me why it was necessary.

@mcmire
Copy link

mcmire commented Dec 7, 2021

It seems that the types are now located in the subrepos instead of DefinitelyTyped. Why they are incorrect I'm not sure, but @pollyjs/adapter-node-http/types.d.ts should be:

import Adapter from '@pollyjs/adapter';
import { Request } from '@pollyjs/core';

export default class NodeHttpAdapter<
  TOptions extends {} = {},
  TRequest extends Request = Request
> extends Adapter<TOptions, TRequest> {}

and @pollyjs/persister-fs/types.d.ts should be:

import Persister from '@pollyjs/persister';

export default class FSPersister<
  TOptions extends {
    recordingsDir?: string;
  } = {}
> extends Persister<TOptions> {}

@offirgolan
Copy link
Collaborator

offirgolan commented Dec 7, 2021

Can you give v6.0.2 a try?

@guyellis
Copy link
Author

guyellis commented Dec 7, 2021

Thanks for looking at this. Moved on to some new errors:

node_modules/@pollyjs/core/types.d.ts:108:3 - error TS7010: 'constructor', which lacks return-type annotation, implicitly has an 'any' return type.

108   constructor(
      ~~~~~~~~~~~~
109     polly: Polly,
    ~~~~~~~~~~~~~~~~~
... 
116     }
    ~~~~~
117   );
    ~~~~

node_modules/@pollyjs/persister/types.d.ts:2:24 - error TS7016: Could not find a declaration file for module 'set-cookie-parser'. '/<my-repo-redacated>/node_modules/set-cookie-parser/lib/set-cookie.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/set-cookie-parser` if it exists or add a new declaration (.d.ts) file containing `declare module 'set-cookie-parser';`

2 import { Cookie } from 'set-cookie-parser';
                         ~~~~~~~~~~~~~~~~~~~

Found 2 errors.

@offirgolan
Copy link
Collaborator

@guyellis apologies for the back and forth. Can you give v6.0.3 a try?

@guyellis
Copy link
Author

guyellis commented Dec 8, 2021

@offirgolan - yes that worked - thank you! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants