Data Module: Expose State using a GraphQL API by youknowriad · Pull Request #4083 · WordPress/gutenberg

youknowriad · 2017-12-19T11:41:36Z

This is a POC to trigger a discussion of whether we should continue this way or not. Alternative to #4105 closes #2473

So the idea here is that a module that wants to expose a specific part of its state registers a GraphQL schema describing the exposed data and a GraphQL resolver to fetch this data from the state. ( see editor/state/data.js for an example of a (schema, resolver) pair.

The data module exposes a registerSchema function to register the module's schema/resolver pair.

And a query Higher Order Component is also exposed to fetch data like so:

import gql from 'graphql-tag';
import { query } from '@wordpress/data';
const MY_QUERY = gql`query { editor { post { title } } }`;
const MyComponentWithData = query( MY_QUERY )( props => 
  <div>{ get( props.data, 'editor.post.title', 'no title' ) }</div>
);

youknowriad · 2017-12-19T11:41:54Z

cc @atimmer

aduth · 2017-12-19T13:56:07Z

data/query.js

+ */
+import { Component } from '@wordpress/element';
+
+const createQueryHigherOrderComponent = ( client ) => ( mapPropsToQuery, mapPropsToVariables = () => ( {} ) ) => ( WrappedComponent ) => {


Would these more accurately be mapQueryToProps and mapVariablesToProps ?

Or am I wrong in thinking that these functions would take values from the query result and map them to props for the component?

Actually, they are correctly names, these are useful if you want to create custom queries based on props.

I can see something like mapResultToProp being usefull as well. It could be a third optional callback.

Edit In a GraphQL API, this is less important though because you already ask for the data you want, it could be convenient though for "undefined" checks and stuff like that.

aduth · 2017-12-19T13:57:04Z

In your example, wouldn't you want a mapping function from the query result to the props to pass to the underlying component, like connect's mapStateToProps ? To make the component less dependent on the schema shape?

youknowriad · 2017-12-19T15:03:35Z

There's a bug in the production build npm run build but not sure why. It looks like it's caused by the webpack UglifyJsPlugin.

gziolo · 2017-12-20T08:34:45Z

data/query.js

@@ -0,0 +1,66 @@
+/**


Minor, it might be located next to other HOCs inside @wordpress/components.

gziolo · 2017-12-20T09:03:59Z

get( props.data, 'editor.post.title', 'no title' )

It would be nice to be able to use it as props.title inside the component. I assumed get is from Lodash, which makes it look like an inlined selector in the current shape. Which leads me to the conclusion that you need to get the same item twice. First in the query resolver and then when applying query HOC.

I know all the benefits of GraphQL, how it can scale well, provides a unified interface and helps to limit the number of requests. I can imagine you even would be able to wrap withApiData with query to unify experience. My biggest concern is that the learning curve might be higher than in case of selectors. I'm especially worried about people coming with jQuery + Backbone background and now suddenly they need to learn ES.Next, React, Redux, GraphQL, etc :)

Do you see benefits of having GraphQL in the core on top of selectors and have the latter exposed for plugin developers? I'm just trying to find a way that satisfies needs of all parties involved here.

youknowriad · 2017-12-20T09:45:18Z

Do you see benefits of having GraphQL in the core on top of selectors and have the latter exposed for plugin developers? I'm just trying to find a way that satisfies needs of all parties involved here.

No, it will be useless in this case. The advantage of GraphQL is being this discoverable and easy to use API. It certains adds learning curve to people creating data (module creators and advanced plugins that want to use the system to expose their own data), but from a consumer's perspective, which I think is the majority of plugin authors and developpers, its API is even simpler to grasp than having to learn and also find every available selector.

atimmer · 2017-12-20T14:43:01Z

data/query.js

+			this.cancelRequest();
+			const query = client.query( { query: this.query, variables: this.variables } );
+			const observer = query.subscribe( ( results ) => {
+				this.setState( results );


In your example in the pull request description, the data is accessed using props.data, but in the higher order component, you only set the state. How does this work?

Yes, actually a GraphQL result is always like this { data, errors } so we'll end up with these two props.

For convenience, we could add an optional callback to "polish" the props like suggested by @aduth. mapResultsToProps

youknowriad · 2018-01-15T09:03:48Z

Closing this as I think we should go with the selectors API

youknowriad added the Framework Issues related to broader framework topics, especially as it relates to javascript label Dec 19, 2017

youknowriad self-assigned this Dec 19, 2017

youknowriad requested review from aduth, gziolo and mcsf December 19, 2017 11:41

youknowriad force-pushed the try/graphql-data-api branch 3 times, most recently from ebea8f3 to 26935d7 Compare December 19, 2017 13:49

Data Module: Expose State using a GraphQL API

60249eb

youknowriad force-pushed the try/graphql-data-api branch from 26935d7 to 60249eb Compare December 19, 2017 13:53

aduth reviewed Dec 19, 2017

View reviewed changes

gziolo reviewed Dec 20, 2017

View reviewed changes

data/query.js

@@ -0,0 +1,66 @@

/**

Copy link
Copy Markdown

Member

gziolo Dec 20, 2017

Choose a reason for hiding this comment

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

Minor, it might be located next to other HOCs inside @wordpress/components.

youknowriad mentioned this pull request Dec 20, 2017

Data Module: Expose state using selectors #4105

Merged

atimmer reviewed Dec 20, 2017

View reviewed changes

atimmer mentioned this pull request Jan 12, 2018

Data Module: Expose state using discoverable selectors #4421

Closed

youknowriad closed this Jan 15, 2018

youknowriad deleted the try/graphql-data-api branch January 15, 2018 09:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data Module: Expose State using a GraphQL API#4083

Data Module: Expose State using a GraphQL API#4083
youknowriad wants to merge 1 commit intomasterfrom
try/graphql-data-api

youknowriad commented Dec 19, 2017 •

edited

Loading

Uh oh!

youknowriad commented Dec 19, 2017

Uh oh!

aduth Dec 19, 2017

Uh oh!

youknowriad Dec 19, 2017 •

edited

Loading

Uh oh!

aduth commented Dec 19, 2017

Uh oh!

youknowriad commented Dec 19, 2017

Uh oh!

gziolo Dec 20, 2017

Uh oh!

gziolo commented Dec 20, 2017

Uh oh!

youknowriad commented Dec 20, 2017

Uh oh!

atimmer Dec 20, 2017 •

edited

Loading

Uh oh!

youknowriad Dec 20, 2017 •

edited

Loading

Uh oh!

youknowriad commented Jan 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

youknowriad commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

youknowriad commented Dec 19, 2017

Uh oh!

aduth Dec 19, 2017

Choose a reason for hiding this comment

Uh oh!

youknowriad Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduth commented Dec 19, 2017

Uh oh!

youknowriad commented Dec 19, 2017

Uh oh!

gziolo Dec 20, 2017

Choose a reason for hiding this comment

Uh oh!

gziolo commented Dec 20, 2017

Uh oh!

youknowriad commented Dec 20, 2017

Uh oh!

atimmer Dec 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad Dec 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad commented Jan 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

youknowriad commented Dec 19, 2017 •

edited

Loading

youknowriad Dec 19, 2017 •

edited

Loading

atimmer Dec 20, 2017 •

edited

Loading

youknowriad Dec 20, 2017 •

edited

Loading