[Bug] - Relay fragment variables

mario-jerkovic commented 8 years ago

We have encountered an strange behaviour of Relay. I will try to explain the best I can.

So we have an "main" relay container that fetches the data for corresponding store, and also includes and fragment from Ticket container. Ticket container render out custom table that has filter and sorting. So you can see that in StoreFrom component StoreTicketList container is import all required props are passed like Store fragment.

The problem occurs when you try to filter StoreList Ticket, I mean set filter or sort relay variables. You will get this error:

Warning: RelayContainer: component TicketList was rendered with variables that differ from the variables used to fetch fragment Store. The fragment was fetched with variables {"first":5,"after":null,"last":null,"before":null,"sort":null,"filter":null}, but rendered with variables {"first":5,"after":null,"last":null,"before":null,"sort":null,"filter":{"authorAccount":{"email":{"__e":"wrongEmail@email.com"}}}}. This can indicate one of two possibilities:

The parent set the correct variables in the query - TicketList.getFragment('Store', {...}) - but did not pass the same variables when rendering the component. Be sure to tell the component what variables to use by passing them as props: <TicketList ... first={...} after={...} last={...} before={...} sort={...} filter={...} />.
You are intentionally passing fake data to this component, in which case ignore this warning.

But those filter/sort variables are on StoreTicketList and they arent passed dow from parent to child container like in this case Store container to StoreListTicket container.

export class StoreForm extends React.Component {

  constructor(props) {
    super(props);

    const { Viewer: { store } } = props;

    this.state = {
      number: store && store.number !== null ? store.number : '',
    };
  }

  handleInsert = (model) => {
    console.log('Form mutation model : ', model);
  };

  render() {
    const { Viewer, relay: { variables: { update } } } = this.props;
    return (
      <div>
        <Form>
          <FormTitle title='Store Info' />
          <FormBody>
            <TextField
              required
              fullWidth
              name='number'
              value={this.state.number}
              floatingLabelText='Number'
            />
            <StoreTicketList Store={this.props.Viewer.store} />
          </FormBody>
        </Form>
      </div>
    );
  }
}

export default Relay.createContainer(StoreForm, {
  initialVariables: {
    id: null,
    update: false
  },
  prepareVariables({ id = null }) {
    return { id, update: (id !== null) };
  },
    fragments: {
        Viewer: (variables) => Relay.QL`
      fragment on Viewer {
        store(id: $id) @include(if: $update) {
          id,
          number
          ${StoreTIcketList.getFragment('Store')}
        }
      }
    `
    }
});

Ticket Container:

export const StoreTicketList = Relay.createContainer(TicketList, {
  initialVariables: {
    first: 5,
    after: null,
    last: null,
    before: null,
    sort: null,
    filter: null
  },
    fragments: {
        Store: () => Relay.QL`
      fragment on Store {
        ticketConnection(first: $first, after: $after, last: $last, before: $before, sort: $sort, filter: $filter) {
          count,
          pageInfo {
            hasNextPage,
            hasPreviousPage,
            startCursor,
            endCursor
          },
          edges{
            node{
              created,
              subject
            }
          }
        }
      }
    `
    }
});

josephsavona commented 8 years ago

Thanks for your question! I have an answer for you, but we'd prefer to answer this in a forum that is easily searchable by other members of the community who have a similar question. Would you like to post this question to Stack Overflow with the tag #relayjs? We'll be happy to answer there. Please post a link to your Stack Overflow question here, to so that we don't lose track of it. Thanks!

https://stackoverflow.com/questions/ask?tags=relayjs

mario-jerkovic commented 8 years ago

No problem, here is the Stack Overflow question.

http://stackoverflow.com/questions/37866342/relay-fragment-variables

mario-jerkovic commented 8 years ago

I have tried to isolate the problem, and have created small example of the models on GraphQL side and Containers/Components on the frontend side.

Here is the repo link: https://github.com/mario-jerkovic/Relay-FragmentVariables

If you uncomment the code inside/js/routes/AppHomeRoute.js and/js/app.js, relay wont complaint about different variables, the only difference between working and not working code the the react-router-relay.

skosch commented 8 years ago

Ha, I just ran into the same problem. The error message could be more helpful. It turns out you need to do two things:

Pass the props into the component. I'm using react-relay-router, so for me that was a matter of adding this line

<Route path="interviews">
  <IndexRoute component={InterviewsList} queries={ViewerQuery} />
  <Route path=":id" component={InterviewSession} queries={NodeViewerQuery}
              render={({ props }) => props ? <InterviewSession {...props} /> : <Loading />}/> // <--- this line
</Route>

Also inject the variable's values into the getFragment function call, like so:

    fragments: {
        Viewer: (variables) => Relay.QL`
      fragment on Viewer {
        store(id: $id) @include(if: $update) {
          id,
          number
          ${StoreTIcketList.getFragment('Store', {... variables})} // <---- this thing!
        }
      }
    `
    }

For me the problem was in the root query, where it looks instead like:

const NodeViewerQuery = {
  node: (component, variables) => Relay.QL`query { // <---- extra "component" argument
    node(id: $id) {
      ${component.getFragment('node', {...variables})}
    }
  }`,

I haven't looked at your repo, but maybe this helps you out. To the Relay folks, it'd be nice if this error message pointed to a dedicated example page, or at least explicitly mentioned all the places variables need to be passed in.

josephsavona commented 8 years ago

@skosch Thanks for answering. Would you mind including your answer at Stack Overflow as well to help the community find this answer?

We're definitely aware that this is tricky, and we're looking at ways to simplify passing variables. We'll see how this goes and consider documenting the current system more in the meantime.

skosch commented 8 years ago

Done, thanks :)

mario-jerkovic commented 8 years ago

@josephsavona I have added those changes @skosch proposed and i have the same thing/problem ... You can take a look at my repo from last post... I dont see why passing variables from parent to child container should work, beacuse they are using different set of initial variables to render the container.. Store container has only id but ticket container has filter, first, last, after and before. so those are tied to ticket container.

initially ticket container is renderd with filter: null the you update it with:

relay.setVariables({
      filter: {
        subject: !relay.variables.filter ? { __e: 'something' } : { __ne: 'else' }
      }
    })

on second render when filter is this.props.relay.variables.filter = { subject: { __e: 'something' } relay dosen't complain but when you update the filter variable second time it is.

skosch commented 8 years ago

I suspect (being on mobile and unable to test it) that the problem is <StoreListTickets store={this.props.viewer.store} /> -- try to pass those variables as props to that component?

josephsavona commented 8 years ago

The problem appears to be in StoreForm - StoreForm is not passing variables to StoreTicketList as props. You should be able to fix this by either:

Not passing the second {...variables} argument in https://github.com/mario-jerkovic/Relay-FragmentVariables/blob/master/js/components/App.js#L31
Or by leaving that and also passing all of the variables on line 15 as props to the inner component.

I would also highly recommend trying this on Relay master - we added some refinements to this warning and I'm not sure if they made the cut for the last release. I have a feeling that doing option 1 above plus upgrading to master will fix this warning.

mario-jerkovic commented 8 years ago

Sorry for the late response.

I have tried both solutions, and also updated relay to master branch, and sadly none of them worked, getting the same error message...I mean if that is an issue with relay then ok, probably it's solvable, it's just annoying to see that error message

jardakotesovec commented 8 years ago

Also getting this warning in scenario where I use only initialVariables and setVariables within one component to fetch data on demand and I don't pass any variables via fragments.

Is @mario-jerkovic repro case enough to identify the issue or should I provide more details?

skosch commented 8 years ago

I did get a chance to look at @mario-jerkovic's repo and while I didn't have a chance to take a closer look at it (let alone find a solution) I did confirm that my original suggestion didn't help, because the issue is caused by setVariables (unlike mine, which wasn't). Sorry!

@jardakotesovec sounds like you've come exactly to the right place – hopefully someone can help you guys out!

josephsavona commented 8 years ago

@jardakotesovec Do you have an isolated repro that would help us debug?

jardakotesovec commented 8 years ago

@josephsavona Nope.. but I can attempt to create repro in relay playground tomorrow if that would help.

josephsavona commented 8 years ago

@jardakotesovec That would be super helpful in diagnosing this.

mario-jerkovic commented 8 years ago

@josephsavona I will recreate my repo in playground if the repo by itself wasent helpful

josephsavona commented 8 years ago

@mario-jerkovic I looked at the repo to try diagnose, but it would definitely be much easier for us to run and debug if you can repro in the Relay playground.

mario-jerkovic commented 8 years ago

@josephsavona Here is the Relay Playground link (sorry for just copy and pasting, I have tried to use many link shortners but it always says that it's invalid :smile: )

josephsavona commented 8 years ago

@mario-jerkovic This is super helpful, thank you for taking the time to create this. Having the playground instance makes it pretty clear what's happening - the setState from the parent causes RelayContainer to recheck its props and it ends up comparing the variables in those props (empty, nothing is being passed) with what it fetched (the local setVariables changes) and reporting the discrepancy. This is just a warning so feel free to ignore for now, I'll work on a fix (I'd normally suggest creating a PR but this is a notoriously tricky area in Relay).

mario-jerkovic commented 8 years ago

No problem, I am glad that I could help 😃

philiptzou commented 8 years ago

Thank goodness I found this page. I have the same scenario as @jardakotesovec and get the same warnings. It took me days to suspect that I used Relay wrongly.

mjurincic commented 7 years ago

Any news on this, in our relay app we have the global filter that starts with initialVariables but can be modified later by users interaction with users setVariables. There is no parent to pass this props down and we are receiving this warning.

josephsavona commented 7 years ago

@mjurincic We're working on a new variation of RelayContainer that avoids issues such as these, and that will hopefully be available within the next release or two.

For now, this is just a warning.

wincent commented 7 years ago

Closing this as there's nothing actionable here. New Relay APIs and core are already rolling out to master, and will continue to do so over the coming weeks/months. Thanks for all the input on this issue, folks!

LyndaOuni commented 6 years ago

hello, i have the same problem with react relay, i need help please, class ProductRoute extends Relay.Route { static routeName = 'BasketProductsRoute'; static queries = { product: queriesComponent => Relay.QL query { node(id: $id) { ${queriesComponent.getFragment('product')} } }, }; } const AddToBasketWarrantyContainer = Relay.createContainer(AddToBasketWarrantyPage, { fragments: { product: () => Relay.QLfragment on Product { id uuid logo ref ecoPart category { id } images { uris(size: [MEDIUM]) } brand ref name { text } discount ean promotionsList{ idPrmotion, seuilMinumumDeclenchement, numeroPromotion, typeSeuilDeclenchement, finOffre, valeurDeclencheur, cibleClient, cumulable, uniteRemise, remiseEnseigne, typeRemise, montantMaximumRemise, statut, financement, remiseFournisseur, debutOffre, typeDeclencheur, priceProd, priceProdDiscounted, discount, valuePromo } tvaList { SWListProd { idTva DUREEANNEE isSelected DUREE PVTTC TAUX DESIGNAT PRIXMAX SERVICETYPE GENCOD }, SPCWListProd { idTva, DESIGNAT, SERVICETYPE, COMMPREST, LBPREST, CDPREST, ADDITIONALINFO, isSelected }, SRTWListProd{ idTva, DESIGNAT, SERVICETYPE, isSelected, value } }, price { integralPart fractionalPart valueInCents } categoriesOfProduct { catLevel1, catLevel2, catLevel3 } basePrice { integralPart fractionalPart } codeRemu codeGLN categoriesOfProduct { catLevel1 catLevel2 catLevel3 } creditWithWarranty }, }, });

warning : relay Container: component `Connect(Service Warrantywas rendered with variables that differ from the variables used to fetch fragmentproduct. the fragment was fetched with varibales (not fetched), but rendered with variables {} .

facebook / relay

[Bug] - Relay fragment variables #1218