Skip to content

fix(typescript): dedupe discriminant in generated SSE protocol-union wire tests#15563

Merged
patrickthornton merged 2 commits into
mainfrom
patrick/typescript/sse-protocol-union-test-dedup
Apr 29, 2026
Merged

fix(typescript): dedupe discriminant in generated SSE protocol-union wire tests#15563
patrickthornton merged 2 commits into
mainfrom
patrick/typescript/sse-protocol-union-test-dedup

Conversation

@patrickthornton
Copy link
Copy Markdown
Contributor

@patrickthornton patrickthornton commented Apr 29, 2026

Summary

Generated wire tests for SSE endpoints with protocol-discriminated unions previously emitted both an explicit discriminant key and a spread of the deserialized data, which itself already contained the discriminant. Under TypeScript strict, this produced a TS2783: '<field>' is specified more than once, so this usage will be overwritten build error (e.g. auth0/node-auth0#1331).

This PR also fixes a related dead-code bug: the override that promotes a discriminant value from the data payload onto the SSE event: line was running after the discriminant had been deleted from the copy, so it never fired.

Example

For an SSE endpoint with a protocol-discriminated union and an example whose event: line is empty:

- event: ""
  data:
    type: "group.created"
    offset: "offset-1"
    group_id: "g1"

Before:

expect(events).toEqual([
    {
        type: "",                       // TS2783 — overwritten by the spread below
        ...{
            type: "group.created",
            offset: "offset-1",
            group_id: "g1",
        },
    },
]);
// raw response body: 'event: \ndata: {...}\n'  — empty event line

After:

expect(events).toEqual([
    {
        type: "group.created",
        offset: "offset-1",
        group_id: "g1",
    },
]);
// raw response body: 'event: group.created\ndata: {"offset":"offset-1","group_id":"g1"}\n'

The discriminant is now lifted onto the SSE event: line and stripped from data, matching what the runtime SDK's injectDiscriminator produces.

Test plan

  • Added a new seed fixture endpoint streamEventsDiscriminantInData in server-sent-event-examples that exercises the empty-event:-line + discriminant-in-data shape.
  • Existing streamEventsContextProtocol snapshot loses its redundant event: "completion", ...{event: "completion", ...} wrapper — diff is clean.
  • tsc --strict --noEmit on the regenerated completions.test.ts passes (no TS2783).
  • pnpm seed test --generator ts-sdk --fixture server-sent-event-examples passes.

🤖 Generated with Claude Code


Open in Devin Review

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@patrickthornton patrickthornton self-assigned this Apr 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🌱 Seed Test Selector

Select languages to run seed tests for:

  • Python
  • TypeScript
  • Java
  • Go
  • Ruby
  • C#
  • PHP
  • Swift
  • Rust
  • OpenAPI

How to use: Click the ⋯ menu above → "Edit" → check the boxes you want → click "Update comment". Tests will run automatically and snapshots will be committed to this PR.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-04-23T04:59:11Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
csharp-sdk square 56s (n=5) 86s (n=5) 55s -1s (-1.8%)
go-sdk square 115s (n=5) 132s (n=5) 117s +2s (+1.7%)
java-sdk square 179s (n=5) 207s (n=5) 188s +9s (+5.0%)
php-sdk square 43s (n=5) 66s (n=5) 42s -1s (-2.3%)
python-sdk square 115s (n=5) 223s (n=5) 122s +7s (+6.1%)
ruby-sdk-v2 square 130s (n=5) 159s (n=5) 69s -61s (-46.9%)
rust-sdk square 166s (n=5) 163s (n=5) 184s +18s (+10.8%)
swift-sdk square 41s (n=5) 290s (n=5) 43s +2s (+4.9%)
ts-sdk square 74s (n=5) 86s (n=5) 97s +23s (+31.1%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-04-23T04:59:11Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-04-29 16:29 UTC

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

Docs Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-04-23T04:59:11Z).

Fixture main PR Delta
docs 301.0s (n=5) 298.7s (35 versions) -2.3s (-0.8%)

Docs generation runs fern generate --docs --preview end-to-end against the benchmark fixture with 35 API versions (each version: markdown processing + OpenAPI-to-IR + FDR upload).
Delta is computed against the nightly baseline on main.
Baseline from nightly run(s) on main (latest: 2026-04-23T04:59:11Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-04-29 16:28 UTC

@patrickthornton patrickthornton merged commit 8729bff into main Apr 29, 2026
197 checks passed
@patrickthornton patrickthornton deleted the patrick/typescript/sse-protocol-union-test-dedup branch April 29, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants