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

Experiment with Relay and GraphQL #4319

Closed
wants to merge 1 commit into from
Closed

Experiment with Relay and GraphQL #4319

wants to merge 1 commit into from

Conversation

gravityrail
Copy link
Contributor

Long story... but I'll try to keep it short.

So, recently I was doing some work on the REST API in order to provide better syncing for Jetpack sites, and to ultimately serve Jetpack site GET requests directly from WPCOM.

As part of this work I ended up refactoring some of the primary objects (Site and Post so far) into what I call the "Site Abstraction Layer", or SAL, which basically provides an object model to retrieve API fields and neatly separate rendering of those fields between WPCOM, Jetpack, and Jetpack-Shadow-Site implementations.

The goal of this was to allow myself to separate out platform-specific hacks and ship less WPCOM code with Jetpack, but along the way I realised that I'd just created an object graph for WordPress.com.

And then I thought "I wonder how easy it would be to port this to a GraphQL API".

As it turns out, reasonably easy.

What is GraphQL?

GraphQL is query language that mirrors React's composition-oriented approach. The idea is that by composing just the data you need in one request rather than many, you can overcome bandwidth and latency limitations of building very rich apps against a REST API, where separate requests must be issued for each object or list, and all fields are returned every time (by default, anyway).

In theory this means less work for the client, AND the network, AND the server. In practice, of course, it's complicated.

The prototype GraphQL server that I implemented uses the graphql-php library.

The server side of this is in patch D1443-code. Apply the patch to your WPCOM sandbox in order to try this out.

What is Relay?

Relay is a React library for binding GraphQL queries and fragments to components. When a component tree under a Relay.RootContainer is rendered, it fetches all the data at once, compiling together the data requirements of the entire component tree.

The benefits of Relay IMO is (1) that it's an officially endorsed data binding solution for React, and (2) that it allows components to be developed in a moderately decoupled manner both from each other and from the server. In addition, by using babel-relay-plugin it validates the syntax of all queries against the server's schema at compile time - extremely handy, both for validating the client AND the server implementation.

The isolation of Relay queries also makes testing easier - you only need to satisfy the declared data dependencies of the component wrapper in order to build a valid test.

How do I run this?

  1. Install patch D1443-code to your sandbox
  2. make run and head to http://calypso.localhost:3000/devdocs/data-binding
  3. If you see errors in the server or browser consoles, it could be that your schema.json is out of date - see below for how to update it.

How do I update the schema.json?

The client needs access to a schema.json model that matches the server. To do that we have to issue a special query to the graphql endpoint and dump the results in a file.

Eventually this could be an automated part of the build process.

  1. Make a POST request to https://public-api.wordpress.com/rest/v1.1/graphql with the following POST fields:
query: schema

(note: this is not valid graphql, it's a hack that I put in for specifically requesting the schema)

  1. Paste the results of the query into server/relay/schema.json

How do I test out graphql queries?

Very similar to the above, POST to https://public-api.wordpress.com/rest/v1.1/graphql with something like this as your query parameter:

query Site (id:88276458) {
  id,
  url
}

How does this work?

By default Relay issues requests to $(host)/graphql. I implemented a custom network layer which instead uses the wpcom object to retrieve responses from public-api.wordpress.com using whatever underlying protocol wpcom is configured to use.

/**
 * Custom network layer that routes graphql queries to the 
 * correct WPCOM REST API endpoint
 */
class WpcomNetworkLayer {
    sendQueries(queryRequests) {
        return Promise.all(queryRequests.map(
            queryRequest => {
                wpcom.req.post( { path: '/graphql' }, { query: queryRequest.getQueryString() }, ( error, response ) => {
                    if ( error ) {
                        queryRequest.reject( error );
                    }

                    queryRequest.resolve( { response: response.data } );
                } );
            }
        ));
    }

    sendMutation(mutationRequest) {
        // TODO
    }

    supports(...options) {
        // TODO
        return true;
    }
}

// configure Relay to use our network layer
Relay.injectNetworkLayer( new WpcomNetworkLayer() );

Everything from there on is standard Relay - implement a container with a root query (typically Relay you'll see this called "viewer". I called it "site").

Right now it's hard-coded to retrieve my own site, goldsounds.com. You can see the hard-coded parameter in the AppRoute instantiation:

new AppRoute( { siteID: 88276458 } )

In the real world we'd probably have the user as the root and then fetch sites etc. under that.

This code also demonstrates basic composition, where we have an App component which displays the site URL, and a nested SitePlan component that renders the plan name.

Unsolved problems

Oh, so many.

  • writes! (aka "mutations") - haven't even started trying to figure out those.
  • refactor the SAL so that it's not oriented around retrieving a single site and individual posts for that site, but rather around retrieving any set of objects for a specified user.
  • add support for all API objects by converting every existing endpoint to use the SAL and then expose those properties in the schema (an undeniably enormous amount of work).
  • how does this work with Redux? do they have overlapping responsibilities?
  • tests
  • more issues than my caffeinated brain can summon right now.

@gravityrail gravityrail self-assigned this Mar 25, 2016
@gravityrail gravityrail added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 25, 2016
@jordwest
Copy link
Contributor

jordwest commented Apr 8, 2016

This is really cool!

how does this work with Redux? do they have overlapping responsibilities?

From the discussions below it seems like they do have overlapping responsibilities; currently Relay is focused on managing local copies of server state (as fetched via GraphQL), while Redux just manages local state and it's up to you to populate the local state with API calls.

@apeatling
Copy link
Member

@gravityrail What's the path forward with this? Right now it's gathering dust so I'm not sure we want to leave this PR indefinitely. @mtias maybe you have some thoughts on this one?

If there is not a clear path for this, we should close it but use it for reference in the future.

@apeatling apeatling added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jun 3, 2016
@gravityrail
Copy link
Contributor Author

@apeatling apologies for the delay in replying. I think the question of whether there's a path forward is something for the whole team.

Broadly, here are my thoughts though:

  • With Jetpack 4.2 and upwards we will be getting a pretty complete set of site data synced to WPCOM, allowing us to ship new APIs (at least, the GET portion) without having to introduce them into Jetpack first.
  • That's ok because it's GET requests which are the slow & gnarly ones. POSTs and DELETEs tend to happen in isolation in response to user input, but GETs absolutely stampede across the connection when you fire up any part of Calypso.
  • We can require specific Jetpack versions in order to use Calypso, and the Jetpack version is synced to WPCOM. In other words, we can "grey out" sites which aren't using Jetpack 4.2 and force them to upgrade (at some point)
  • ... and yes, I think we absolutely should do something like Relay+GraphQL. I believe it's possible to do so incrementally, and the performance benefits are huge. In a year where performance is one of our key goals, being able to load Calypso from a cold start in less than 5 seconds (and/or a few hundred kb) would be a fantastic and timely achievement.

@lancewillett
Copy link
Contributor

Closing as no action in several months. Feel free to re-open or bring it back in the future.

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

Successfully merging this pull request may close these issues.

5 participants