Skip to content

fix(fmt): update markup_fmt to fix quadratic inline CSS formatting#34663

Merged
bartlomieju merged 5 commits into
denoland:mainfrom
Xavrir:fix-30982-bump-markup-fmt
Jun 2, 2026
Merged

fix(fmt): update markup_fmt to fix quadratic inline CSS formatting#34663
bartlomieju merged 5 commits into
denoland:mainfrom
Xavrir:fix-30982-bump-markup-fmt

Conversation

@Xavrir

@Xavrir Xavrir commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

What

Bumps the pinned markup_fmt dependency from =0.27.0 to =0.27.2
(Cargo.toml and Cargo.lock), plus a one-line type fix in cli/tools/fmt.rs
that the new version requires.

Why

deno fmt did not terminate on large HTML files with many inline style="..."
attributes (#30982; a 10 MB file ran for 15+ minutes with no output).

The cause is in markup_fmt 0.27.0. FormatCtx::format_style_attr
(src/ctx.rs) prepended a whitespace-masked copy of the entire preceding
document (self.source[0..start]) to the few bytes of inline CSS before handing
it to the external CSS formatter (malva). Since start grows linearly down the
file, each of the N style attributes does O(start) work, which is O(n^2) overall.
That prefix only existed to keep byte offsets aligned for error reporting, and
the style-attr result is trimmed and uses print_width = u16::MAX, so the prefix
never affects the output.

markup_fmt 0.27.2 (g-plane/markup_fmt#223, merged upstream) drops the prefix
and instead computes source line/column lazily, only on error, via
pos_to_line_col. That makes formatting linear again. The fix is already
released upstream, so this PR advances Deno's pin to pick it up.

How

  • Cargo.toml: change markup_fmt = "=0.27.0" to "=0.27.2".
  • Cargo.lock: regenerated with cargo update -p markup_fmt --precise 0.27.2.
    0.27.2 also pulls in memchr 2.7.5 to 2.8.1 and adds tiny_pretty 0.4.2;
    both are markup_fmt's own declared deps.
  • cli/tools/fmt.rs: change the format_embedded_html closure error type from
    std::convert::Infallible to deno_core::anyhow::Error (see below).

Source change required by 0.27.2

0.27.2 changed format_text from Result<_, FormatError<E>> to a concrete
Result<_, FormatError>, where the external formatter closure must return an
error type that converts into anyhow::Error. Deno has two call sites:

  • format_html (line 451) already returns AnyError
    (deno_core::error::AnyError = anyhow::Error), so it needed no change.
  • format_embedded_html (line 750) returned Ok::<_, std::convert::Infallible>,
    which no longer satisfies the bound. I changed it to
    Ok::<_, deno_core::anyhow::Error> to match the other site.

My first pass only checked the format_html closure and missed this second one,
so the initial commit broke the debug build. CI caught it at fmt.rs:751; this
is now fixed and cargo check -p deno compiles clean.

Verification / benchmark

I reproduced this end to end against Deno's exact pinned crate versions (stock
0.27.0 vs 0.27.2 via [patch.crates-io]), formatting HTML with N
<div style="..."> lines:

N divs 0.27.0 0.27.2 output
500 16.8 ms 4.3 ms identical
2000 225.7 ms 8.8 ms identical
8000 4180 ms 37.0 ms identical

Stock is quadratic; the bumped version is linear (about 113x faster at N=8000),
and the formatted output is byte-identical.

Checklist (PR template)

  • Descriptive conventional-commit title (fix(fmt): ...).
  • Related issue referenced (Closes #30982).
  • ./x fmt: cargo fmt --check passes on cli/tools/fmt.rs (edition 2024),
    and dprint check Cargo.toml passes.
  • ./x lint: cargo check -p deno compiles clean after the closure fix.
  • Tests: added tests/specs/fmt/html_many_inline_styles, a fmt spec that
    runs fmt --check on an HTML file with 150 inline style="..." divs and
    asserts it completes. It passes in well under a second against a local
    build; on 0.27.0 the same input does not terminate.

AI usage disclosure

This PR was prepared with help from an AI coding agent. I reviewed and verified
the diagnosis, the upstream root cause (g-plane/markup_fmt#223), the version
bump, the closure type fix, and the benchmark locally.

@deno-cla-assistant

deno-cla-assistant Bot commented Jun 1, 2026

Copy link
Copy Markdown

Deno Individual Contributor License Agreement

All contributors have signed the CLA. Thank you!

Re-run CLA check


This is an automated message from CLA Assistant

@Xavrir
Xavrir force-pushed the fix-30982-bump-markup-fmt branch from 13a3e1a to 8e3aa49 Compare June 1, 2026 12:51
`deno fmt` did not terminate on large HTML files containing many inline
`style="..."` attributes. markup_fmt 0.27.0's `format_style_attr` prepended a
whitespace-masked copy of the entire preceding document to every inline style
before handing it to the CSS formatter, making formatting O(n^2) in the number
of style attributes.

markup_fmt 0.27.2 (g-plane/markup_fmt#223) removed that prefix and instead
computes source line/column lazily only on error, restoring linear time.
Bumping the pin picks up the fix.

Adds a fmt spec test that formats an HTML file with many inline style
attributes and asserts it completes.

Closes denoland#30982
@Xavrir
Xavrir force-pushed the fix-30982-bump-markup-fmt branch from 8e3aa49 to 91e0dfb Compare June 1, 2026 13:51
markup_fmt 0.27.2 requires the external formatter closure to return an
error type that converts into anyhow::Error. The format_embedded_html
closure still returned Infallible, which no longer satisfies the bound
and broke the debug build. Use deno_core::anyhow::Error to match the
other call site.
@bartlomieju

Copy link
Copy Markdown
Member

The failing specs::fmt::external_formatter::syntax_error_in_html test is hitting a regression that shipped in markup_fmt 0.27.2, not a stale expectation.

The test asserts the syntax-error message includes correct line/column info:

syntax error 'expected close tag for opening tag <h1> from line 4, column 2' at line 4, column 7

With 0.27.2 the columns become column 0 / column 0. The cause is in markup_fmt's helpers::pos_to_line_col, changed by the same upstream commit that fixed the O(n^2) perf issue (g-plane/markup_fmt#223):

// before (correct):
ControlFlow::Break((line, offset)) => (line, pos - offset + 1),
// after (always 0):
ControlFlow::Break((line, offset)) => (line, pos - offset.max(pos)),

On the Break arm, offset is the previous newline's byte offset, always <= pos, so offset.max(pos) == pos and the column is always 0. The max guard was unnecessary (the subtraction can't underflow on that arm) and broke the result. I verified that restoring pos - offset + 1 makes this test pass again with the existing .out unchanged, so the .out is correct and should not be modified.

I opened an upstream fix with a regression test: g-plane/markup_fmt#226. Once that's released (e.g. 0.27.3), this PR just needs to bump the pin to it and CI goes green. In the meantime a temporary [patch.crates-io] git pin to the fix branch would also unblock CI.

@Xavrir

Xavrir commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for digging into this and finding the exact cause, that's a much better diagnosis than I had. Makes sense that the offset.max(pos) guard collapses the column to 0 on the Break arm.

I've pinned markup_fmt to your fix commit (bartlomieju/markup_fmt @ cc67281) via [patch.crates-io] so CI can go green and show the rest of the change is fine. I'll drop the pin and bump to 0.27.3 once #226 lands and gets released. Left the .out untouched since you confirmed it's correct.

…gression

markup_fmt 0.27.2 introduced a regression in helpers::pos_to_line_col
that reports column 0 (g-plane/markup_fmt#226). Pin to the fix commit
via [patch.crates-io] to unblock CI; revert to a 0.27.3 version bump
once released.
@Xavrir
Xavrir force-pushed the fix-30982-bump-markup-fmt branch from 002b89a to 086fa95 Compare June 1, 2026 21:16
Xavrir and others added 2 commits June 2, 2026 11:56
markup_fmt 0.27.3 includes the upstream fix for the pos_to_line_col
column-0 regression (g-plane/markup_fmt#226, now merged and released),
so the temporary [patch.crates-io] git pin is no longer needed.
@bartlomieju bartlomieju changed the title fix(fmt): bump markup_fmt to 0.27.2 to fix O(n^2) inline CSS formatting fix(fmt): update markup_fmt to fix quadratic inline CSS formatting Jun 2, 2026

@bartlomieju bartlomieju 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.

Thanks!

@bartlomieju
bartlomieju enabled auto-merge (squash) June 2, 2026 07:12
@bartlomieju
bartlomieju merged commit d18fe0a into denoland:main Jun 2, 2026
137 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants