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

fix(gql.tada): Fix recursive optional fragment types causing exponential complexity #319

Merged
merged 2 commits into from
Jun 13, 2024

Conversation

kitten
Copy link
Member

@kitten kitten commented Jun 13, 2024

Summary

Patterns that require a T | {} optionality being merged into selections are causing exponential complexity. This is because all fields in a selection are merged using the obj utility. However, T | {} causes 2^n complexity in evaluating and flattening types in TypeScript.

This is because for each A & (B | {}) merge two variants are created, (A & B) | (A & {}). For each subsequent addition (e.g. A & (B | {}) & (C | {})) the complexity doubles ((A & B) | (A & C) | (A & B & C) | A. This can quickly cause a recursion complexity error in TypeScript once we reach the limit of 16 variants.

This fix addresses this by limiting the flattening of fields to just the fields and not optional sub-selections.

This fix however does have a few problems:

  • We can probably add a shortcut for fragment masks, since those are represented by a single field consistently
  • This can maybe still occur with unions/interfaces that create multiple different variants of types
  • The implementation of this fix makes getPossibleTypeSelectionRec just slightly too hard to follow for my taste
  • The output type shape quality is sometimes now compromised on

As such, post-merge, we're not planning to release this fix as another stable release straightaway and are still looking for improvements.

Set of changes

  • Update tests for new output type shapes
  • Separate field flattening from "non-flatteneable" types

Copy link

changeset-bot bot commented Jun 13, 2024

🦋 Changeset detected

Latest commit: c25848c

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@kitten kitten requested a review from JoviDeCroock June 13, 2024 16:04
@kitten kitten merged commit 49e9b78 into main Jun 13, 2024
2 checks passed
@kitten kitten deleted the fix/recursive-optional-fragments branch June 13, 2024 16:23
@github-actions github-actions bot mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants