fix!: use arrays for collections instead of sets #70
Conversation
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
Because we have `uniqueItems: true` in our service definition yaml, fields with `type: array` are generated as `Set`s by the typescript generator. Serialization/deserialization of these types is [broken](OpenAPITools/openapi-generator#11746) so where we use these APIs [we pass arrays with `@ts-expect-error` anyway](https://github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts#L155). It doesn't look possible to override the use of Set by the generator in a way that works (`type-mappings=set=array` produces uncompilable code) so manually change the generated code. Also adds a note to the readme reminding people to do it. BREAKING CHANGE: fields that were Sets such as `PinResults.results` and `PinsGetRequest.cid` are now Arrays
achingbrain
force-pushed
the
fix/use-arrays-for-collections
branch
from
7a25aa1 to
ca888c4
Compare
Member
Author
SgtPooki
approved these changes
Oct 7, 2024
Member
SgtPooki
left a comment
SgtPooki
left a comment
There was a problem hiding this comment.
I'm not a big fan of having to manually modify more of the generated code here, but I'm okay with this change if you're recommending it.
Member
Author
|
Me neither TBH, I've opened ipfs/pinning-services-api-spec#112 but who knows if we're making changes here any more. |
github-actions bot
pushed a commit
that referenced
this pull request
Oct 8, 2024
## [3.0.0](v2.0.0...v3.0.0) (2024-10-08) ### ⚠ BREAKING CHANGES * fields that were Sets such as `PinResults.results` and `PinsGetRequest.cid` are now Arrays ### Bug Fixes * use arrays for collections instead of sets ([#70](#70)) ([75736f5](75736f5)), closes [/github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts#L155](https://github.com/ipfs-shipyard//github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts/issues/L155)
|
🎉 This PR is included in version 3.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Because we have
uniqueItems: truein our service definition yaml, fields withtype: arrayare generated asSets by the typescript generator.Serialization/deserialization of these types is broken so where we use these APIs we pass arrays with
@ts-expect-erroranyway.It also makes pagination harder to use, since we need to set the
.afterfield to get the next page of pins - sets have no guarantee of order so you have to first convert the current set of results into an array and sort it by creation date which is inefficient.It doesn't look possible to override the use of Set by the generator in a way that works (
type-mappings=set=arrayproduces uncompilable code) so manually change the generated code in the same way we have to change the import settings (due, I think, to another option that doesn't work).Also adds a note to the readme reminding people to do it.
BREAKING CHANGE: fields that were
Sets such asPinResults.resultsandPinsGetRequest.cidare nowArrays