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

How to get it working TypeScript? #14

Closed
rfgamaral opened this issue Feb 29, 2020 · 3 comments · Fixed by #26
Closed

How to get it working TypeScript? #14

rfgamaral opened this issue Feb 29, 2020 · 3 comments · Fixed by #26

Comments

@rfgamaral
Copy link

I've followed the example in the README.md but I get this:

image

However, replacing p-memoize with mem works just fine in TypeScript without any issues.

This leads me to two questions:

  1. What am I missing to get p-memoize properly working with TypeScript?
  2. What exactly is the advantage of p-memoize over mem for using with got?
@klvenky
Copy link

klvenky commented Jun 24, 2020

I think it's a little too late to answer your question, but the issue could be with the memGot not having a type defined. Instead it can be often improved as below.

const memGot = (url:string, opts: GotOptions<string>) => pMemoize(() => got(url, opts), {maxAge: 6000});

It would be better to follow this syntax as the options passed to the got function can be varying and this will help in keeping the code clean.

Also the advantage of using p-memoize over mem is explained here.
Answered best of my knowledge. Correct me if I'm wrong.

@fregante
Copy link
Collaborator

fregante commented Oct 10, 2020

What am I missing to get p-memoize properly working with TypeScript?

Nothing, probably the types are wrong.

What exactly is the advantage of p-memoize over mem for using with got?

No advantage unless you're using cachePromiseRejection. p-memoize is a duplicate module and just wraps mem to add that option.

Also the advantage of using p-memoize over mem is explained here.

Nothing there mentions p-memoize; all of that also applies to p-memoize

@kentcdodds
Copy link

For anyone else having TS issues with p-memoize, after reading this: "No advantage unless you're using cachePromiseRejection" and realizing that I'm not using that feature, I switched to mem and TS issues disappeared.

Richienb added a commit to Richienb/p-memoize that referenced this issue Sep 24, 2021
Fixes sindresorhus#3, Fixes sindresorhus#14

Signed-off-by: Richie Bendall <richiebendall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants