Allow certain packages to be excluded from fixers and the check #9
Conversation
gluxon
force-pushed
the
ignored-config
branch
from
29b304e
to
6f18072
Compare
| throw err; | ||
| } | ||
|
|
||
| const parsed = json5.parse(buf.toString()); |
There was a problem hiding this comment.
Using json5 to allow comments in the new config file. I'd like to write the reason some packages are excluded from the fixer/check.
| // This function manually validates the config currently since it's small. If | ||
| // the config ever grows in size, consider using a runtime validation library | ||
| // such as runtypes, ajv, etc. | ||
| export function assertValidConfig(obj: unknown): asserts obj is Config { |
There was a problem hiding this comment.
Let me know if we should just use runtypes or ajv. Avoided that since it can be a large dependency, but maybe it's worthwhile to have.
| const config = await readConfig(root); | ||
| const ignoredSet = new Set(config?.ignored ?? []); | ||
| const isIgnored = (duplicate: Package) => ignoredSet.has(duplicate.name); |
There was a problem hiding this comment.
There's a bit of duplicate code between listDuplicates and fixDuplicates. Is that something we'd want to factor out?
|
If you just write this fixed version number of |
|
That would probably work locally. What if it's desirable to publish with the There's also the case of dependencies that only appear in the dependency graph through non-direct transitive dependencies. In that case it'd be difficult (or hacky at the last) to rewrite those version specifiers since they're not declared in |
|
If you see type declarations as part of an API, then it is the fault of If the packages only appear in the dependency graph through non-direct transitive dependencies, then (I do know that It's generally more difficult to maintain backward compatibility for TS declarations than JS implementations. Therefore, it's common for a minor version update to include type declaration breaking changes, especially for I view |
|
You made a good point that Thanks for explaining politely and thoroughly. Appreciate the work you've done to create this CLI as-is. 🙂 |
Hey @ocavue, I'm hoping to roll this tool out to my team. Thanks for making it!
One problem we hit pretty quickly was a significant typing change in a minor version of
@types/lodash. Afterpnpm-deduplicateremoved earlier versions of@types/lodashfrom our monorepo, I spent a few hours attempting to update the existing code for these changes. Finishing this out would be a multi-day effort.I'd like to require
pnpm-deduplicate checkto pass on CI before builds are merged into the main branch. This would prevent developers from introducing further duplicates inpnpm-lock.yamlwhile we handle the existing duplicates.Would adding an ignore config for certain dependencies be reasonable?