React-apollo: Nested components with data masking

[sedubois]

Hi and apologies if this is redundant! But I didn't find documentation about nested components using fragments and preserving data isolation?

Similar to Relay, how can Apollo GraphQL containers be nested within each other, with the child container passing its fragment to the parent which then assembles the full query, while at the same time ensuring that the parent is not exposed to the data that should only be consumed in the child?

The first part (assembling fragments) is visible in the GitHunt example, but I didn't find about the second part (avoiding implicit dependencies). E.g VoteButton exposes a fragment describing its data requirements (which is good), but this data is manually queried then injected from the parent FeedEntry.

As Apollo is documented as being fully featured, I suppose this should be possible. In my view, data isolation is one of the fundamental features made possible in GraphQL clients, an improvement on the concept of component isolation already core to React.

Thanks and keep up the good work 😊

[helfer]

@sedubois you're right, it's not documented because we currently don't do anything to pass the data only to the component which defined the fragment. If you have some ideas for how to add it without tying Apollo Client to a specific router or force a specific setup, I'd love to hear your ideas.

[stubailo]

In my view, data isolation is one of the fundamental features made possible in GraphQL clients, an improvement on the concept of component isolation already core to React.

I don't think it's a foregone conclusion that data isolation specifically through fragments is a core benefit of GraphQL, but it's certainly a feature we should consider including a simple implementation of.

[stubailo]

Do you have a specific example of the kind of developer experience you would be looking for? Is it something like having a property on the result with the fragment name as the key, which could be passed down into the component?

[sedubois]

Do you have a specific example of the kind of developer experience you would be looking for?

I'd be looking for an experience like Relay: being able to pass objects to nested components as props just like now, but if looking at the object's contents from within the parent, the nested data shouldn't be visible.

I don't think it's a foregone conclusion that data isolation specifically through fragments is a core benefit of GraphQL

In my view A core benefit of GraphQL is presenting data exactly where it is consumed, so IMHO _data masking_ is essential, whether it be through fragments or otherwise. Right now in Apollo I just don't know any way to ensure that. I don't know the Apollo codebase, so however can't give advice 😉

More reading on data masking in the Relay doc.

[seeden]

Hi guys I am using fragments too
Here is my example with "multiple fragments" and my current logic maybe it will help you. Anyway let me know what do you think. Maybe it is not ideal solution.

UserThumb.jsx - has own fragments

import style from './UserThumb.css';

export default function UserThumb({ user}) {
  return (
    <div className={style.root}>
       <img src={user.pricture} />
    </div>
  );
}

UserThumb.fragments = createFragment(gql`
  fragment userPhotoThumb on User {
    picture,
  }
`);

Profile.jsx - Main container (this container is using UserThumb in render function)

import UserThumb from '../user/UserThumb';

/* I know that I am using this component in render and 
*  therefore I am trying to use fragments from these component
*/
const fragments = combineFragments(UserThumb);

@graphql(gql`
  query profile($slug: String!) {
    user(slug: $slug) {
      firstName,
      ${fragments.getQuery('User')}, // => ...userPhotoThumb,
    }
  }
`, {
  options: ({ params: { userSlag } }) => ({
    fragments,
    variables: {
      slug: userSlag,
    },
  }),
  props: ({ data: { loading, user } }) => ({ loading, user }),
})
export default class Profile extends Component {
  render() {
    const { loading, user } = this.props;
    if (loading) {
      return <Loading />;
    }

    return (
      <UserThumb user={user} />
    );
  }
}

Here is my combineFragments function.

export default function combineFragments(...args) {
  const types = {};
  const registered = {};
  const allFragments = [];

  allFragments.getQuery = (type) => {
    if (!types[type]) {
      return '';
    }

    return types[type]
      .map((fragmentName) => `...${fragmentName}`)
      .join(', ');
  };

  args.forEach((component) => {
    let { fragments } = component;
    if (!fragments && Array.isArray(component)) {
      fragments = component;
    }

    if (!fragments || !Array.isArray(fragments)) {
      throw new Error('Component has no fragments');
    }

    fragments.forEach((fragment) => {
      const {
        name: { value: fragmentName },
        typeCondition: { name: { value: type } },
      } = fragment;

      if (!fragmentName || !type) {
        throw new Error('Fragment is not compatible with combineFragments');
      }

      if (registered[fragmentName]) {
        return;
      }

      allFragments.push(fragment);
      registered[fragmentName] = true;

      if (!types[type]) {
        types[type] = [];
      }

      types[type].push(fragmentName);
    });
  });

  return allFragments;
}

This function will return array of all fragments from all components. And you can use "getQuery" function if you want to get list of fragments with specific type for example User. Duplicate fragments will be automatically removed.

[seeden]

