facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.38k stars 1.82k forks source link

Support readInlineData with @relay(plural: true) Fragments #3041

Open derekdowling opened 4 years ago

derekdowling commented 4 years ago

Despite spread the fragment in the parent that is passing the ref to the function, you get a runtime error when attempting to use readInlineData on a Fragment that uses the plural: true directive such as:

function filter(itemsRef) {
  const items = readInlineData(
    graphql`
      fragment ItemList_items on Item @relay(plural: true) {
        name
      }
    `,
     itemsRef
  );

  // process
}

Error:

readInlineData(): Expected fragment `ItemList_items` to be spread in the parent fragment.

I also tried mapping with readInlineData, but that didn't work either:

function filter(itemsRef) {
  const items = itemsRef.map(i => readInlineData(
    graphql`
      fragment ItemList_items on Item @relay(plural: true) {
        name
      }
    `,
     i
  );

  // process
}

Presumably readInlineData is expecting an object that contains a specific fragment ref key rather than an array of fragments.

taion commented 4 years ago

We're hitting this too, but I wonder if the answer here isn't just "don't use @relay(plural)"? The plural annotation seems like it'd mostly just be a signal to the fragment container to expect an array.

But in this case, couldn't you just make the fragment non-plural, then iterate over each instance?

Daniel15 commented 4 years ago

I hit the same issue. It should work if you iterate over the array (as per your second code snippet) if you remove @relay(plural: true) from the fragment.

const items = itemsRef.map(i => readInlineData(
  graphql`
    fragment ItemList_items on Item {
      name
    }
  `,
   i
);

The other way I've worked around this issue is by passing in a connection rather than a raw array, for example:

const items = readInlineData(
  graphql`
    fragment ItemList_items on ItemConnection {
      nodes {
        name
      }
    }
  `,
  itemsRef,
);
// use items.nodes
stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Daniel15 commented 3 years ago

@alunyov Do you know why the bot tagged this with wontfix? I think this should be a "will-fix-one-day" 😛 The internal wishlist task for this (T65272734) is still open too.

Daniel15 commented 3 years ago

^ Oh, okay, so that's how the bot works. 🧐

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

sibelius commented 2 years ago

can you try this using @inline directive?

vknez commented 9 months ago

@sibelius We're hitting the same error message. We do have @inline on the fragment marked as plural.

Daniel15 commented 9 months ago

@vknez As far as I know, @inline on plural fragments is currently unsupported. The workaround is to map over the list, e.g.:

// Doesn't work
const data = readInlineData(
  graphql`
    fragment Foo_bar on YourObject @inline @relay(plural: true) {
      field_name
    }
  `,
  props.data,
);
// Works
// Notice NO `@relay(plural: true)` in the fragment!
const data = props.data.map(item => readInlineData(
  graphql`
    fragment Foo_bar on YourObject @inline {
      field_name
    }
  `,
  item,
));

Passing in the connection object rather than an array of items would work too. I have a code example for that earlier in this issue.

vknez commented 9 months ago

Thanks @Daniel15, we managed to solve that yesterday.

Fun fact: When I saw the photo, I realized you're the same guy who made React.NET, which I used to implement SSR for my ASP.NET React SPA somewhere in 2014 :) Great work!!!