Idea: Combining Routes and RootContainer into one root component

[dougli]

I'm curious what you guys think of combining Routes and RootContainer into just one component.

It often is the case that most Relay pages / apps have a top level element that handles the initial entry point of Relay. This top level element is typically always paired with a specific route, and requires a RootContainer or RelayRenderer to work. As @ianstormtaylor has mentioned in #822, the Routes / RelayQueryConfig concept is very lightweight and feels almost like a dangling object that needs to coexist with other things.

I know that containers are meant to be composable anywhere, but given that there often is this top level element in practice, I was wondering if we can get rid of the complexity of these concepts and just have a root component that combines these together. The root component can also take care of handling error states, if they're passed in as props:

var StarWarsPage = React.createClass({
  // Error and fetch states are handled here. Fetched props from Relay passed in as usual.
  propTypes: {
    relayFetchStatus: {
      error: ...,
      retry: ...,
      stale: ...,
      done: ...,
    }
  },

  render: ...,
});

var StarWarsPageContainer = Relay.createContainer(StarWarsPage, {
  fragments: {
    factions: () => Relay.QL`
      fragment on Faction {
        name
      }
    `
  }
});

------

// Combines Routes & RootContainer together
class StarWarsApp extends Relay.RootComponent {
  static routeName = 'StarWarsAppHomeRoute';

  static queries = {
    factions: () => Relay.QL`query { factions }`,
  };

  static container = StarWarsPageContainer;
}

// Variables that the Route needs can be passed in as props to this component, if necessary.
ReactDOM.render(
  <StarWarsApp />,
  document.body
);

This goes a step short of making this enabled for all relay containers (so StarWarsPageContainer is still composable just like any other component), but introduces a distinction that it's possible to compose a top level element into an initial entry point for Relay. It also gets rid of a few concepts that I hope would be a simplification for everyone :).

[wincent]

This idea came up in internal discussion too. CC'ing some of the participants in case they want to respond: @devknoll, @yungsters, @cpojer, @joshduck, @sahrens, @josephsavona, @elynde.

(If nobody chimes in, I'll try and summarize the thread later on.)

[dougli]

Yeah, this bubbled up internally too in P56213804. @josephsavona suggested I toss it here.

[josephsavona]

Thanks for moving the discussion here so the community can participate (this started as an internal discussion). Overall I agree that a higher-level component along the lines of your proposal could be useful in certain situations. This is a great example of something that should be possible (and not too difficult) to build in user-land after we've completed the Relay Core API in #559.

It often is the case that most Relay pages / apps have a top level element that handles the initial entry point of Relay

This is a common pattern (let's call it one-root-per-page), but it isn't the only approach to handling top-level views. Many apps use a router such that there is one fixed RelayRootContainer that is mounted once and whose Component and queryConfig props change depending on the active route. See react-router-relay for examples of this pattern.

I know that containers are meant to be composable anywhere, but given that there often is this top level element in practice, I was wondering if we can get rid of the complexity of these concepts and just have a root component that combines these together.

Here too, some containers that are rendered at the top-level may be reused as sub-containers elsewhere and some may not. Requiring every container to implement handling for loading states and errors would introduce an unnecessary burden on developers - as you point out, there is something special about the root component.

The existing APIs - RelayRootContainer, RelayContainer, and query configs - are designed to be put together in different ways to support both the fixed root with routing, one-root-per-page, server rendering, etc. We've also separated the concerns of dealing with loading data (root containers) from rendering data once it's available (containers).

That said, it's clear that a merged query config + data fetcher + root container style API could be quite useful. At this point most of what's necessary to implement this in user-land is exposed via the RelayEnvironment API (not yet public). I'd encourage you to submit a PR demonstrating the implementation of the RootComponent API, and let us know what (currently internal) methods you'd need on RelayEnvironment to make it feasible. We can use this to help flush out the API in #559.

Note: I suspect you'll end up needing getRelayQueries and RelayFragmentPointer.createForRoot.

[josephsavona]

One additional thought: what if the API for the RootContainer was something like:

const StarWarsApp = createRootContainer(
  // props are the data for each query
  ({error, props}) => {
    if (error) {
      return <Text>{error.message}</Text>;
    }
    return (
      <View>
        {props.viewer.factions.map(faction => <Faction key={faction.id} faction={faction} />)}
      </View>
    );
  },
  {
    initialVariables: {
      count: 10,
    },
    // no nested fragments
    queries: {
      viewer: () => Relay.QL`
        query {
          viewer {
            factions(first: $count) {
              id
              ${Faction.getFragment('faction')}
            }
          }
        }
      `,
   }
  }
);
React.render(
  <StarWarsApp />,
  ...
);

i.e., make this a higher-order component instead of sub-subclassing React.Component.

[joshduck]

Our initial reasoning behind this was that we envisioned a single application component that would conditionally compose sub components. For example an <App /> component may compose <Chrome /> as well as either <ProfilePage />or <InboxPage /> content depending on the current URI.

This isn't incompatible with the initial suggestion. But it does mean that has to be aware of all possible root fields (which is currently the case) and when to fetch those fields and how to populate the properties of the fields.

This becomes difficult when you consider that a root fields might have different meanings depending on the route. For example /me might require the fields user(id: $currentID) { ... } and /profile/foo might require user(id: $paramID) { ... }. Very quickly the <App /> root component accumulates logic that covers every single route that could be loaded.

This could be overcome if you had a root component per route, e.g. <ProfileRoot /> and and each root composed <Chrome />. But then you get into weirdness if you want to persist parts of the chrome, because React reconciliation won't try to preserve the underlying component.

[joshduck]

@mroch has a lot of context on this, too.

[josephsavona]

I'm marking this as "help wanted" - the general direction is promising, but this seems like a prime candidate for something that should be achievable given #559. The best step is probably for someone to try implementing it, figure out what public API is missing to support this and then build that API as part of #559.

[dougli]

I can take a stab at it when I have time, after I come back from vacation :).

On 22 Mar 2016, at 05:17, Joseph Savona notifications@github.com wrote:

I'm marking this as "help wanted" - the general direction is promising, but this seems like a prime candidate for something that should be achievable given #559. The best step is probably for someone to try implementing it, figure out what public API is missing to support this and then build that API as part of #559.

―
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

[steveluscher]

Because of the progress that's been made developing Relay.Renderer and the soon-to-be-public Relay.QueryConfig, let's close this out. Thanks for the discussion everyone! It helped move us in the right direction.