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

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

Open
adamesque opened this issue Mar 13, 2024 · 1 comment
Labels
core Feature requests related to core functionality

Comments

@adamesque
Copy link

adamesque commented Mar 13, 2024

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:

  • During development, someone adds a component with a fragment depending on variable and a new useFragment call, and in local testing data never appears (because they forgot to wire in the variable). Usually generates a dev support request.
  • During development, a field that needs a variable is added to an existing fragment, but the variable is never wired in. But because the test harness calls cache.writeFragment with no variables to bootstrap the test, tests pass and the experience ships broken.

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
Copy link
Member

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!!

@jerelmiller jerelmiller added the core Feature requests related to core functionality label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Feature requests related to core functionality
Projects
None yet
Development

No branches or pull requests

2 participants