4.0.2 is broken?

[henhal]

Am I mistaken, or are all releases between 4.0.2 up to 6.0.0 broken?
I was running 4.0.1 for a long time in production with good results, but today when measuring some caching metrics I realized I had lots of misses. Turns out this commit breaks the caching of promises by calling await on the returned promise before putting it in the cache. Am I right?
I realize this is not a big deal since it IS fixed in 6.0.0, but perhaps put a note on the Readme or something to stay away from 4.0.2 and 5.x? Can you even revert the 4.0.2 release so that ^4.0.1 in package.json does not pick it up?

(BTW I had severe problems going to 6.0 due to the ESM change. I can't for my life get tsconfig to play ball when this module uses ESM. Still struggling...)

[sindresorhus]

I think the issue is only with v4.0.2, as from what I can remember, it was fixed in v5.0.0: 343ac6e

[sindresorhus]

(BTW I had severe problems going to 6.0 due to the ESM change. I can't for my life get tsconfig to play ball when this module uses ESM. Still struggling...)

It will be much better with the upcoming TS 4.5: https://devblogs.microsoft.com/typescript/announcing-typescript-4-5-beta/

[henhal]

Thanks.

Since I found this by chance and it impacted my network performance significantly, do you think it would be good to unpublish 4.0.2? Sure, that will break some installs, but it's better to go back to 4.0.1 if you're on ^4.

At least, wiser from this, I've now added caching metrics to my code to keep track on the hit rate, so there's some good out of this. :)

[sindresorhus]

It's not possible to remove versions, but I have deprecated 4.0.2 with a message to stay on 4.0.1 or upgrade to 5.0.0.

[fregante]

I have deprecated 4.0.2 with a message to stay on 4.0.1

That's unfortunate when mem is used as a sub-dependency or without a lockfile. I think it's best to release 4.0.3 since 4.0.2 was a breaking version anyway.

[sindresorhus]

@fregante Why is it unfortunate? People that don't have a lockfile get notified that they're running on a broken version.

[fregante]

Who reads npm install logs? They're so noisy. Leaving a broken version as HEAD of v4 is not great practice, especially if the solution is just republishing 4.0.1 as 4.0.3 (even if breaking for the 1 user who depends on the new behavior, it's not breaking for literally everyone else)

[fregante]

Not to mention that "stay on 4.0.1" means manually editing every package.json that uses "^4.0.0"

[niksy]

@sindresorhus is it possible to backport fixes from 343ac6e to 4.0.3?

I agree with @fregante, people rarely read through all install logs, and just switching to >= 5.0.0 is not convenient for projects where ESM is not fully embraced.

[sindresorhus]

and just switching to >= 5.0.0 is not convenient for projects where ESM is not fully embraced.

You can use await import('p-memoize') from CommonJS.

[sindresorhus]

@fregante I don't disagree, but there's also a risk to just revert to 4.0.1.

Would backporting 343ac6e work for anyone? That seems like a less risky change.

[niksy]

and just switching to >= 5.0.0 is not convenient for projects where ESM is not fully embraced.

You can use await import('p-memoize') from CommonJS.

Right, but that works only for Node >= 12 if I’m correct?

@fregante I don't disagree, but there's also a risk to just revert to 4.0.1.

Would backporting 343ac6e work for anyone? That seems like a less risky change.

This seems like reasonable solution. We would need ponyfill (or basic implementation) for Promise.allSettled to cover Node <= 12.9.0.

[sindresorhus]

Right, but that works only for Node >= 12 if I’m correct?

Node.js 14. So it's a good solution if you're not making reusable packages.

[niksy]

I’ve created PR for this. 343ac6e has been cherry-picked and I’ve used p-settle for Promise.allSettled implementation.

[sindresorhus]

https://github.com/sindresorhus/p-memoize/releases/tag/v4.0.3