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

Use cached npm info #451

Merged
merged 5 commits into from
Jun 9, 2022
Merged

Use cached npm info #451

merged 5 commits into from
Jun 9, 2022

Conversation

jablko
Copy link
Contributor

@jablko jablko commented May 5, 2022

  • In generate-packages we know the npm info was cached in calculate-versions, so we don't consult the origin (npm registry) --- except in this one case, so fix that case (replace fetchAndCacheNpmInfo() with getNpmInfoFromCache()).
  • Add some static checks that we're not going to the origin, except in calculate-versions.
  • Don't bother writing the cache if there's no way we could have gone to the origin (correct but inconsequential optimization).

@jablko jablko force-pushed the patch-48 branch 2 times, most recently from 50afb55 to ffda973 Compare May 8, 2022 19:46
@jablko jablko force-pushed the patch-48 branch 3 times, most recently from 999ea2d to 74a5653 Compare May 16, 2022 15:09
@jablko
Copy link
Contributor Author

jablko commented May 16, 2022

I've rebased this on #456 and replaced UncachedNpmInfoClient/CachedNpmInfoClient with pacote, which the npm cli uses internally for fetching metadata from the registry.

The advantages over UncachedNpmInfoClient/CachedNpmInfoClient are caching and concurrency control:

  • Instead of hard coding whether the cache is fresh or stale (getNpmInfoFromCache() vs. fetchAndCacheNpmInfo()), using pacote.manifest() either way will use the cache if it's fresh and revalidate if not.
  • There's one case where we already check whether the cached typesPublisherContentHash is up to date, which I retained.
  • pacote handles concurrency internally via maxSockets, so wrapping it in nAtATime() is no longer needed.

@andrewbranch andrewbranch merged commit e246b08 into microsoft:master Jun 9, 2022
@andrewbranch andrewbranch mentioned this pull request Jun 9, 2022
andrewbranch added a commit that referenced this pull request Jun 9, 2022
andrewbranch added a commit that referenced this pull request Jun 9, 2022
* Revert "Use cached npm info (#451)"

This reverts commit e246b08.

* Fix package.json version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants