Skip to content

fix: reuse catalog resolutions of complex version references correctly#8280

Closed
gluxon wants to merge 3 commits intomainfrom
gluxon/catalog-fix-alias
Closed

fix: reuse catalog resolutions of complex version references correctly#8280
gluxon wants to merge 3 commits intomainfrom
gluxon/catalog-fix-alias

Conversation

@gluxon
Copy link
Member

@gluxon gluxon commented Jul 5, 2024

Problem

I noticed in the pnpm repo that the catalog snapshot version we're storing for npm aliases is wrong.

# pnpm-lock.yaml

catalogs:
  default:
    execa:
      specifier: npm:safe-execa@0.1.2
      version: 0.1.2 # 👈 This is wrong

It should match the version used by importers. (i.e. safe-execa@0.1.2)

# pnpm-lock.yaml

importers:

  __utils__/scripts:
    dependencies:
      execa:
        specifier: 'catalog:'
        version: safe-execa@0.1.2 # 👈 Catalogs should be storing this

This caused a bug where pnpm update accidentally looked for execa@0.1.2 and 404'ed instead of looking for safe-execa@0.1.2.

Screenshot 2024-07-04 at 8 40 17 PM

Changes

Using the depPathToRef function to compute the version stored for the pnpm-lock.yaml catalogs section, similar to how we compute the version ref for the importers section.

const ref = depPathToRef(dep.pkgId, {
alias: dep.alias,
realName: dep.name,
resolution: dep.resolution,
})

After this change, catalog snapshots store the right version reference.

# pnpm-lock.yaml

catalogs:
  default:
    execa:
      specifier: npm:safe-execa@0.1.2
      version: safe-execa@0.1.2

On the resolution side, we also need to parse the stored version reference correctly.

Comment on lines +15 to +23
// Note: The version recorded here is used for lookups of this cataloged
// dependency in future installs. This should be computed the same as
// other version refs in the lockfile.
version: depPathToRef(dep.pkgId, {
alias: dep.alias,
realName: dep.name,
resolution: dep.resolution,
}),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in the PR description, this follows how we compute the version ref for importers.

const ref = depPathToRef(dep.pkgId, {
alias: dep.alias,
realName: dep.name,
resolution: dep.resolution,
})

@gluxon gluxon force-pushed the gluxon/catalog-fix-alias branch 2 times, most recently from cfa55a5 to 660d462 Compare July 5, 2024 03:55
@gluxon gluxon changed the title fix: store correct version reference for npm aliases in catalog snapshots fix: reuse catalog resolutions of complex version references correctly Jul 5, 2024
@gluxon gluxon force-pushed the gluxon/catalog-fix-alias branch from 660d462 to 14c2a1a Compare July 5, 2024 04:04
@gluxon
Copy link
Member Author

gluxon commented Jul 5, 2024

Going with a slightly different approach. Closing.

@gluxon gluxon closed this Jul 5, 2024
@gluxon gluxon deleted the gluxon/catalog-fix-alias branch July 5, 2024 04:08
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.

1 participant