search

magento / architecture

A place where Magento architectural discussions happen
275 stars 153 forks source link

Added design document with nullability recommendations for GraphQL schemas #392

Closed DrewML closed 4 years ago

DrewML commented 4 years ago

Rendered Document

Prior discussions about nullability:

DrewML commented 4 years ago

Added a section with some guidance on scalars based on feedback from @paales on community slack

DrewML commented 4 years ago

Please correct indentation in the examples (verify by opening the "rich diff": #392 (files) )

@paliarush pushed a fix for the formatting

jimbo commented 4 years ago

Makes sense to me. I'll concur with the takeaway from @buskamuza here:

field may be nullable if it is not critical for the parent object

It's worth noting that for many list fields, if we were to query for them independently, the resulting list would have to be nullable anyway because of the rule you defined earlier:

Top-level Query fields should always be nullable

For example, if we requested RelatedProducts using a top-level query with productId as an argument, the result would have to be nullable. So we might as well also make such lists nullable when they're included as fields in other queries.

DrewML commented 4 years ago

We've had this open for 2 weeks with several approvals. Going to merge, but if new concerns arise please open an issue or a follow-up PR and we can iterate.