Skip to content

Allow catalogs to work with descriptors without resolvers#6930

Merged
arcanis merged 6 commits intoyarnpkg:masterfrom
parksb:6928
Nov 7, 2025
Merged

Allow catalogs to work with descriptors without resolvers#6930
arcanis merged 6 commits intoyarnpkg:masterfrom
parksb:6928

Conversation

@parksb
Copy link
Contributor

@parksb parksb commented Oct 2, 2025

What's the problem this PR addresses?

Resolves #6928

How did you fix it?

I modified the catalog plugin to skip binding a descriptor when the descriptor referenced by the catalog protocol is not supported by any resolver.

Since an error occurs when binding a descriptor that has no supporting resolver, I added a check before calling resolver.bindDescriptor. Even if the catalog plugin doesn't resolve the descriptor, an error will still be properly raised during the final resolution step if no resolver can handle the descriptor.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@arcanis
Copy link
Member

arcanis commented Nov 7, 2025

Thanks! Can you migrate the test to our integration tests?
https://github.com/parksb/berry/blob/d982143e9704351eb0913b7fe11ff356e092a86b/packages/acceptance-tests/pkg-tests-specs/sources/features/catalogs.test.ts

This will make it more resilient to refactorings (I'm reworking part of the architecture and unit tests won't be reusable without efforts, but integration tests are very easy to port).

@parksb
Copy link
Contributor Author

parksb commented Nov 7, 2025

Thank you for reviewing the PR :) In the unit tests, I tested the case where 'a descriptor is not supported by any resolver'. However, it wasn't easy to test only this specific case in the integration tests.

I added a plugin that supports a custom protocol via reduceDependency to the integration tests, which tests the case where a custom protocol without a resolver is handled correctly. (720b7cd)

@arcanis arcanis merged commit afe40b1 into yarnpkg:master Nov 7, 2025
26 checks passed
@arcanis
Copy link
Member

arcanis commented Nov 7, 2025

Thanks!

@parksb parksb deleted the 6928 branch November 7, 2025 13:59
Gudahtt added a commit to Gudahtt/berry that referenced this pull request Feb 9, 2026
…lowlist

* origin/master: (212 commits)
  CI: Select node version to run CI against automatically (yarnpkg#7032)
  Fixes foreach order when --topological isnt set (yarnpkg#6997)
  fix(fslib): handle float timestamps in convertToBigIntStats (yarnpkg#6988)
  Fixes the `/<name>/<version>` format (yarnpkg#6993)
  docs(constraints): add missing @typedef alias for `Context` (yarnpkg#6989)
  Sync master with the changes from master
  Releasing 3 new packages
  Fix JSON Schema (yarnpkg#6973)
  Implements npm web login support (yarnpkg#6981)
  fix(git): split `-c` and `core.autocrlf=false` into separate args for `clone` (yarnpkg#6983)
  Sync master with the changes from master
  Releasing 8 new packages
  Allow catalogs to work with descriptors without resolvers (yarnpkg#6930)
  docs: Clarify additional use-case of npmMinimalAgeGate (yarnpkg#6945)
  Migrates the "typescript" dependencies to a catalog (yarnpkg#6969)
  Support escaping template variables in environment values (yarnpkg#6935)
  Core: Create DURATION settings type (yarnpkg#6942)
  fix: use correct env var to detect gitlab CI for OIDC (yarnpkg#6938)
  Update README.md badge link (yarnpkg#6947)
  fix(publish): use correct workspace name in --json output (yarnpkg#6949)
  ...
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.

[Bug?]: Catalogs fail to resolve custom protocols from plugins using reduceDependency hook

2 participants