Allow narrower boolean predicates in pattern match codecs - #1809
Merged
Conversation
🦋 Changeset detectedLatest commit: 1acc0d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 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 |
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mcintyre94
marked this pull request as ready for review
July 1, 2026 13:59
BundleMonUnchanged files (150)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
Member
Author
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Aligns getPatternMatchCodec / getPatternMatchEncoder predicate typing with getPredicateCodec / getPredicateEncoder by making the boolean-predicate parameter bivariant. Previously, predicates had to be typed against the full value type of the matched variants (e.g. (value: number | bigint) => ... for the number codecs), whereas the sibling predicate APIs already infer the predicate type independently and accept (value: number) => .... The fix introduces a BivariantBooleanPredicate<TFrom> helper ({ predicate(value: TFrom): boolean }['predicate'] — a method-syntax escape hatch that reintroduces bivariance under strictFunctionTypes) and uses it for the boolean-predicate branches of both PatternMatchEncoderEntry and PatternMatchCodecEntry. Encoder/codec assignability is untouched, so the existing cross-branch type-consistency check and size inference still hold.
Correctness
- The bivariant trick is narrowly scoped to the predicate parameter only — the encoder/codec slot in the tuple is unchanged, so the invariant that all boolean-predicate branches must share the same
TFrom(andTTofor codecs) is preserved. The existing@ts-expect-errorcase for mixingEncoder<number>andEncoder<string>still fires, confirmed by the untouched negative typetests. - The order of the two union members was swapped so that the type-guard variant (
(value: TFrom) => value is TNarrowed) comes first and the boolean variant second. This is semantically neutral for assignability (both are alternatives in a union) but a nicer default for error messages. A type-guard is trivially assignable to the boolean-predicate slot too now that the parameter is bivariant, but since the codec/encoder type in each tuple variant still pins the correct shape, there's no accidental widening. - The runtime code is entirely unchanged; this is a pure type-level refinement.
Consistency with getPredicateCodec
Worth noting for future readers: this brings the same runtime foot-gun that getPredicateCodec already has — a caller can now type a predicate more narrowly than the codec's actual value type, and at runtime the predicate will be invoked with values outside its declared parameter type (e.g. a (v: number) => v < 256 predicate can be handed a bigint and will throw a TypeError on the comparison). That's an accepted tradeoff for ergonomics and is consistent with the sibling API, but it's the reason to keep the boolean-predicate path opt-in via a plain arrow (versus type-guard predicates which retain full parameter variance).
Test coverage
The new typetest blocks — one for getPatternMatchEncoder, one for getPatternMatchCodec — exercise exactly the scenario from the PR description: a (value: number) => boolean predicate paired with an Encoder<bigint | number> / Codec<bigint | number, number>. The existing negative typetests for cross-branch type mismatches still pass, which is the key safety property.
The docs example revert to (value: number) is a nice touch — the previous (value: number | bigint) was a workaround for exactly this issue.
Changeset is present and correctly scoped to @solana/codecs-data-structures as a patch (this is a strict typing relaxation with no runtime or API changes).
Notes for subsequent reviewers
- Verify the two new typetest blocks compile as expected (they should — the bivariant method syntax is a well-known TS escape hatch used elsewhere in the ecosystem, e.g. React's event handler types).
- Nothing to check on the docs change beyond confirming the code snippet still renders correctly on solanakit.com after publish.
LGTM — clean, well-scoped, and the test coverage matches the change surface.
Boolean predicates passed to `getPatternMatchCodec` and `getPatternMatchEncoder` previously had to be typed against the full value type of the matched variants. Because the number codecs accept `number | bigint`, matching against them forced predicates such as `(value: number | bigint) => ...` even when the caller only handles `number`, unlike `getPredicateCodec`/`getPredicateEncoder` which infer the predicate type independently and accept `(value: number) => ...`. This aligns the two APIs by checking the boolean predicate parameter bivariantly via a method-syntax escape hatch, so a predicate that narrows to a subtype of the variant's value type is accepted. Codec and encoder assignability are untouched, so the existing cross-branch type-consistency check and size inference still hold. Adds typetests covering the new behavior for both the encoder and codec, and reverts the docs example back to plain `(value: number)`.
mcintyre94
force-pushed
the
narrower-pattern-match-predicates
branch
from
July 1, 2026 14:24
04cc193 to
1acc0d9
Compare
Contributor
|
Documentation Preview: https://kit-docs-cngdfs1m3-anza-tech.vercel.app |
lorisleiva
approved these changes
Jul 13, 2026
Comment on lines
+12
to
+15
| // A boolean predicate whose parameter is checked bivariantly (method-syntax escape hatch), so a | ||
| // predicate that narrows to a subtype of the variant's value type — e.g. `(value: number)` against a | ||
| // number codec whose value type is `number | bigint` — is still accepted, mirroring `getPredicateCodec`. | ||
| type BivariantBooleanPredicate<TFrom> = { predicate(value: TFrom): boolean }['predicate']; |
Member
There was a problem hiding this comment.
Interesting TS trick. TIL.
This was referenced Jul 13, 2026
Merged
Contributor
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 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.
Boolean predicates passed to
getPatternMatchCodecandgetPatternMatchEncoderpreviously had to be typed against the full value type of the matched variants. Because the number codecs acceptnumber | bigint, matching against them forced predicates such as(value: number | bigint) => ...even when the caller only handlesnumber, unlikegetPredicateCodec/getPredicateEncoderwhich infer the predicate type independently and accept(value: number) => ....This aligns the two APIs by checking the boolean predicate parameter bivariantly via a method-syntax escape hatch, so a predicate that narrows to a subtype of the variant's value type is accepted. Codec and encoder assignability are untouched, so the existing cross-branch type-consistency check and size inference still hold. Adds typetests covering the new behavior for both the encoder and codec, and reverts the docs example back to plain
(value: number).After this we can update the docs back: #1810