fix(dep-check): a workspace: range is not a moved contract (0.11.1) - #60
Merged
Conversation
0.11.0 compared the workspace manifest against the published one key by key, and `pnpm pack` rewrites `workspace:^` to the version it resolves to. The two therefore differ on that key in EVERY package that depends on a sibling, so `contractMoved` was true almost everywhere — handing a local tarball to nearly every sibling and undoing the exact narrowness the comparison exists to keep. Measured on the usetheokit/theokit workspace: three packages substituted with no contract having moved, each one reported with a reason that was not true of it. The pre-existing test protecting the narrowness passed throughout, because it uses literal ranges and never a workspace protocol — which is why a test for this case is added rather than an assertion tightened. A local protocol (workspace:, link:, file:, portal:) is a placeholder rather than a contract, and whether the version it resolves to is published is already the question the unpublished branch answers. Those keys are skipped. Re-proved on the real checkout, both directions: #659 condition on disk only @theokit/presenter substituted, reason correct condition removed no substitution at all 0.11.0 is published and should not be used. The defect makes the check weaker, never falsely blocking, but the pin moves to 0.11.1 in the same commit so no caller stays on it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corrects a false positive shipped in 0.11.0, which the pin currently points at.
What 0.11.0 got wrong
The contract comparison ran key by key over the workspace manifest and the published one.
pnpm packrewritesworkspace:^to the version it resolves to, so the two differ on that key in every package that depends on a sibling:contractMovedwas therefore true almost everywhere. The substitution meant to be narrow became broad — handing a local tarball to nearly every sibling, which is precisely what stops check D testing what a consumer resolves.Measured on the real
usetheokit/theokitworkspace: three packages substituted with no contract having moved, each reported with a reason that was not true of it.Why the existing test did not catch it
test_leaves_a_sibling_the_registry_already_has_to_be_installed_from_the_registryprotects exactly this narrowness and passed throughout — it uses literal ranges and never a workspace protocol. So this adds a case rather than tightening an assertion.The fix
A local protocol (
workspace:,link:,file:,portal:) is a placeholder, not a contract, and whether the version it resolves to is published is already the question theunpublishedbranch answers. Those keys are skipped, reusing theLOCAL_PROTOCOLmatcher the module already had.Re-proved on the real checkout
110 unit tests pass.
On 0.11.0
It is published and should not be used. Its defect makes the check weaker — a local tarball where the registry copy would have been more honest — and cannot falsely block a release. The pin moves to 0.11.1 in the same commit so no caller stays on it.