gql-dart / gql

Libraries supporting GraphQL in Dart
MIT License
267 stars 121 forks source link

feat(dedupe_link): Add shouldDedupe option #429

Closed nhannah closed 9 months ago

nhannah commented 9 months ago

Dedupe link is an all or nothing solution at the moment, but in use it is likely you would want to dedupe queries, but not mutations, or possibly filter deduplication in some other granular way. This modification allows for devs to consider the incoming request and prevent deduplication if desired.

knaeckeKami commented 9 months ago

Thanks!

Do you think it would make sense to pass in a bool Function shouldConsiderForDeduplication(Request request) instead of a list of operation types?

the user could then pass in (req) => req.operation.getOperationType() == OperationType.query

nhannah commented 9 months ago

Thanks!

Do you think it would make sense to pass in a bool Function shouldConsiderForDeduplication(Request request) instead of a list of operation types?

the user could then pass in (req) => req.operation.getOperationType() == OperationType.query

Always a fan of allowing more freedom, discussed with a colleague and went with shouldPreventDedupe so logic written by a developer can be inclusive of the group they want to prevent deduplication on, i.e. (req) => req.operation.getOperationType() == OperationType.mutation vs (req) => req.operation.getOperationType() != OperationType.mutation

knaeckeKami commented 9 months ago

Thanks! one last small nit:

From Effective Dart:

PREFER the “positive” name for a boolean property or variable Most boolean names have conceptually “positive” and “negative” forms where the former feels like the fundamental concept and the latter is its negation—”open” and “closed”, “enabled” and “disabled”, etc. Often the latter name literally has a prefix that negates the former: “visible” and “in-visible”, “connected” and “dis-connected”, “zero” and “non-zero”. When choosing which of the two cases that true represents—and thus which case the property is named for—prefer the positive or more fundamental one. Boolean members are often nested inside logical expressions, including negation operators. If your property itself reads like a negation, it’s harder for the reader to mentally perform the double negation and understand what the code means.

https://dart.dev/effective-dart/design#prefer-the-positive-name-for-a-boolean-property-or-variable)

shouldPreventDedupe is IMO conceptually slightly more complicated than shouldDedupe.

I would prefer a positively worded field (which then would default to true to dedupe every request, which is the current behaviour)

nhannah commented 9 months ago

Works for me I'll swap it around and update.