Repository navigation
Preserve fixedSize for single-field struct codecs - #1971
Merged
mcintyre94 merged 2 commits intoAug 27, 2026
Merged
Conversation
Forward the only field’s literal fixedSize through struct encoder, decoder, and codec overloads while keeping multi-field structs on the existing number fallback. Add focused typetests and a patch changeset.
🦋 Changeset detectedLatest commit: 18d0aeb 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 |
mcintyre94
requested changes
Aug 27, 2026
mcintyre94
left a comment
mcintyre94
left a comment
Member
There was a problem hiding this comment.
Thanks for this! We need to tighten these typetests up a bit but this looks like a great solution to me!
|
|
||
| import { getStructCodec, getStructDecoder, getStructEncoder } from '../struct'; | ||
|
|
||
| type IsWidenedNumber<T extends number> = number extends T ? true : false; |
Member
There was a problem hiding this comment.
I appreciate what you're doing here, we want to check that the type is exactly Number and not a specific fixed size literal.
We have precedent for this with Equal:
Please could you copy over that and its long explanatory comment? It covers some edge cases that this one doesn't, and has a very detailed explanation
| } | ||
|
|
||
| { | ||
| // [getStructEncoder/Decoder/Codec]: It only preserves literal sizes for exactly one fixed-size field. |
Member
There was a problem hiding this comment.
Please only test one of these at a time, so three different blocks for these tests
Comment on lines
+95
to
+97
| true satisfies IsWidenedNumber<typeof encoder.fixedSize>; | ||
| true satisfies IsWidenedNumber<typeof decoder.fixedSize>; | ||
| true satisfies IsWidenedNumber<typeof codec.fixedSize>; |
Member
There was a problem hiding this comment.
This should be 3 blocks too, but to demonstrate how we'd use Equal here, this is how I'd write the encoder test:
{
// [getStructEncoder]: Two fixed-size fields keep a widened size; it is not narrowed to a literal.
const encoder = getStructEncoder([
['a', {} as FixedSizeEncoder<number, 4>],
['b', {} as FixedSizeEncoder<number, 8>],
]);
encoder satisfies FixedSizeEncoder<{ a: number; b: number }>;
true satisfies Equal<typeof encoder.fixedSize, number>;
}}Use the repository’s strict Equal helper and separate encoder, decoder, and codec assertions so widened fixed sizes are checked exactly as number.
Contributor
Author
|
Thanks! Addressed in 18d0aeb. I copied the existing Re-ran package typecheck, |
Member
BundleMonUnchanged files (150)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
This PR adds a narrow overload to getStructEncoder, getStructDecoder, and getStructCodec that forwards the field's literal fixedSize when the struct has exactly one fixed-size field, plus focused typetests and a patch changeset. No runtime changes — overloads only.
The solution looks solid and the typetests are now nicely tightened per the prior review — this would be an approval, but I don't have approve permissions on this repo:
- Overload resolution is correct. The single-element-tuple constraint (
readonly [readonly [string, FixedSizeEncoder<any>]]) only matches literal one-field tuples. Multi-field tuples, non-tupleFields<...>arrays, and[]all fall through to the existing widened overloads — and the newEqual-based typetests pin each of those cases down exactly, whichsatisfies numberalone couldn't do (a literal4would still satisfynumber). - No generic-call-site regressions. A call with a generic
TFields extends Fields<FixedSizeEncoder<any>>doesn't match the tuple overload and resolves to the widened one, same as before. - Changeset level looks right. Narrowing
numberto a literal is non-breaking for consumers (FixedSizeEncoder<T, 4>remains assignable toFixedSizeEncoder<T, number>), so patch fits.
Two minor nits inline, neither blocking: the Equal helper's doc comment describes rationale that doesn't match its actual use in this file, and the JSDoc attachment in struct.ts shifts to the new overload (worth a quick hover check in an editor).
For subsequent reviewers: the main thing to verify is that the generated .d.ts output keeps the overload order intact — the narrow overload must stay first for resolution to work in consuming packages. The PR author reports a declaration-consumer typecheck was run, which covers this.
mcintyre94
approved these changes
Aug 27, 2026
mcintyre94
left a comment
mcintyre94
left a comment
Member
There was a problem hiding this comment.
Awesome, thanks for this! 🚀
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.
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.
Problem
Single fixed-size field structs currently widen their literal
fixedSizetonumber.Context
A broader type-arithmetic solution was explored in #1761. Maintainer feedback identified exactly-one-fixed-size-field support as a simpler middle ground.
Summary of Changes
Add narrow overloads that forward the only field's existing literal
fixedSizeforgetStructEncoder,getStructDecoder, andgetStructCodec. Add focused typetests and a patch changeset for@solana/codecs-data-structures.Multi-field structs intentionally retain their existing
numberbehavior. This introduces no type arithmetic and no runtime behavior changes.Tests
pnpm --filter @solana/codecs-data-structures test:typecheckpnpm turbo compile:typedefs --filter=@solana/codecs-data-structures...pnpm --filter @solana/codecs-data-structures test:lintpnpm --filter @solana/codecs-data-structures test:prettierpnpm --filter @solana/codecs-data-structures test:unit:nodepnpm --filter @solana/codecs-data-structures test:unit:browserpnpm --filter @solana/codecs-data-structures compile:js, with Node, browser, and native runtime bundles byte-identical to the pristine buildpnpm changeset statusgit diff --checkAI assistance
AI assistance was used during implementation and verification. Shin reviewed the final change and owns the submission.
Fixes #1738