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

fix(visitor-plugin-common): also include empty selection set types/generate valid type output #2490

Merged
merged 3 commits into from
Sep 12, 2019

Conversation

n1ru4l
Copy link
Collaborator

@n1ru4l n1ru4l commented Sep 3, 2019

This fixes the issue occurring in #2489

@n1ru4l n1ru4l changed the title test: add failing test for #2489 fix(visitor-plugin-common): also include empty selection set types/generate valid type output Sep 3, 2019
@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Sep 3, 2019

There is an issue with flow types that must be fixed: d49dbba

I am on vacation for the next two weeks so someone else could pick this up and finish it. Typescript part is fine.

@dotansimha
Copy link
Owner

dotansimha commented Sep 4, 2019

Thanks @n1ru4l ! We'll take a look soon :)

@ardatan

@dotansimha dotansimha requested a review from ardatan September 4, 2019 05:16
@dotansimha
Copy link
Owner

Thanks @n1ru4l , I'm doing some more fixes for the selection set transformation, also changed this current fix to output more clear code (without empty objects).
I'll push it to master when it's done.

Thanks!!

@dotansimha dotansimha merged commit d49dbba into dotansimha:master Sep 12, 2019
@n1ru4l n1ru4l deleted the test-2489 branch September 12, 2019 10:00
@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Sep 12, 2019

@dotansimha Removing the empty objects could possibly result in incorrect types

@dotansimha
Copy link
Owner

Yeah it's related to this as well #2540
I'm working on a fix now. Thanks @n1ru4l !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants