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

feat(apollo-client): Adds type vars to subscribeToMore for variables #4608

Conversation

timhwang21
Copy link
Contributor

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

Addresses #4246.

I was having some issues installing all needed dependencies, but this is just a type change and TSC doesn't show any errors for me.

@apollo-cla
Copy link

@timhwang21: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@timhwang21
Copy link
Contributor Author

I wasn't able to run the test locally, and it seems like the failure is just due to me not setting the tests up locally. The only thing the test does is add some dummy variables to the query and the subscription. Would really appreciate it if a maintainer could take a look!

@benjamn
Copy link
Member

benjamn commented Apr 6, 2019

Two notes:

@timhwang21 timhwang21 force-pushed the devs/timothy/improve-subscribe-types branch from bf0777d to 9ff1608 Compare April 6, 2019 16:12
@timhwang21
Copy link
Contributor Author

Sounds good -- I just rebased and dropped duplicate changes (though I kept a rename of TVariables to TSubscriptionVariables as I think it's more accurate). Will try to solve the test failure next, but was hoping you can take a look as well, since the issue is definitely with my test and me not configuring it properly.

The only thing the test is testing anyways is a situation that should typecheck...

@timhwang21 timhwang21 force-pushed the devs/timothy/improve-subscribe-types branch from 9ff1608 to 12f0283 Compare April 6, 2019 16:24
@benjamn benjamn merged commit bac653b into apollographql:master Apr 8, 2019
@timhwang21 timhwang21 deleted the devs/timothy/improve-subscribe-types branch April 8, 2019 15:14
benjamn added a commit that referenced this pull request Apr 8, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants