feat: support yarn catalogs #1582
Conversation
| } | ||
|
|
||
| // For pnpm, also expose the 'default' catalog as a top-level 'catalog' property | ||
| if (yamlData.catalogs?.default) { |
There was a problem hiding this comment.
this doesn't make really sense to me and would create an catalog entry which wasn't defined before (see also workspace.test.ts)
There was a problem hiding this comment.
Are we sure this was wrong? I'm hesitant to remove anything from the existing catalog implementation, as this would be a breaking change.
i.e. is this a patch change or major change, and how many people are relying on this behavior currently.
@jakeboone02 Can you weigh in on the purpose of this block and the implications of removing it?
There was a problem hiding this comment.
Hi @raineorshine, Hi @jakeboone02 ,
yes I'm pretty sure that this is the wrong behaviour and needs to be removed. Would even classify it as bug and not as breaking change.
Details:
For everyone that explicit is using a named catalog with the name default (e.g. catalogs.default it will also break the usage of pnpm (see provide screenshot) because it will automatically introduce the default catalog catalog (in the current ncu implementation called singular catalog) after running ncu. I tested it without ncu just added a named catalog default and run install with v10.13.1 as well as v10.26.2 of pnpm.
| output.should.deep.equal({ | ||
| 'pnpm-workspace.yaml': { | ||
| packages: ['packages/**'], | ||
| catalog: { 'ncu-test-v2': '2.0.0' }, |
There was a problem hiding this comment.
this shouldn't happen because catalog wasn't defined in the config before so it shouldn't be created by ncu
|
Thanks so much for your submission! |
2 participants
see #1576
additional fixed the unwanted behaviour to create an catalog entry if a named catalog
defaultis present