apollographql / apollo-feature-requests

🧑‍🚀 Apollo Client Feature Requests | (no 🐛 please).
Other
130 stars 7 forks source link

Warn/error when fragments that depend on operation variables are used without them #430

Open adamesque opened 8 months ago

adamesque commented 8 months ago

We're building more and more with useFragment and for the most part are pretty happy with it. Something that comes up frequently as a footgun, though, is folks forgetting to add variables to their useFragment calls. This can disrupt the development cycle in multiple ways:

Ideally, we could catch many of these cases with a document-aware lint rule, but some cases are trickier to lint (like when test fragments are defined in a test file but passed to a utility bootstrapping the test that programmatically calls cache.writeFragment).

I haven't worked with TypedDocumentNode enough to know if this is something TypeScript could help guard against either, but that would be even better than a lint rule.

As a fallback, would it be prohibitively expensive to scan fragment ASTs at runtime to warn or error when a var appears in a fragment but isn't passed to any of the APIs that accept fragments as arguments? As @jerelmiller mentioned in a side discussion, it's possible doing this in a dev-only check might be sufficient to ensure that tests fail.

jerelmiller commented 8 months ago

This is an excellent idea and I see no reason why we shouldn't add a new dev-only warning. Thanks so much for the suggestion!!