Skip to content

fix(formatter_core): measure decided-flat fill separator as flat during group re-measure - #25276

Merged
graphite-app[bot] merged 1 commit into
mainfrom
08-04-fix_formatter_core_measure_decided-flat_fill_separator_as_flat_during_group_re-measure
Aug 4, 2026
Merged

fix(formatter_core): measure decided-flat fill separator as flat during group re-measure#25276
graphite-app[bot] merged 1 commit into
mainfrom
08-04-fix_formatter_core_measure_decided-flat_fill_separator_as_flat_during_group_re-measure

Conversation

@leaysgur

@leaysgur leaysgur commented Aug 4, 2026

Copy link
Copy Markdown
Member

Fixes #21916, As it turned out, it served as a follow-up to #16320.


When a group inside a fill item re-measures (after a preceding hard line reset measured_group_fits), the fits walk escapes the item's entry and measures the following separator with the fill's own expanded mode, materializing expanded-only content like the raw JSX whitespace {" "} and over-counting the width.

Prettier avoids this because its command stack holds the decided separator command before the item prints.

The fix: print_fill_item exposes that mode via PrinterState::fill_separator_mode, and the fits walk applies it (single-shot) to the first entry at the printed fill's own level, exactly that separator.

Skipping the re-measure instead (as Biome upstream does unconditionally) was rejected: it was removed in #16320 for diverging from Prettier.

NOTE: As noted in comment, there is a known limitation for now. This PR covers only 1-level nested fill. But in practice, I couldn't find actual code snippet hits this.

leaysgur commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the A-formatter Area - Formatter label Aug 4, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 57 untouched benchmarks
⏩ 19 skipped benchmarks1


Comparing 08-04-fix_formatter_core_measure_decided-flat_fill_separator_as_flat_during_group_re-measure (085595e) with main (ab12665)2

Open in CodSpeed

Footnotes

  1. 19 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 (f3c6953) during the generation of this report, so ab12665 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@graphite-app

graphite-app Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Merge activity

…ng group re-measure (#25276)

Fixes #21916, As it turned out, it served as a follow-up to #16320.

---

When a `group` inside a `fill` item re-measures (after a preceding hard line reset `measured_group_fits`), the fits walk escapes the item's entry and measures the following separator with the fill's own expanded mode, materializing expanded-only content like the raw JSX whitespace `{" "}` and over-counting the width.

Prettier avoids this because its command stack holds the decided separator command before the item prints.

The fix: `print_fill_item` exposes that mode via `PrinterState::fill_separator_mode`, and the fits walk applies it (single-shot) to the first entry at the printed fill's own level, exactly that separator.

Skipping the re-measure instead (as Biome upstream does unconditionally) was rejected: it was removed in #16320 for diverging from Prettier.

NOTE: As noted in comment, there is a known limitation for now. This PR covers only 1-level nested fill. But in practice, I couldn't find actual code snippet hits this.
@graphite-app
graphite-app Bot force-pushed the 08-04-fix_formatter_core_measure_decided-flat_fill_separator_as_flat_during_group_re-measure branch from 085595e to c29b587 Compare August 4, 2026 06:39
@graphite-app
graphite-app Bot merged commit c29b587 into main Aug 4, 2026
30 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Aug 4, 2026
@graphite-app
graphite-app Bot deleted the 08-04-fix_formatter_core_measure_decided-flat_fill_separator_as_flat_during_group_re-measure branch August 4, 2026 06:47
graphite-app Bot pushed a commit that referenced this pull request Aug 10, 2026
# Oxlint
### 💥 BREAKING CHANGES

- a33788e ast: [**BREAKING**] Group class heritage into `ClassHeritage` (#25360) (camc314)
- 5c5cdcd ast: [**BREAKING**] Narrow `TSInterfaceHeritage::expression` to TSTypeName (#24360) (camc314)
- 6be314f ast: [**BREAKING**] Remove duplicated `VariableDeclarator::kind` (#25319) (camc314)
- 44fd320 ast: [**BREAKING**] Split TS external modules & Namespace Declarations (#25284) (camc314)

### 🚀 Features

- ccb8fe8 linter/jsdoc: Implement `no-blank-blocks` rule (#25207) (Mikhail Baev)
- d4a897c linter/eslint: Implement `one-var` rule (#24470) (Cole Ellison)
- 5ab9340 linter/jsx-a11y/anchor-has-content: Add options to match eslint (#24571) (Cole Ellison)

### 🐛 Bug Fixes

- b746e00 linter/eslint/no-implicit-coercion: Preserve template coercion whitespace (#25470) (camc314)
- a92c541 linter: Preserve source text for JS plugin ignore fixes (#25280) (Norcleeh)
- 675c840 linter/eslint/prefer-promise-reject-errors: Handle parenthesized calls (#25378) (camc314)
- 1703739 linter/unicorn/new-for-builtins: Ignore optional chains (#25411) (tanakalucky)
- 95ece63 linter/unicorn/prefer-code-point: Downgrade the auto-fix to dangerous (#25412) (leemr)
- c451a0e linter/vitest: Validate `consistent-test-filename` regex patterns (#25408) (Mikhail Baev)
- 937825c react_compiler: Disable exhaustive memo validation by default (#25417) (Boshen)
- f0f7dae linter/eslint/no-unused-vars: Report invalid regex options (#25380) (Cameron)
- 44e73fd linter/unicorn/prefer-array-flat: Fix `concat.apply` suggestions (#25373) (Cameron)
- 6846a9a linter/react/rules-of-hooks: Detect constructor callbacks (#25377) (camc314)
- b247a9d linter/unicorn/new-for-builtins: Support `Float16Array` (#25382) (tanakalucky)
- 19109cd linter/unicorn/error-message: Support `SuppressedError` messages (#25375) (camc314)
- 9c13f5e linter: Assert token lookup invariants (#25368) (camc314)
- bc35f83 linter/eslint/no-unused-vars: Bound catch parameter lookup (#25367) (camc314)
- c159fb9 linter/unicorn/switch-case-braces: Bound token lookup (#25363) (camc314)
- 03b2eb2 linter/unicorn/no-static-only-class: Bound token lookup (#25361) (camc314)
- 0afc59e linter/unicorn/empty-brace-spaces: Bound token lookup (#25353) (camc314)
- 2963d98 linter/eslint/no-unreachable-loop: Do not report loops whose body has a finally block (#25335) (Todor Andonov)
- 589e5fb linter/eslint/no-param-reassign: Validate `ignorePropertyModificationsForRegex` property (#25346) (Mikhail Baev)
- aae5d8b linter/eslint/no-throw-literal: False positive on variable declared without initializer (#25275) (cjnoname)
- 6b1c479 oxlint: Normalize customized rule names (#25316) (camc314)
- d494eb5 linter/unicorn/consistent-existence-index-check: Bound token lookup (#25325) (camc314)
- 4266037 linter/typescript/prefer-namespace-keyword: Bound token lookup (#25322) (camc314)
- 4745b4e linter/typescript/no-namespace: Bound token lookup (#25321) (camc314)
- 648a481 linter/eslint/one-var: Avoid joining exported declarations (#25314) (camc314)
- 9573937 linter/typescript: Validate `ban-ts-comment` description_format (#25320) (Mikhail Baev)
- ebf7d18 linter/typescript/consistent-type-definitions: Bound token lookup (#25281) (camc314)
- 1501ccf linter/typescript/consistent-generic-constructors: Bound token lookup (#25258) (camc314)

### ⚡ Performance

- 8f784f3 linter: Reduce rule config dispatch size (#25461) (Boshen)
- 2de4ec2 linter: Reduce visitor code size (#25441) (Boshen)
- 6fb7f47 linter/unicorn/prefer-export-from: Narrow `ExportFromDeclaration` lookup (#25381) (camc314)
- e3f6263 linter/unicorn/prefer-default-parameters: Avoid reference allocation  (#25379) (camc314)
- d863473 linter/vue/max-props: Narrow AST dispatch (#25372) (camc314)
- 273d867 linter: Avoid diagnostic sorting after applying fixes (#25079) (Sysix)
- 4ec9189 oxlint/lsp: Avoid second lock for getting/removing unused directives (#25350) (Sysix)
- 6c0d01b oxlint/lsp: Preallocate fix-content vec (#25351) (Sysix)
- 3a94055 linter: Avoid per-call heap allocations in jest and unicorn helpers (#25210) (Connor Shea)
- 4abff11 linter: Avoid redundant message work in the agent reporter (#25315) (Connor Shea)
- 6bb5421 linter: Hoist `current_dir` out of the stylish reporter loop (#25313) (Connor Shea)
- 9a7c323 linter: Compute diagnostic `Info` once per diagnostic in junit reporter (#25312) (Connor Shea)
- 1cf7dde oxlint: Render JSON report into a single buffer (#25295) (connorshea)
- 8492cfd linter/typescript/ban-ts-comment: Bail early with substring guard (#25301) (Jacob Asper)
- 7607f04 linter/typescript/ban-tslint-comment: Replace regex with manual parser (#25299) (Jacob Asper)
# Oxfmt
### 💥 BREAKING CHANGES

- a33788e ast: [**BREAKING**] Group class heritage into `ClassHeritage` (#25360) (camc314)
- 5c5cdcd ast: [**BREAKING**] Narrow `TSInterfaceHeritage::expression` to TSTypeName (#24360) (camc314)
- 6be314f ast: [**BREAKING**] Remove duplicated `VariableDeclarator::kind` (#25319) (camc314)
- 44fd320 ast: [**BREAKING**] Split TS external modules & Namespace Declarations (#25284) (camc314)

### 🚀 Features

- fd02a89 oxfmt: Dispatch yaml-in-css(frontmatter) to `oxc_formatter_yaml` (#25336) (leaysgur)
- ab12665 formatter_core: Add `hardlineWithoutBreakParent` equivalent IR (#25273) (leaysgur)

### 🐛 Bug Fixes

- 95dc917 oxfmt: Drop IR Space at line start for js-in-xxx (#25460) (leaysgur)
- b63eccc formatter: Keep comments after TS this_param (#25459) (leaysgur)
- ab52a59 formatter: Format xxx-in-js inside JSDoc js fence (#25414) (leaysgur)
- 1a2c64a formatter,oxfmt: Apply effective print width for JSDoc fence (#25413) (leaysgur)
- 2eaede9 formatter_core: Unify leading-BOM handlings (#25340) (leaysgur)
- ef1d04b formatter: Break mapped type brackets (#25297) (leaysgur)
- 4e6f3f1 formatter: Break index signature brackets (#25296) (leaysgur)
- e23dccf formatter_css: Bump oxc-css-parser to accept unknown at-rule with interpolated (#25277) (leaysgur)
- c29b587 formatter_core: Measure decided-flat fill separator as flat during group re-measure (#25276) (leaysgur)
- f3c6953 formatter_yaml: Don't rewrite overflowing key to implicit (#25274) (leaysgur)

### ⚡ Performance

- c9d1a5b oxfmt: Spawn tinypool lazily (#25298) (leaysgur)

### 📚 Documentation

- 6eae5c9 formatter,oxfmt: Record embed-layer decisions in place (#25422) (leaysgur)
- 51224a7 formatter_yaml: Pin EOF blank lines divergence (#25269) (leaysgur)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-formatter Area - Formatter

1 participant