implement NormalizedSelection::has_defer()

apollographql / federation-next

Moved to the Router repository

https://github.com/apollographql/router/tree/dev/apollo-federation

Other

19 stars 1 forks source link

implement NormalizedSelection::has_defer() #262

Closed lrlna closed 4 months ago

lrlna commented 4 months ago

Along with NormalizedSelection::has_defer(), additionally implements:

NormalizedFieldSelection::has_defer()
NormalizedFragment::has_defer() (for NormalizedOperation::without_defer() implementation)
NormalizedFragmentSpreadSelection::has_defer()
NormalizedInlineFragmentSelection::has_defer()
NormalizedSelectionSet::has_defer (for NormalizedOperation::without_defer() implementation)

lrlna commented 4 months ago

can we move all those selections has_defers to top level impls? per previous review comments the inner mods created to keep the key/value in sync but all the other stuff should be moved outside

@dariuszkuc I don't follow. They are at as top level as they can be? NormalizedSelection needs a has_defer implementation in QueryPlanningTraversal::handle_open_branches(). We can remove the child impls from NormalizedFragmentSpread etc, but it's what the JS code has as well, and it's a lot more ergonomic to keep them for future use cases.

goto-bus-stop commented 4 months ago

This PR adds the has_defer() impl blocks inside the nested modules inside operation.rs, which are meant to only contain the functions that need to maintain type invariants (like selection keys and selections staying in sync). has_defer() doesn't mutate and doesn't require keeping things in sync so it can be at the "root" of operation.rs.

Currently the module separation doesn't actually do anything as the fields are pub(crate) anyways. if we want to enforce invariants the fields should be private

lrlna commented 4 months ago

@goto-bus-stop ah, I see, thanks for clarifying. Removed them from the internal mod then.