Change IInstruction to use ReadonlyUint8Array for data #3632
Conversation
🦋 Changeset detectedLatest commit: 584c637 The changes in this PR will be included in the next version bump. 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 |
BundleMonUnchanged files (125)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
| // TODO: This can just be `parseTransferSolInstruction(firstInstruction)` when the client is updated | ||
| // with the `@solana/web3.js` version that changes the instruction data type to `ReadonlyUint8Array` | ||
| const parsedFirstInstruction = parseTransferSolInstruction({ | ||
| ...firstInstruction, | ||
| data: firstInstruction.data as unknown as Uint8Array, | ||
| }); |
There was a problem hiding this comment.
We'll want to create a new build of the Codama JS clients with this change to make this go away.
This was actually where I first hit this issue. I was using an API that returned instruction data as base58, and then trying to convert its output into an IInstruction that I could parse with Codama. Since instruction data is Uint8Array this required me to cast my ReadonlyUint8Array from encoding the API's base58 data to a Uint8Array like this. I figured it makes sense to just widen the type on IInstruction as in this PR
|
A preview of the GitHub Pages site based on this PR is now available here: |
| '@solana/transaction-messages': patch | ||
| '@solana/instructions': patch | ||
| '@solana/errors': patch |
There was a problem hiding this comment.
This is debatable. I'm widening the type of IInstruction, so any code that creates one of those is unaffected, but any code that treats its data as mutable (relying on the existing narrower type) is affected. In our code nothing relies on it being mutable, but I needed to update some type params. Downstream consumers like Codama clients need to be updated to avoid any code changes for their dependencies. Happy to change this to whatever we think is best.
|
I've pushed a commit that refactors I noticed the type felt too restrictive because as-is we can't change Codama to use It also better matches what we do in eg Happy to revert if this is too much of a breaking change to justify though! Edit: On second thought, I don't think this adds much value, removed it. The change to |
mcintyre94
force-pushed
the
iinstruction-with-data-read-only
branch
from
2580fea to
584c637
Compare
|
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 PR modifies the
datafield ofIInstructionto useReadonlyUint8Array. This means that you can create anIInstructionusing bytes output from an encoder as input.