Repository navigation
perf(npm-registry,scanner): include publishedAt in fix version resolutions to avoid redundant resolveVersionPublishDate call - #742
Conversation
sonukapoor
left a comment
There was a problem hiding this comment.
Two things to address before merging:
-
The
!packumentearly return inresolvePublishedFixVersion(before the first hunk in this file) omitspublishedAt. Since the type marks it optional with?, TypeScript will not flag this - butsetPublishedDatein scanner.ts checksif (date === undefined) returnand skips settingfinding.fixVersionPublishedAttonull. The oldattachFixVersionPublishDateexplicitly setfixVersionPublishedAt = nullwhen validation failed; this new path loses that guarantee. AddingpublishedAt: nullto the!packumentreturn restores it. -
resolveVersionPublishDateinnpm-registry.tsis now a dead export - the import was removed fromscanner.tsbut the function definition still exists. Worth deleting to keep the module surface clean.
…tions, remove redundant resolveVersionPublishDate call resolvePublishedFixVersion and resolveLowestKnownNonVulnerableVersion both already have the packument in scope. Adding publishedAt lets validateDirectFixTargets skip the redundant resolveVersionPublishDate call (packument cache hit + time lookup). - Extended PublishedFixVersionResolution with publishedAt?: string | null - resolvePublishedFixVersion: added publishedAt to all 3 success return paths - resolveLowestKnownNonVulnerableVersion: added publishedAt to success return - Removed attachFixVersionPublishDate; replaced with inline setPublishedDate helper - Removed unused resolveVersionPublishDate import from scanner.ts
… functions - Extended mockPackument to accept optional time map - Added publishedAt assertion to existing resolveLowestKnownNonVulnerableVersion test - Added new resolvePublishedFixVersion describe with exact match publishedAt test
58ad247 to
39d6fa3
Compare
…s in resolveLowestKnownNonVulnerableVersion
…ishedAt is mandatory
|
Merged — thank you @luojiyin1987! |
Closes #741
Problem
In
validateDirectFixTargets, bothresolvePublishedFixVersionandresolveVersionPublishDatefetch the same packument for the same package.resolvePublishedFixVersionalready has the packument in scope — it could also readpackument.time[version]and return the publish date, avoiding the secondresolveVersionPublishDatecall (packument cache lookup + time access + cache write).Changes
PublishedFixVersionResolutionwithpublishedAt: string | nullresolvePublishedFixVersion: addedpublishedAtto all return pathsresolveLowestKnownNonVulnerableVersion: addedpublishedAtto success return pathsattachFixVersionPublishDatehelper; replaced with inlinesetPublishedDatehelperresolveVersionPublishDateimport fromscanner.tsVerification