Skip to content

Conversation

ruimartin
Copy link

@ruimartin ruimartin commented Aug 26, 2025

What's the problem this PR addresses?

Resolves: #6400 by implementing basic support for using catalog

This PR implements catalog via the project configuration yarnrc.yml. We are opting for it instead of package.json to prevent adding complexity in situations where catalogs with the same name could be implemented at different scopes. Named catalogs should be able to address most of use cases.

How did you fix it?

  • Adds support for base catalog and named catalogs by:
    • Implements a new plugin-catalog that hooks into reduceDependency and replaces catalog ranges with the ones defined in a catalog
    • Hooks into beforeWorkspacePacking replacing catalogs with actual ranges before packing
  • Adds relevant unit and integration tests

QA Instructions

  • Edit and play with the catalog definition on .yarnrc.yml and run yarn to see error scenarios (by removing entries or naming then incorrectly), changing versions, etc.
Screen.Recording.2025-08-26.at.09.59.17.mov
  • When running yarn pack, the resulting package should have no references to catalog: on package.json files.

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.

@ruimartin ruimartin force-pushed the ruimartin/feat/catalog-basic branch from 876a1c1 to eedb750 Compare August 26, 2025 07:48
@ruimartin ruimartin force-pushed the ruimartin/feat/catalog-basic branch from f7f5c41 to a05fd71 Compare August 26, 2025 09:32
@ruimartin ruimartin force-pushed the ruimartin/feat/catalog-basic branch from a05fd71 to 3675832 Compare August 26, 2025 10:50
@ruimartin ruimartin marked this pull request as ready for review August 26, 2025 14:33
} catch {
// If resolution fails, leave the catalog reference as-is
// This will allow the error to be caught during normal resolution
continue;
Copy link
Member

Choose a reason for hiding this comment

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

The error will be "unsupported protocol catalog:" which might be surprising, right? Could we make resolveDescriptorFromCatalog return a nullable value, and throw a UsageError when it returns that (I prefer to avoid blanket try statements as they often hide unrelated exceptions)?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, my mistake here. I am now letting the error bubble up, but it isn't being properly catched by the StreamReport.

image

...

After some digging, it seems this process.nextTick() throws the error outside of the StreamReport scope. Removing it doesn't break any of the tests, but makes things slightly slower. Do you have recollections on why it was introduced originally?

image

https://github.com/yarnpkg/berry/pull/125/files#diff-0f0e8cfb83ca10fffa5acf788887422f5f26377c114ea0862219357c4806fdc0L16

@@ -0,0 +1,25 @@
releases:
Copy link
Author

Choose a reason for hiding this comment

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

Will be updated after other discussions are resolved.

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.

[Feature] Catalog like pnpm
2 participants