Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stream messages are actually items of a collection #1100

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

challet
Copy link
Contributor

@challet challet commented Nov 13, 2024

When using the stream function of the call builders, typescript thinks the messages passed to the onmessage callback are of type ServerApi.CollectionPage<TypeFromTheBuilderClass> when they are actually single items from the said collection.

@challet
Copy link
Contributor Author

challet commented Nov 13, 2024

Duplicate of #1040, not made at the exact same place though

@Shaptic
Copy link
Contributor

Shaptic commented Nov 13, 2024

@challet nice find! No idea why that hadn't been merged before 😢 I'm happy to merge yours but I'm curious if you think fixing it at the type source (EventSourceOptions) in #1040 is cleaner.

@challet
Copy link
Contributor Author

challet commented Nov 13, 2024

I'm wondering it too :)
The result is exactly the same but here it keeps the EventSourceOptions as a generic interface, which might eventually be re-used somewhere else.

@challet
Copy link
Contributor Author

challet commented Nov 13, 2024

However, inferring the type from the generic parameter is better than extracting it from how it is applied into the inner types.

@challet
Copy link
Contributor Author

challet commented Nov 13, 2024

I don't understand what's wrong with prettier here. It doesn't change a file by running it locally and the yarn command is made to only apply on './test/**/*.js'

@Shaptic
Copy link
Contributor

Shaptic commented Nov 13, 2024

Eh don't worry about it 👍

@Shaptic Shaptic merged commit 94e1ce8 into stellar:master Nov 13, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants