fix(dep-check): substitute a sibling whose contract moved without its version (usetheokit/theokit#659) - #58
Merged
Merged
Conversation
… version (usetheokit/theokit#659) Check D skipped any sibling whose version number the registry already had. That answers "does this version exist?", which is a different question from "is the published copy the one this workspace is about to publish?". A release that widens a peer range across two workspace packages moves the contract and not the version — changesets bumps at version time, not at pull-request time. So the leg packed tomorrow's package, resolved yesterday's sibling from npm, and reported two copies of a peer: a pair publication never produces. It is a required check, so the release could not merge, and no ordering of the old design avoided it — bumping first makes the packed tarball ask for a version npm does not have, and the install answers ETARGET instead. A sibling now also earns its local tarball when the registry serves its version with different dependencies, peerDependencies or optionalDependencies. The narrowness is deliberate: an identical contract is still installed from the registry, which is what makes the leg test what a consumer resolves. Absent evidence answers "unchanged" — a registry that returns no manifest must not read as "the contract moved", or every sibling gets a local tarball and the leg tests nothing. Each substitution now records WHICH gap it filled, and the note says so. The single message claimed the registry did not have the version, which is false for this case; a note naming the wrong reason is worse than none, because it is read and believed. Proved in both directions against the real usetheokit/theokit checkout, with the condition reconstructed on disk (a peer range widened without a version bump): with the fix @theokit/presenter@0.9.0-next.0 substituted, note explains why fix reverted no substitution at all — the blind spot, reproduced 109 unit tests pass, including the pre-existing one asserting that a sibling the registry already serves is NOT substituted, which is the guarantee this must not break.
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
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.
Fixes the blind spot reported as usetheokit/theokit#659, which blocked a release PR whose artifact was correct.
The wrong question
unpublishedSiblingsskipped any sibling whose version number the registry already had:That answers does this version exist?, which is a different question from is the published copy the one this workspace is about to publish?
A release that widens a peer range across two workspace packages moves the contract and not the version, because changesets bumps at version time and not at pull-request time. So check D packed tomorrow's package, resolved yesterday's sibling from npm, and reported two copies of a peer — a pair publication never produces.
It is a required check, and no ordering of the old design avoided it: bumping first makes the packed tarball ask for a version npm does not have, and the install answers
ETARGETinstead.The fix, and why it is narrow
A sibling also earns its local tarball when the registry serves its version with different
dependencies,peerDependenciesoroptionalDependencies.An identical contract is still installed from the registry. That narrowness is the point — it is what makes check D test what a consumer actually resolves, and the pre-existing test asserting it (
test_leaves_a_sibling_the_registry_already_has_to_be_installed_from_the_registry) still passes untouched.Absent evidence answers unchanged: a registry that returns no manifest must not read as "the contract moved", or every sibling gets a local tarball and the leg tests nothing.
The note was lying
One message explained every substitution with "the registry does not have that version yet" — false for this case, where it has it with a different contract. Each substitution now records which gap it filled and the note says so. A note naming the wrong reason is worse than none, because it is read and believed.
Proved in both directions
Against the real
usetheokit/theokitcheckout, with the condition reconstructed on disk (a peer range widened without a version bump):109 unit tests pass.
Reach
@theokit/dep-checkis pinned to an exact version inside the reusable workflow, so no consumer's gate changes until that pin is bumped deliberately.