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

(nullability) Allow @catch directive to apply all usages of interface field in an operation #47

Open
rohandhruva opened this issue Feb 1, 2024 · 4 comments

Comments

@rohandhruva
Copy link

rohandhruva commented Feb 1, 2024

Can we consider extending the nullability spec to allow applying the @catch to an interface type, and have it apply to all fields in that operation which implement that interface? Maybe using schema extensions if that's a better fit.

Here's an example of what our schema looks like:

interface PageRow {
  entitiesConnection: RowEntitiesConnection
}

interface RowEntitiesConnection {
  edges: RowEntitiesEdge 
}

interface RowEntitiesEdge {
  index: Int
  node: RowEntity # this is also an interface
}

type VideoRow implements PageRow {
  entitiesConnection: VideoRowEntitiesConnection
  edges: VideoRowEntitiesEdge
}

type VideoRowEntitiesEdge implements RowEntitiesEdge {
  index: Int
  node: VideoEntity
}

## Imagine similar types for, say, `GamesRow`, `GamesRowEntitiesEdge` and `GameEntity`.

And then in our operations, we use spreads to

fragment RowData on PageRow {
  ...GenericRow
  ...VideoRow
  ...GameRow
}

fragment GenericRow on PageRow {
  ## fetch `index` via the connection -> edge
}

fragment VideoRow on PageRow {
  ## fetch video entities
}

fragment GameRow on PageRow {
  ## fetch game entities 
}

Currently, if I want to @catch something at an entity level, I would need to end up adding the @catch at every single usage of the PageRow, which can be over 20-25 rows for a given operation.

As a result, I think we could add a @catch for, say, RowEntitiesEdge, and allow it to "apply" to all the concrete field usages, I think it would allow us to use that directive in a more straightforward and clean way.

@martinbonnin
Copy link
Contributor

This should be addressed with #48. To always catch to RESULT on RowEntitiesEdge.node, this should do:

extend interface RowEntitiesEdge @catchField(name: "node")

It'll be available in the 4.0.0-beta.5-SNAPSHOT versions once CI terminates

@rohandhruva
Copy link
Author

@martinbonnin , thank you! I'm getting around to trying this now :)
I had a quick question, does this apply to union types as well?

We tend to use unions as our return types to give us flexibility to change it in the future, which means that we need to use inline fragment spreads even when we know that there's only one possible object type this union could have. This leads to us having to deal with nullable inline fragment spreads.

@martinbonnin
Copy link
Contributor

Hi 👋

Apologies for the late response. @catch only applies to fields and unions have no fields so it wouldn't make so much sense:

union Node = Product | Review

extend union RowEntitiesEdge @catchField(name: /*there is no field to put here*/)

if an object or interface has a node type. You can mark it as @catch

type Query {
  node: Node
}

# This works
extend type Query @catchField(name: "node")

us having to deal with nullable inline fragment spreads.

I think this is typically what you want? Same as UNKNOWN__ for enums, your app need to handle new members added to the union so it's more robust to handle the default case.

@rohandhruva
Copy link
Author

Thank you for explaining, @martinbonnin ! I think that makes sense.

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

2 participants