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(graphiql-ws): add websocket support to graphiql #302

Closed
wants to merge 1 commit into from

Conversation

DxCx
Copy link
Contributor

@DxCx DxCx commented Feb 8, 2017

Hi @helfer
I've added web-socket support to graphiql (Running the new proposed WS protocol)
please start with reviewing process, i'll merge after the whole ws integration suite will be ready for merge.

TODO:

  • Update CHANGELOG.md with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

Closes #222
Related to #272

Copy link
Contributor

@helfer helfer left a comment

Choose a reason for hiding this comment

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

This is pretty neat @DxCx! I like how simple the websocket fetcher is. The code should in my opinion be refactored a bit to make it easier to read. The ternary operators with long strings between them are not very legible.
Apart from changing the message format to match the protocol we end up deciding on, I think everything else looks pretty good. We should continue the discussion about the diff format in the other thread.

I'm not a fan of including all of RxJs, but since it's a developer tool the initial startup time probably isn't critical.

@DxCx
Copy link
Contributor Author

DxCx commented Feb 10, 2017

@helfer im planning in the future to replace this code with one that takes the apollo-client's network interface and use it instead.
because of that i approved myself use RxJs in the time being as the client side is not ready yet.

@DxCx
Copy link
Contributor Author

DxCx commented Feb 17, 2017

Replaced by #308

@DxCx DxCx closed this Feb 17, 2017
@DxCx DxCx deleted the graphiql-ws branch February 17, 2017 16:10
trevor-scheer pushed a commit that referenced this pull request May 6, 2020
Co-authored-by: Renovate Bot <bot@renovateapp.com>
trevor-scheer pushed a commit that referenced this pull request May 14, 2020
Co-authored-by: Renovate Bot <bot@renovateapp.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants