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

Relay 10: difficulty with optimistic response and @raw_response_type #3129

Open
mrtnzlml opened this issue Jul 15, 2020 · 11 comments
Open

Relay 10: difficulty with optimistic response and @raw_response_type #3129

mrtnzlml opened this issue Jul 15, 2020 · 11 comments

Comments

@mrtnzlml
Copy link
Contributor

mrtnzlml commented Jul 15, 2020

Hi, we have an issue with an optimistic response when using the new Relay 10. This is our mutation:

mutation SidebarLayoutMarkAsSeenMutation($leadID: ID!) @raw_response_type {
	updateLead(input: { id: $leadID, wasSeen: true }) {
		... on Lead {
			wasSeen
		}
	}
}

And here is how we do the optimistic response:

markAsSeen({
	variables: { leadID },
	optimisticResponse: {
		updateLead: {
			__typename: 'Lead',
			// __isNode: 'Lead', <<< should this be here or not?
			id: leadID,
			wasSeen: true,
		},
	},
});

This works fine with Relay 9, however, Relay 10 generates different Flow (or in our case TS) types which forces us to write also __isNode into the optimistic response. That's however also not okay because Relay complains with the following error:

Warning: Expected `optimisticResponse` to match structure of server response for mutation `SidebarLayoutMarkAsSeenMutation`, please remove all fields of
{
  "updateLead": {
    "__isNode": {}
  }
}

Here are the generated types for Flow:

export type SidebarLayoutMarkAsSeenMutationRawResponse = {|
  +updateLead: ?({|
    +__typename: "Lead",
    +__isNode: "Lead",
    +id: string,
    +wasSeen: ?boolean,
  |} | {|
    +__typename: string,
    +__isNode: string,
    +id: string,
  |})
|};

Or for TypeScript which looks fairly similar so I don't think that's the problem:

export type SidebarLayoutMarkAsSeenMutationRawResponse = {
    readonly updateLead: ({
        readonly __typename: "Lead";
        readonly __isNode: "Lead";
        readonly id: string;
        readonly wasSeen: boolean | null;
    } | {
        readonly __typename: string;
        readonly __isNode: string;
        readonly id: string;
    }) | null;
};

My question is: how should we handle this situation? I am not sure what is the right strategy here and what does __isNode exactly mean in this case. 🤔

Thank you very much for having a look!

@josephsavona
Copy link
Contributor

The __isNode field here is fetched so that Relay can determine whether the object that is returned (whose type we don't know statically) implements the Node interface or not. The mutation will be transformed as follows:

mutation SidebarLayoutMarkAsSeenMutation($leadID: ID!) @raw_response_type {
	updateLead(input: { id: $leadID, wasSeen: true }) {
		wasSeen # note the "... on Lead { }" is redundant since the field returns a Lead object (otherwise Relay would fetch "__isLead: __typename" too)
                 ... on Node {
                     __isNode: __typename # if this field is present, we know the return type implements `Node`
                     id
                }
	}
}

So your optimistic response should include __isNode: 'Lead' if the type implements Node, or exclude that key if the type does not implement Node.

@josephsavona
Copy link
Contributor

That's however also not okay because Relay complains with the following error:

This warning seems like a false positive, the optimistic response you provided looks correct.

@mrtnzlml
Copy link
Contributor Author

@josephsavona I see, thank you very much for the explanation. The Lead type indeed implements Node interface, so the optimistic response should be like this, right?

optimisticResponse: {
	updateLead: {
		__typename: 'Lead',
		__isNode: 'Lead',
		id: leadID,
		wasSeen: true,
	},
},

In this case, the generated types are OK. But I am getting the following false positive (?) warning:

Screenshot 2020-07-16 at 16 40 39

BTW the mutation actually returns a union so the transformed query looks like this (prolly not important in this case):

mutation SidebarLayoutMarkAsSeenMutation(
  $leadID: ID!
) {
  updateLead(input: {id: $leadID, wasSeen: true}) {
    __typename
    ... on Lead {
      wasSeen
    }
    ... on Node {
      __isNode: __typename
      id
    }
  }
}

@josephsavona
Copy link
Contributor

Sounds like a bug in the optimistic response validation code, the warning looks incorrect in your case

@steobrien
Copy link

@josephsavona hitting something similar. Is there a way to suppress these warnings, to mitigate?

@stale
Copy link

stale bot commented Dec 25, 2020

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.

@stale stale bot added wontfix and removed wontfix labels Dec 25, 2020
@mrtnzlml
Copy link
Contributor Author

I am afraid this is still valid and should not be closed.

@huv1k
Copy link

huv1k commented Nov 8, 2021

So we are facing a similar problem with using unions and extending the error interface.

mutation useCreateNoteMutation($input: CreateNoteInput!, $connections: [ID!]!) @raw_response_type {
  createNote(input: $input) {
    ... on CreateNoteSuccess {
      note @appendNode(connections: $connections, edgeTypeName: "NoteEdge") {
        id
        text
      }
    }
    ... on ForbiddenTermError {
      blockedTerms
      code
      message
    }
    ... on Error {
      code
      message
    }
  }
}

and with generated typescript code

export type useCreateNoteMutationRawResponse = {
    readonly createNote: {
        readonly __typename: "CreateNoteSuccess";
        readonly __isError: "CreateNoteSuccess";
        readonly code: unknown;
        readonly message: string;
        readonly note: {
            readonly id: string;
            readonly text: string;
        };
    } | {
        readonly __typename: "ForbiddenTermError";
        readonly __isError: "ForbiddenTermError";
        readonly code: unknown;
        readonly message: string;
        readonly blockedTerms: ReadonlyArray<string | null> | null;
    } | {
        readonly __typename: string;
        readonly __isError: string;
        readonly code: unknown;
        readonly message: string;
    };
};

Where we can see that createNote has code and message. that is not part of this type and it's coming from Error interface. At least from my understanding, this shouldn't be part of the optimistic response, because we can't get it from this normal response anyways. Or I am understanding it wrongly? I would say that generated types should look like this:

export type useCreateNoteMutationRawResponse = {
    readonly createNote: {
        readonly __typename: "CreateNoteSuccess";
        readonly note: {
            readonly id: string;
            readonly text: string;
        };
    } | {
        readonly __typename: "ForbiddenTermError";
        readonly code: unknown;
        readonly message: string;
        readonly blockedTerms: ReadonlyArray<string | null> | null;
    } | {
        readonly __typename: 'Error';
        readonly code: unknown;
        readonly message: string;
    };
};

@stale
Copy link

stale bot commented Jan 9, 2022

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.

@stale stale bot added the wontfix label Jan 9, 2022
@artola
Copy link
Contributor

artola commented Jul 16, 2022

@huv1k Do you have some workaround?

@stale stale bot removed the wontfix label Jul 16, 2022
@holmesal
Copy link

holmesal commented Nov 8, 2022

We're seeing something similar to @hub1k's comment with fields from one member of a union being added to other members that do not provide those fields.

In our case, our union type contains both Nodes, and non-Nodes, but the fields __isNode and id are being added to both types in the generated Typescript type for the mutation fragment, causing the optimistic response validation to fail.

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

No branches or pull requests

6 participants