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.

Sign up for GitHub

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

Jump to bottom

fix: updates to stream types #365

Merged
merged 2 commits into from
May 18, 2021
Merged

fix: updates to stream types #365

merged 2 commits into from
May 18, 2021

Conversation

ecwyne
Copy link
Contributor

@ecwyne ecwyne commented Nov 18, 2020

Types for client.stream() callback handlers had incorrect number of arguments. This PR also adds types for client.stream.document() and correctly allows snapshot events in client.stream.document() and not in client.stream()

@ecwyne
Copy link
Contributor Author

ecwyne commented Nov 18, 2020 •
edited
Loading

Here's a small reproduction of the issue running against v4.0.0

https://codesandbox.io/s/wispy-grass-izf2d?file=/src/index.ts

@artem-kurnikov
Copy link
Contributor

artem-kurnikov commented Mar 18, 2021 •
edited by fireridlle
Loading

Hey, @ecwyne!
Thank you for your contribution, changes look good to me! One issue #440 with the .document method I've fixed in this PR - #439. So there're merge conflicts for your changes due to this. Could you please resolve them so that I could merge your changes as well?

@artem-kurnikov
Merge branch 'master' into stream-types
e3b43a3 
# Conflicts:
#	src/types/Client.d.ts
@fireridlle fireridlle mentioned this pull request Apr 29, 2021
Closed
@fireridlle fireridlle requested review from marrony and n400 May 17, 2021 07:40
@fireridlle fireridlle changed the title updates to stream types fix: updates to stream types May 17, 2021
erickpintor
erickpintor approved these changes May 17, 2021
View reviewed changes
Copy link
Contributor

@erickpintor erickpintor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@artem-kurnikov artem-kurnikov merged commit 1d67e3c into fauna:master May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fireridlle fireridlle fireridlle approved these changes

@erickpintor erickpintor erickpintor approved these changes

@parkhomenko parkhomenko Awaiting requested review from parkhomenko

@marrony marrony Awaiting requested review from marrony

@n400 n400 Awaiting requested review from n400

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ecwyne @artem-kurnikov @erickpintor @fireridlle