Allow versioned private packages to depend on skipped packages without requiring them to also be skipped#1842
Conversation
🦋 Changeset detectedLatest commit: 00d882d The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
RodrigoHamuy
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1842 +/- ##
==========================================
+ Coverage 81.33% 81.84% +0.51%
==========================================
Files 54 55 +1
Lines 2277 2391 +114
Branches 679 722 +43
==========================================
+ Hits 1852 1957 +105
- Misses 420 429 +9
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "@changesets/config": patch | ||
| --- | ||
|
|
||
| Allow versioned private packages to depend on skipped packages without requiring them to also be skipped. Private packages are not published to npm, so it is safe for them to have dependencies on ignored or unversioned packages. |
There was a problem hiding this comment.
@RodrigoHamuy any reason why do you think only versioned private packages should be exempt of this validation?
There was a problem hiding this comment.
No strong feelings, always simpler if it's up the the user. You mean we should also allow versioned public packages to depend on skipped pckages, right? Happy to action if you prefer
There was a problem hiding this comment.
Actually, I think as it is is correct. Publishing a dependency that depends on non-published libraries ain't gonna work.
There was a problem hiding this comment.
You mean we should also allow versioned public packages to depend on skipped pckages, right?
No, I meant that we perhaps should allow unversioned private packages to depend on skipped packages. Those parhaps shouldn't reach the relevant functions on many occassions but I couldn't see a strong reason for gating the new behavior on config.privatePackages.version === true
There was a problem hiding this comment.
I think unversioned private packages were already implicitly exempt — since they never go through the versioning pipeline, they wouldn't trigger this validation in practice. So the net new behavior is really for versioned private packages, which is what the changeset describes.
Happy to update the wording if you feel it's misleading, but I think it's accurate as-is.
There was a problem hiding this comment.
I'll take care of the wording. That's not an issue. I just want to doublecheck the original intention of the PR. Notice that I already simplified the condition here - I'm just making sure it was the right call now.
There was a problem hiding this comment.
@Andarist personally I think it was better as it was before, as only if config.version is set we consider this an app and not a package based on the docs: https://github.com/changesets/changesets/blob/main/docs/versioning-apps.md
There was a problem hiding this comment.
but if it works, great!! But you may end up with published package.json files that depend on things that aren't being published (even if to a private registry). Not sure if that's OK? Maybe it is if they are devDependencies?
|
Overall, LGTM - but I asked one question that has to be resolved before we could land this. Pending the answer, we might have to adjust the added changeset and the docs change. |
Andarist
changed the title
…t requiring them to also be skipped (changesets#1842) * allow skipping apps dependencies * more fixes * lint * simplify * tweak * tweak * edit changesets --------- Co-authored-by: Mateusz Burzyński <[email protected]>
2 participants
Fixes #906
Fixes #1093
Fixes #1167
Summary
Allow versioned private packages (apps) to depend on skipped/ignored packages without erroring.
Previously,
changesets versionwould fail with "depends on the skipped package" even when the dependent was a private package that would never be published to npm. This fix applies to both the--ignoreCLI flag (@changesets/cli) and theignoreconfig option (@changesets/config).packages/cli/src/run.test.tsfor the CLI validation pathpackages/config/src/index.test.tsfor the config validation path