Maybe I can create a high order component which will get all fragments from all children during the render.

[seeden]

I found http://relax.github.io/relate/fragments.html They are using mergeFragments. It is almost same functionality.

[stubailo]

Thanks for the example! I think it should be pretty simple to make this work.

Why do we need mergeFragments by the way? Is it not good enough to just say ...userPhotoThumb?

[tmeasday]

@seeden - that looks like an interesting approach, I would consider making a "fragment manager" package or something if you think it's useful to people.

@sedubois et al, if you have not seen http://dev.apollodata.com/react/fragments.html I think we have a reasonable solution to this now, which is conceptually pretty close to what Relay gives you.

I'll close this issue but if there are details about it you'd like to discuss, perhaps graphql-anywhere or the docs could be a good place to do so? Or we can reopen this if you think it doesn't fundamentally address the problem.

[sedubois]

Thanks @tmeasday but from those docs, I don't see where data masking occurs? It looks like parent components can still access child props. So it looks like this got closed as WONTFIX.

[tmeasday]

Hi @sedubois,

I'm not quite sure what you mean by data masking, but is it what the filter function does (limit the passed in object to the fields specified in the fragment?)

I think if you wanted something more like Relay (where it happens automatically) it might make sense to build a "fragment container" higher-order-component. I think this could happen as an addon package and doesn't need to live in React Apollo however.

[denvned]

I'm not quite sure what you mean by data masking, but is it what the filter function does (limit the passed in object to the fields specified in the fragment?)

Looks like the filter function can only help to prevent a child from seeing the parent's and the siblings' data, but it does not prevent a parent from seeing children's data.

Relay solves it by using opaque fragment pointers. I.e. the parent component has access only to opaque fragment pointers for the children, not to the children's data, and it passes children these fragment pointers instead of the data; the child's Relay HOC resolves the passed fragment pointers to the actual data.

[sedubois]

Yep, what @denvned said.

[stubailo]

I agree - the first description on this issue talked about parents not seeing data that was passed to their child, so this issue is not resolved even though there is a way to prevent children from seeing extra data.

I can see how this is advantageous from the perspective of avoiding some errors where the parent uses fields it didn't ask for. On the other hand it breaks the nice property that Apollo's job is just to return the same plain JSON object the server would return - it means that instead of passing around "normal" GraphQL results we would need to have some sort of special data structure instead.

This sounds like a problem that could easily be avoided with static typing using Flow or TypeScript, are you using those in your app?

[sedubois]

@stubailo in the OP I did state "while at the same time ensuring that the parent is not exposed to the data that should only be consumed in the child". But indeed, I could have maybe formulated things more clearly.

I never worked with Typescript and Flow and was kind of hoping it would be less necessary once I started to use Apollo/GraphQL, as it already has a strong type system... Maybe TS/Flow could be short term solutions (although need to check how usable it would be, i.e if it doesn't require too much boilerplate) but longer term it would be great to have Apollo itself ensure the type safety.

[stubailo]

I don't have a strong position here, but my claim would be that leaving type validation to a runtime library will inherently make it less efficient. IMO that's the optimal thing to handle via static analysis tools so that you don't have to load all of that code into your actual app.

[stubailo]

Right now you have two options:

1. No static analysis, use Apollo out of the box - it loads the data just fine, but you don't get that much checking of stuff.
2. You use Flow/TS - you get all of the checking of everything.

I suppose runtime checking would be something like 1.5, and maybe it could be optional like React propTypes?

[tmeasday]

Thanks for explaining it to me guys, I'm up to speed now ;)

I'm inclined to agree with @stubailo here: "it breaks the nice property that Apollo's job is just to return the same plain JSON object the server would return" --- I think a goal of vanilla Apollo is to be simple to understand, and magic data masking is probably a bridge too far in this respect.

However, it seems eminently possible for this to happen in a "smart" version of react-apollo; i.e. another package. I hope we've managed to refactor out the majority of RA back into AC so most of the work in writing a different React integration would be literally describing the masking behaviour desired here.

I'm not sure what's best to do with this issue, if we agree that this functionality doesn't make sense for RA.

[sedubois]

If you don't want to implement this, I guess you could close it back as WONTFIX as it was before 😄

[tmeasday]

Well I'm not the final arbiter on this; thoughts @jbaxleyiii?

[abhiaiyer91]

@sedubois why not make this a community module!?

[sedubois]

@abhiaiyer91 that might work, although its use will be more limited if it's opt-in. But anyway you can give a try 🙂

[jbaxleyiii]

@sedubois I'm still on the fence about this. I tried a few times to make fragments feel natural without a build step and never found something I like. I'm going to close this as WONTFIX again, but if the interest is still there / we can come up with something that feels natural I'm game to keep discussing