feat(dynamic-client): extract dynamic-instructions#999
Conversation
* feat: extract dynamic-instructions package * feat: add dynamic-instructions to dynamic-client - Cleaned dynamic-client from dynamic-instructions and import dynamic-instructions * fix: cleanup * fix: cleanup * fix: cleanup * fix: add note to readme * chore: add note to dynamic-instruction typesgen readme * fix: dynamic-client index re-exports
🦋 Changeset detectedLatest commit: 91a79f1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
This PR extracts the instruction-building logic out of @codama/dynamic-client into a new @codama/dynamic-instructions package, mirroring the split done for dynamic-address-resolution in #996. dynamic-client now consumes the new package via MethodsBuilder and re-exports a smaller surface; everything that used to live under dynamic-client/src/instruction-encoding, plus the relevant shared helpers and tests, is moved over with the imports rewritten.
Alongside the move, the public API picks up generics (TArgs, TAccounts, TSigners, TResolvers) so generated types can flow through, createIxBuilder is renamed to createInstructionsBuilder, and BuildIxFn becomes InstructionsBuilderFn. Tests are rebuilt against in-memory IDL nodes instead of fixture JSON, which is a nice cleanup — the new package has no fixture/SVM dependency.
Overall the split is clean and the diff is mostly mechanical. A few things worth a closer look below, none of them blocking.
Things to watch out for
- Behavior change in
encodeInstructionArguments:mergeBytes(chunks as Uint8Array[])is nowmergeBytes(chunks.map(chunk => Uint8Array.from(chunk))). That's a real per-chunk copy on every encode call (notably hot forremainderOptionTypeNode(bytesTypeNode())payloads likewrite). Inline below — would be good to confirm this was intentional and, if so, add a comment. - Lost memoization:
getMemoizedUtf8Encoder()was removed fromdynamic-client/src/shared/codecs.tsand the one remaining caller invalidators.tsnow callsgetUtf8Encoder()directly inside a validator closure. Probably negligible, but the memoization was deliberate. Inline below. - Changeset bump levels:
dynamic-instructionsminor /dynamic-clientpatch looks right — the new package is 0.1.0, and the only user-visible change indynamic-clientis that it now (transitively) depends ondynamic-instructions. The renames (createIxBuilder,BuildIxFn) were internal todynamic-clientand not re-exported from its index, so no breaking external surface there. - README cross-link:
packages/dynamic-client/README.mdnow points to../dynamic-address-resolution/README.md#automatic-resolution-rules. Worth double-checking that anchor exists onmain(or is added in the #996 stack) so the link doesn't 404 on npm.
Notes for subsequent reviewers
- The deleted
dynamic-client/src/shared/{bytes-encoding,codecs}.tshad a handful of memoized codec/encoder helpers and agetCodecFromBytesEncodingutility. OnlygetMemoizedUtf8Encoderhad a remaining caller (invalidators.ts); the rest were only used by deleted tests. Worth a quick search to confirm nothing else in the repo (outside these two packages) imported fromdynamic-client/src/shared/*— I didn't see anything but it's easy to miss. InstructionsBuilderFnnow has four generic parameters;TSigners extends EitherSigners = EitherSignersdoesn't add any narrowing power (EitherSignersis juststring[]). Harmless but worth a thought as to whether it earns its place in the public type signature.- The new tests construct IDL nodes inline via
codama's node helpers — this is a good direction and removes the fixture coupling, but worth scanning for whether any meaningful coverage that depended on real fixtures (e.g. thepmp-idl.jsoninitialize roundtrip viagetInitializeInstructionDataDecoder) has been dropped without replacement. Looks like thedefinedTypeLinkNode → enumTypeNodecase is still covered with a synthetic enum, which is the main thing.
lorisleiva
left a comment
There was a problem hiding this comment.
Thanks! Trusting your judgement on that one as well. 🙏
Description
This PR [2] extracts
dynamic-instructionsinto a separate package with minimal changes. Packagedynamic-clientwas cleaned again and now usesdynamic-instructions.Previous PR#996 contained split for dynamic-address-resolution
Change
dynamic-instructionspackage with initial structure (LICENSE, README, configs).dynamic-instructionsincluding args encoding, validation.dynamic-address-resolutionfor Account address auto-resolution.dynamic-clientwith@codama/dynamic-instructions.Public API