-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: add cleanupUnusedCatalogs
config
#9793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The test failure should have nothing to do with this modification. |
I could see some users wanting to leave around a catalog entry in case it'll be used in the future. Should we use a new config for this instead of going off of the existing I've found reusing configs for new behavior can cause coupling of behaviors that aren't desirable for all users. People setting |
IMO, setting To avoid confusion, we can update the description of If we add new configurations, will that be too much? |
While I personally agree, I don't know if this is the case for all users. There's a new case we should consider: What if users add catalog entries through pnpm config dependencies? We would probably want to omit catalog entries added in this way. Config dependencies are used to share configuration across multiple repos. A given repo may not use every catalog entry in the shared config dependency. |
Indeed, there is such a scenario. 🤔 |
Well, this is the description of
It doesn't mention that unused catalog entries are prohibited. Also, I don't think it should necessarily be limited to |
What's the best approach for handling new configurations? Should I add a configuration that takes effect when removing a catalog dependency, or create a new command to clear unused catalog dependencies? |
I think it can work on any install command, not just on remove. |
dedupeCatalog
config
|
Looks great. |
dedupeCatalog
configcleanupUnusedCatalogs
config
I don't think it is a good idea to do the update of the workspace manifest inside Also, I think it is bad already that we currently have 2 functions that write the workspace manifest and you add a third one. Potentially we will be writing this file multiple times. I think we should have only one function with options. So even the |
I've made the changes, but I'm not sure I understood you correctly. |
When
cleanupUnusedCatalogs
is set totrue
, unused catalog dependencies inpnpm-workspace.yaml
are automatically cleared when installing or removing dependencies.