Skip to content

fix(formatter): keep JSX expression child flat when fill expands its separator#21917

Open
jsmecham wants to merge 1 commit into
oxc-project:mainfrom
jsmecham:fix/jsx-expression-child-fill-expansion
Open

fix(formatter): keep JSX expression child flat when fill expands its separator#21917
jsmecham wants to merge 1 commit into
oxc-project:mainfrom
jsmecham:fix/jsx-expression-child-fill-expansion

Conversation

@jsmecham

@jsmecham jsmecham commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #21916

When a JSX expression child whose flat width fits at its current indent is followed by literal text such as (, the formatter was breaking the expression across three lines ({, content, }) even though the line as written is within printWidth. Prettier leaves it inline.

Input

const App = () => {
  return (
    <O>
      <I>
        <x />
        {f(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).y} (
        {label})
      </I>
    </O>
  );
};

Before (incorrect)

const App = () => {
  return (
    <O>
      <I>
        <x />
        {
          f(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).y
        } (
        {label})
      </I>
    </O>
  );
};

After (matches Prettier)

const App = () => {
  return (
    <O>
      <I>
        <x />
        {f(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).y} (
        {label})
      </I>
    </O>
  );
};

Root cause

In the printer's Fill, each entry's fit is measured with a flat-only FitsMeasurer bounded by SingleEntryPredicate, so the fill correctly determined the expression entry fit on a single line. While printing that flat entry, however, the inner expression group (group(["{", indent([softline, expr]), softline, lineSuffixBoundary, "}"])) re-ran its own fits check via Printer::fits, which uses AllPredicate and walks past the entry boundary into subsequent fill entries. The accumulated width frequently exceeded printWidth, so the inner group expanded — even though the fill had already proven the entry fits flat at the current column.

Soft line breaks don't terminate a flat-mode fits walk (they're a no-op), so the measurement happily kept reading through the next item, separator, and so on until either a hard break or the end of the document.

Fix

Adds an in_flat_fill_item flag on the printer state that's set while print_fill_item is printing an entry in flat mode. The StartGroup arm honors it and treats nested groups the same way it would when a parent group has already verified the fit — i.e. it skips the re-measurement and prints the inner group flat.

The flag is intentionally scoped to regular groups: BestFitting still goes through print_best_fitting's variant selection so JSX child lists with both flat and expanded variants (the <Link>BREAK THIS</Link> pattern in issue-16199.jsx) continue to choose the multi-line variant when the surrounding context is too wide for the flat one.

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.

@codspeed-hq

codspeed-hq Bot commented Apr 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing jsmecham:fix/jsx-expression-child-fill-expansion (79d2bfd) with main (4380812)

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.

@jsmecham jsmecham force-pushed the fix/jsx-expression-child-fill-expansion branch 2 times, most recently from 370a8e3 to 54b0a82 Compare April 28, 2026 20:16
…separator

Fixes oxc-project#21916

When formatting a JSX child list, the printer's `Fill` correctly determined
that an expression entry (e.g. `{toCash(...).display_cents}`) fit in flat
mode even though the trailing separator needed to break. While printing
that flat entry, however, inner groups re-ran their own `fits` check using
`AllPredicate`, which walked past the entry boundary into subsequent fill
entries — frequently overshooting `printWidth` and causing the expression
group to needlessly expand into `{ \n expr \n }`.

This adds an `in_flat_fill_item` flag on the printer state that's set while
`print_fill_item` is printing a flat entry. Inner regular groups honor it
and skip re-measuring (since the fill already verified the entry fits),
while `BestFitting` still goes through its variant selection so JSX child
lists with both flat and expanded variants can still pick the right form.

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

Co-authored-by: Claude <noreply@anthropic.com>
@jsmecham jsmecham force-pushed the fix/jsx-expression-child-fill-expansion branch from 54b0a82 to 79d2bfd Compare April 28, 2026 20:32
@leaysgur

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@leaysgur leaysgur left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

# This branch
const App = () => {
  return (
    <O>
      <I>
        <x />
        {f(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).y}{" "}
        ({label})
      </I>
    </O>
  );
};


# pnpm dlx prettier --print-width=100 a.tsx
const App = () => {
  return (
    <O>
      <I>
        <x />
        {
          f(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).y
        }{" "}
        ({label})
      </I>
    </O>
  );
};

❯ cat a.tsx
const App = () => {
  return (
    <O>
      <I>
        <x />
        {f(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).y} (
        {label})
      </I>
    </O>
  );
};

Found a regression.
Looks similar with attached fixture, but length of xxxx... is different.

main branch seems fine for this. Playground

@camc314 camc314 added the A-formatter Area - Formatter label May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-formatter Area - Formatter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

formatter: JSX expression child unnecessarily expanded across 3 lines when it fits within printWidth

3 participants