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

Discussion: grapql and/or websockets support? #83

Open
josx opened this issue Oct 23, 2018 · 6 comments
Open

Discussion: grapql and/or websockets support? #83

josx opened this issue Oct 23, 2018 · 6 comments

Comments

@josx
Copy link
Owner

josx commented Oct 23, 2018

  • Both react-admin and feathers support at least graphql
  • Feathers support websockets but react-admin not out of the box

is there any way we can support that?

@nicholasnelson
Copy link
Contributor

Websockets: Yes. Feathers abstracts the transport from the consumer. Whether feathersClient is configured for REST, Websockets, etc, etc should be irrelevant to ra-data-feathers.

Having said that, I've found a bug that prevents Websockets specifically from working with ra-data-feathers. Full details in #84. I've written a fix for this, will PR shortly.

Graphql: Should just work. I don't have a graphql setup to easily test this, but looking at feathers-plus/graphql, it appears that graphql is just mounted as a Feathers endpoint like any other service, and queried by passing graphql queries in params.query. Would be interesting to try this out. Will do it if I get some time.

@nicholasnelson
Copy link
Contributor

nicholasnelson commented Oct 25, 2018

Ok so the graphql thing is a bit more complex than I originally thought.

The main issue is that feathersClient does not support graphql. Feathers itself supports the creation of a graphql endpoint, however the client only supports REST, Websockets, and Primus. It is plausible to use REST / Websockets and run all queries against feathersClient.service('graphql'), but is there a good reason to do this that outweighs the ease of use afforded by REST?

The data provider needs to know about the graphql schema to know which fields to include in the graphql query. This means either completing an 'introspection query', which would slow down the initial load of the provider, or alternatively providing the schema as part of the client package, which will "significantly increase the bundle size" according to the ra-data-graphql documentation.

Using graphql also creates additional work on the client which has to:

  • Look up the schema for the requested resource
  • Write a query including all fields from the requested resource
  • Reformat the results into a format react-admin can use

The main reason I can see to use graphql in this case (where the actual query syntax is hidden from the user and so any qualitative elegance gain of using graphql is irrelevant) is possible efficiency improvements in being able to include related records in a single query. However react-admin generates multiple requests for getting related records anyway, and expects information about a single resource per request; so I don't see how we'd be able to use this advantage without modifying the behaviour of react-admin.

The other reason to use graphql could be if graphql is the only endpoint available, but given that REST can be enabled in Feathers with a couple of lines, I don't see this being a major problem.

@josx Do you have a use-case for graphql in this project? Or is this more of a "It would be nice if we could support this, can it be done?" question?

EDIT: Just to add to the above, 'feathers-plus generate' doesn't include introspection types from what I can see, and so it becomes necessary to either write custom introspection types on the server side, or include the schema in the client.

@josx
Copy link
Owner Author

josx commented Oct 26, 2018

I dont have any real use-case for graphql, was just "would be nice to have it"

So, We can wait until someone has a real use-case and want to contribute here!

@josx
Copy link
Owner Author

josx commented Oct 26, 2018

if some one wants to use it vía websockets, do we need to add some more documentation about that?

@nicholasnelson
Copy link
Contributor

Whoops, I'm certain I wrote a reply to this but I must have not hit the button.

I don't think this needs to be documented here. The transport used by Feathers Client is abstracted away from ra-data-feathers. Feathers client could be using Mystery Protocol X, but it still presents the same service.method(params) functions to us.

The documentation currently includes:

Configuration of the Feathers client is beyond the scope of this document. See the Feathers documentation for more information on configuring the Feathers client. Both of the following need to be configured in the Feathers client for use with ra-data-feathers.

- A client type such as REST, Socket.io, or Primus
...

In my view, pointing out that the Feathers client must be configured, that the user is free to use REST, Socket.io, or Primus, and that configuration of these client types in the Feathers client is documented in the Feathers project should be sufficient. Additionally, it may be detrimental to document configuration of Feathers client here, as we'd need to ensure this documentation is updated in the event of any changes to the configuration options for Feathers client.

An alternative approach if you feel that more explanation is required could be to create a new example project showing the full configuration of ra-data-feathers, feathers client, etc. The existing example project was written when react-admin was still admin-on-rest and ra-data-feathers was aor-feathers-client and could probably do with being updated/replaced.

@josx
Copy link
Owner Author

josx commented Nov 20, 2018

Maybe as you said We can replace the existing example with a newer one.

@josx josx mentioned this issue Oct 2, 2019
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

No branches or pull requests

2 participants