fix(store): do not render push and pop for static arrays, use static length [M-02] #2175
Conversation
🦋 Changeset detectedLatest commit: dabf602 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 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 |
packages/store/ts/codegen/field.ts
Outdated
| renderWithStore( | ||
| storeArgument, | ||
| ({ _typedStore, _store, _commentSuffix, _methodNamePrefix }) => ` | ||
| if (field.typeWrappingData && field.typeWrappingData.kind === "staticArray") { |
There was a problem hiding this comment.
I don't love this check as is, we repeat it for pop and push, then again for length.
It's also weird to treat static arrays as dynamic with the if (field.isDynamic) check, but then differentiate them using this.
There was a problem hiding this comment.
curious if @dk1a has any insight into this
There was a problem hiding this comment.
terminology is weird here
isDynamic, as well as dynamic and static table fields is a mud-specific thing
A better name for isDynamic might be isNonPrimitive - it indicates any array or struct
The word static in static arrays is used in an unrelated context, taken from solidity
There was a problem hiding this comment.
#1668 is related, I also don't like how hacky this check is, especially since now it's used in more places
Static arrays are now treated differently to dynamic arrays:
pushandpopmethods should not be used in static arrays so they are not rendered.lengthmethod now returns the hardcoded known length.