Skip to content

Add regression test for RelayResponseNormalizer nested-list-with-abstract-leaf (#5056)#5268

Open
markselby9 wants to merge 2 commits into
facebook:mainfrom
markselby9:regression-test-5056
Open

Add regression test for RelayResponseNormalizer nested-list-with-abstract-leaf (#5056)#5268
markselby9 wants to merge 2 commits into
facebook:mainfrom
markselby9:regression-test-5056

Conversation

@markselby9
Copy link
Copy Markdown
Contributor

@markselby9 markselby9 commented May 1, 2026

Summary

Adds a regression test in RelayResponseNormalizer-test.js that demonstrates the bug @captbaritone identified in #5056: when a plural field contains items that are themselves arrays of abstract-typed records, _normalizePluralLink_getRecordType receives an array (not a record) and throws Expected a typename for record [...].

This is test-only per @captbaritone's request in #5056 (comment) — no production code changes. The test asserts current (buggy) behavior; whoever fixes the underlying bug should flip the assertion.

The reported schema shape [[MyType!]!]! isn't modeled by the OSS test schema, so the test feeds a doubly-nested payload to a regular plural-of-abstract field (User.actors: [Actor]). The runtime code path exercised is the same one described in the issue. The regex assertion is anchored on the JSON-stringified open bracket so the test fails loudly if a different _getRecordType call site fires first.

Refs #5056.

Commits

  • Commit 1: original test, placed inside describe('User-defined getDataID') > describe('plural fields') reusing closure-scoped BarQuery.
  • Commit 2 (review feedback): hoisted the test to top-level under describe('RelayResponseNormalizer', …) since it doesn't exercise getDataID behavior. Tradeoff: lost BarQuery closure reuse, so the PR now ships one new __generated__/RelayResponseNormalizerTest5056Query.graphql.js artifact (137 lines, standard codegen output via relay-compiler@0.0.0-main-38ad9e13).

Test plan

  • yarn jest packages/relay-runtime/store/__tests__/RelayResponseNormalizer-test.js -t '5056' — 1 passed
  • full RelayResponseNormalizer-test.js suite still passes (61 total, was 60)
  • lint clean

@meta-cla meta-cla Bot added the CLA Signed label May 1, 2026
markselby9 added 2 commits May 2, 2026 11:03
…ract-leaf bug (facebook#5056)

Adds a regression test in RelayResponseNormalizer-test.js demonstrating
the bug captbaritone identified in facebook#5056: when a plural field contains
items that are themselves arrays of abstract-typed records, the
normalizer's _getRecordType receives an array and fails with
"Expected a typename for record [...]".

The test schema does not model nested lists ([[T!]!]!), so the test
sends a doubly-nested payload to a plain plural-of-abstract field
(User.actors: [Actor]). This exercises the same
_normalizePluralLink -> _getRecordType code path described in the
issue.

Test asserts current (buggy) behavior via toThrow. The fix is left to
the maintainer; flip the assertion when the underlying bug is fixed.
…ribe block

Reviewer feedback: the regression test was nested inside
describe('User-defined getDataID') > describe('plural fields'), but
the test exercises no getDataID behavior — it just demonstrates the
malformed-payload path on User.actors and asserts the unhelpful
Invariant Violation.

Hoists the test to top-level under describe('RelayResponseNormalizer'),
near the existing scalar-warning cluster. The test can no longer reuse
the closure-scoped BarQuery (which lived in the inner describe), so it
declares its own inline graphql literal — adds one new
__generated__/RelayResponseNormalizerTest5056Query.graphql.js artifact.
Trade: honest placement vs. one extra generated file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant