feat: add dedupeDependencies utility
#175
Conversation
src/api.ts
Outdated
| const resolvedOptions = await resolveOperationOptions(options); | ||
| if (["bun", "deno"].includes(resolvedOptions.packageManager?.name ?? "")) { | ||
| throw new Error( | ||
| `Deduping is currently not supported for \`${resolvedOptions.packageManager?.name ?? "unknown package manager"}\``, |
There was a problem hiding this comment.
In this case, we can fallback to:
- Remove known lockfiles
- Run install
There was a problem hiding this comment.
In this case, we can fallback to:
- Remove known lockfiles
- Run install
Should I use another utility to remove the lockfile or do it in place?
There was a problem hiding this comment.
Inline is fine. We can support { recreateLockFile?: boolean } option also to force this behavior optionaly.
There was a problem hiding this comment.
Inline is fine. We can support
{ recreateLockFile?: boolean }option also to force this behavior optionaly.
I have tried it so that you can also pass a false for deno or bun. This should ensure that you can also prevent recreating. Might be nice for nuxi.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #175 +/- ##
===========================================
- Coverage 82.17% 62.52% -19.65%
===========================================
Files 6 5 -1
Lines 516 515 -1
Branches 71 101 +30
===========================================
- Hits 424 322 -102
- Misses 91 190 +99
- Partials 1 3 +2 ☔ View full report in Codecov by Sentry. |
src/api.ts
Outdated
| const recreateLockfile = options.recreateLockfile ?? !isSupported; | ||
| if(!isSupported) { | ||
| consola.log( | ||
| `Deduping is currently not supported for \`${resolvedOptions.packageManager?.name ?? "unknown package manager"}\`.`, |
There was a problem hiding this comment.
IMO it might only make users more confused, goal of nypm is transparent compatibility and recreate is already closest behavior + we log about what we do.
Also, it might make implicit issues, if a library passes recreateLockfile: false it will be a no-op function on deno/bun.
I suggest we do fallback without warn by default and in case recreateLockfile: false is passed, throw an error so we cover both max compat (default) and strict (if option explicitly asks to not fallback)
There was a problem hiding this comment.
so to summarise:
| recreate | result | |
|---|---|---|
| deno or bun | undefined | recreate lockfile |
| deno or bun | false | error |
| deno or bun | true | recreate lockfile |
| any other pm | true | recreate lockfile |
| deno or bun | undefined | dedupe |
| any other pm | false | dedupe |
- plus no supported warning
Did I understand that correctly?
There was a problem hiding this comment.
Exactly. Without passing recreate (undefined) also:
| recreate | result | |
|---|---|---|
| deno or bun | undefined | recreate lockfile |
| any other pm | undefined | dedupe |
| deno or bun | false | error |
| any other pm | false | dedupe |
| deno or bun | true | recreate lockfile |
| any other pm | true | recreate lockfile |
There was a problem hiding this comment.
I tried to add tests as best as possbile. it was not possible for me to test for deno and bun with recreate false. Because the errors origin is the test.
fix #166