Fix redundant undefined wrapping in declaration emit#4302
Open
jakebailey wants to merge 2 commits into
Open
Conversation
jakebailey
force-pushed
the
jabaile/fix-4079
branch
from
003f8d5 to
c78e0e1
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns tsgo’s declaration emit for defaulted/optional parameters with upstream tsc by avoiding redundant | undefined wrapping (and the extra parentheses that come with it) when the serialized type already includes or could already include undefined. It adds a compiler regression test capturing the reported “curry” nested-signature case.
Changes:
- Add a new compiler test covering primitive vs. non-primitive unions and nested “curry” signatures, with updated baselines matching
tsc’s formatting. - Teach declaration serialization to opportunistically extend simple syntactic type nodes with
| undefined(only when safe), rather than always wrapping via optional-type construction. - Avoid adding an extra
| undefinedpseudo-type wrapper when the pseudo-type may already includeundefined.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/tests/cases/compiler/declarationEmitOptionalParameterUndefined.ts | New regression test reproducing the optional/defaulted parameter emit differences (including nested curry signatures). |
| testdata/baselines/reference/compiler/declarationEmitOptionalParameterUndefined.types | Type baseline asserting the corrected, tsc-style undefined/parentheses behavior. |
| testdata/baselines/reference/compiler/declarationEmitOptionalParameterUndefined.symbols | Symbol baseline for the new regression test. |
| testdata/baselines/reference/compiler/declarationEmitOptionalParameterUndefined.js | JS + .d.ts baseline verifying the fixed declaration emit output. |
| internal/pseudochecker/lookup.go | Export helper to conservatively detect type nodes that might already include undefined. |
| internal/checker/nodebuilderimpl.go | Update declaration type serialization to avoid redundant ` |
hkleungai
reviewed
Jun 13, 2026
| //// [declarationEmitOptionalParameterUndefined.d.ts] | ||
| export declare function simple_primitive(foo: number | boolean | null | undefined, _: string): void; | ||
| export declare function simple_primitive_with_explicit_undefined(foo: number | boolean | null | undefined, _: string): void; | ||
| export declare function simple_nonPrimitive(foo: (number | RegExp | null) | undefined, _: string): void; |
There was a problem hiding this comment.
By keeping the bracket on foo, is it done for matching TS 6 behavior?
But on the other hand, it is indeed redundant, in case when the parameter is purely an union of types. It may cause downstream typechecker to process a bit more lexical tokens I suppose? 😕
Member
Author
There was a problem hiding this comment.
Was able to improve it
jakebailey
force-pushed
the
jabaile/fix-4079
branch
from
c78e0e1 to
716f6c8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4079
Closes #4172