Skip to content

fix(formatter): preserve trailing comma in TSX arrow functions with default type params#21151

Merged
leaysgur merged 1 commit intooxc-project:mainfrom
jsmecham:fix/tsx-trailing-comma-with-default-type-param
Apr 8, 2026
Merged

fix(formatter): preserve trailing comma in TSX arrow functions with default type params#21151
leaysgur merged 1 commit intooxc-project:mainfrom
jsmecham:fix/tsx-trailing-comma-with-default-type-param

Conversation

@jsmecham
Copy link
Copy Markdown
Contributor

@jsmecham jsmecham commented Apr 8, 2026

Summary

Fixes #21150

When a single-parameter generic arrow function in a .tsx file has a default type (e.g. <Options = any>), the formatter was incorrectly removing the trailing comma. Prettier preserves it as <Options = any,>.

The existing should_force_trailing_comma_for_arrow_function logic handles the simple case (<T,>) correctly, but it also skipped forcing the comma when the parameter had a default type. Prettier's equivalent only skips the forced comma when a constraint (extends clause) is present — not for default types. This PR aligns with that behavior.

Input

const AppWrapper = <Options = any,>({
  appName,
}: {
  appName: string;
}) => null;

const getProps = <T = Record<string, unknown>,>(
  option: T | undefined,
): Partial<T> => ({});

Before (incorrect)

const AppWrapper = <Options = any>({ appName }: { appName: string }) => null;

const getProps = <T = Record<string, unknown>>(
  option: T | undefined,
): Partial<T> => ({});

After (matches Prettier)

const AppWrapper = <Options = any,>({ appName }: { appName: string }) => null;

const getProps = <T = Record<string, unknown>,>(
  option: T | undefined,
): Partial<T> => ({});

Root cause

In should_force_trailing_comma_for_arrow_function (type_parameters.rs), the early-return condition checked for both a constraint and a default type:

// Before
if params.first().is_some_and(|t| t.constraint().is_some() || t.default().is_some()) {
    return false;
}

Prettier's equivalent only skips forcing the comma when a constraint is present. See Prettier source.

Fix

Removed || t.default().is_some() from the condition:

// After
if params.first().is_some_and(|t| t.constraint().is_some()) {
    return false;
}

Prettier conformance: no regressions (746/753 JS, 591/601 TS — unchanged).

AI Disclosure

This PR was co-authored with Claude Code (AI assistant), as noted in the commit. The fix was reviewed, tested against the full Prettier conformance suite, and verified to produce no regressions.

@github-actions github-actions bot added A-formatter Area - Formatter C-bug Category - Bug labels Apr 8, 2026
…efault type params

Trailing commas are required in single-param generic arrow functions in
.tsx files to disambiguate from JSX. A default type alone does not
disambiguate, so the comma must still be forced — only a constraint
(extends clause) provides sufficient disambiguation.

Closes oxc-project#21150

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jsmecham jsmecham force-pushed the fix/tsx-trailing-comma-with-default-type-param branch from 5208845 to ea5bfe1 Compare April 8, 2026 01:16
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 8, 2026

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing jsmecham:fix/tsx-trailing-comma-with-default-type-param (ea5bfe1) with main (eab13b3)2

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (addcd02) during the generation of this report, so eab13b3 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copy link
Copy Markdown
Member

@leaysgur leaysgur left a comment

Choose a reason for hiding this comment

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

Thank you~!

@leaysgur leaysgur added the 0-merge Merge with Graphite Merge Queue label Apr 8, 2026
@graphite-app
Copy link
Copy Markdown
Contributor

graphite-app bot commented Apr 8, 2026

Merge activity

  • Apr 8, 5:35 AM UTC: @jsmecham we removed the merge queue label because we could not find a Graphite account associated with your GitHub profile.

You must have a Graphite account in order to use the merge queue. Create an account and try again using this link

@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 8, 2026
@leaysgur leaysgur merged commit 4da53e5 into oxc-project:main Apr 8, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-formatter Area - Formatter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

formatter: Diff with Prettier on trailing comma in generic type parameters in .tsx files

2 participants