fix: prevent TS2590 on dense bidirectional relationship schemas#671
fix: prevent TS2590 on dense bidirectional relationship schemas#671Ktsierra wants to merge 1 commit intoaws-amplify:mainfrom
Conversation
…schemas
Replace recursive `Bag[relatedModel]['__meta']['flatModel']` reference
in ResolveRelationship's flat-model branch with a new
`FlattenRelationships` mapped type that inlines related models as plain
objects from their resolved `type`. This breaks the exponential type
expansion cascade that caused TS2590 ("Expression produces a union type
that is too complex to represent") on schemas with dense bidirectional
relationships.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes TypeScript TS2590 "Expression produces a union type that is too complex to represent" errors in schemas with 11+ models and dense bidirectional relationships. The issue was caused by recursive type expansion in the flat model resolution pipeline, where model A's flatModel needed model B's flatModel, which needed model A's flatModel, creating exponential type expansion.
Changes:
- Introduced
FlattenRelationshipstype that replaces recursiveflatModelreferences withtype(containing LazyLoaders) to break the recursion cascade - Updated
ResolveRelationshipto useFlattenRelationshipsinstead of directflatModelreferences in the flat-model branch - Updated p99-complex-relationships benchmark baseline to reflect actual instantiation count after fix (41,538 → 61,404)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/data-schema/src/ClientSchema/utilities/ResolveField.ts | Added FlattenRelationships type and updated ResolveRelationship to use it instead of flatModel, breaking recursive type expansion |
| packages/benches/p99/over-limit/operations/p99-complex-relationships.bench.ts | Updated benchmark baseline to match actual instantiation count after the fix resolves TS2590 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ktsierra
force-pushed
the
fix/resolve-fields-bidirectional-short-circuit
branch
from
367d6f8 to
2c0d557
Compare
|
@Ktsierra are you able to review the failing checks? thanks for the PR! |
|
Hey @pravanosu, I looked into the failing checks and found the root cause. The FlattenRelationships fix that prevents TS2590 works by truncating the recursive type expansion at a certain depth, which means selection set paths beyond that boundary are no longer generated. This is what's causing the tsc-compliance failure (a deeply nested path like articles.articleOriginalWorks.person.name is no longer valid) and the type perf regressions. The core issue here is selection set depth, how many levels deep the type system resolves related models. The current fix hard-caps it, which solves TS2590 but limits expressiveness for schemas with deep relationships. Would you be open to an API addition that lets users configure selectionSetDepth on their schema? Something like a.schema({...}).selectionSetDepth(n), this way users with simpler schemas can keep deeper traversal, while users with dense bidirectional relationships can dial it down to avoid hitting TS2590. I already have a basis for this implementation in my fork, but held off on submitting. It would also be fully backwards-compatible, not setting selectionSetDepth would just use the current default depth the repo already uses, so no existing behavior changes. Let me know your thoughts and I'll get the other checks (changeset + benchmark baselines) sorted once we align on the approach. |
|
Hey @Ktsierra , i'll be able to investigate this week and get back to you shortly! apologies for the delay |
Problem
Schemas with 11+ models and dense bidirectional relationships (e.g. hub models with 9+
belongsTo/hasManypairs) trigger TS2590: "Expression produces a union type that is too complex to represent" when calling model operations like.get(),.list(), or secondary index queries.The root cause is in
ResolveRelationship's flat-model branch (introduced in #647). It referencesBag[relatedModel]['__meta']['flatModel'], which is computed viaFlatResolveFields→ResolveIndividualField→ResolveRelationship, the same pipeline. This creates a recursive cycle where model A'sflatModelneeds model B'sflatModel, which needs model A'sflatModel, causing exponential type expansion across the entire schema graph.PR #647 mitigated this for most schemas by applying
ShortCircuitBiDirectionalRelationship(which dropsbelongsToback-references), but for sufficiently dense schemas the recursive expansion still exceeds TypeScript's complexity limit before the filter is applied.Issue number, if available:
#424
Changes
Added a
FlattenRelationships<Bag, Model, RawFields>mapped type that takes a model's already-resolvedtype(which containsLazyLoaderfunction types for relationships) and replaces each relationship field with the related model'stypeinlined as a plain object. This breaks the recursive cascade because the inlinedtypecontainsLazyLoaders (functions), whichModelPathInnerdoes not recurse into, making the expansion bounded at one level instead of exponential.Replaced
Bag[relatedModel]['__meta']['flatModel']inResolveRelationship's flat-model branch withFlattenRelationships<Bag, Bag[relatedModel]['type'], Bag[relatedModel]['__meta']['rawType']['fields']>.What is preserved:
DependentLazyLoaderOpIsAvailable=> models with.disableOperations()still correctly hide relationship fieldsShortCircuitBiDirectionalRelationship=>belongsToback-references are still dropped from flat modelsFlatResolveFields,FlatClientFields,__meta.flatModel,__meta.rawType)What changes:
Selection set paths resolve relationships through
FlattenRelationships(type)instead offlatModel, giving 2 levels of depth with LazyLoader-terminated fields at level 2 ( which means typed access up to the 3rd parameter, ex :author.book.publisherworks butauthor.book.chapters.wordcountwon't, users will need to stay at chaptersp99-complex-relationshipsbenchmark baseline updated from 41,538 to 61,404 instantiationsCorresponding docs PR, if applicable: N/A
Validation
544/544 unit tests pass (
npx jest --coverageinpackages/data-schema)All p50 benchmarks within 20% threshold (-3% to +1.5%)
All p99 within-limit benchmarks within 20% threshold (-1.4% to +1.4%)
p99 over-limit complex-relationships benchmark baseline updated to match actual count
Verified against a real-world dense schema (11+ models, 50+ bidirectional relationships), TS2590 and TS2589 both resolved, no new type errors introduced
Checklist
Strategy README)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.