fix(fmt): delegate component expression formatting to markup_fmt by janosh · Pull Request #29763 · denoland/deno

janosh · 2025-06-16T12:43:08Z

This PR now solves component expression formatting by delegating parsing/formatting to markup_fmt instead of using Deno-side regex placeholder preprocessing.

What changed

upgraded markup_fmt and aligned Deno formatter config with the newer API
removed local format_expressions_in_text(...) preprocessing/restore pipeline
routed component formatting through markup_fmt as the single parser/formatter source of truth
updated Svelte/Vue fixtures and related fmt expectations to current formatter output
preserved Jinja/Nunjucks pseudo-block text in callback handling to avoid invalid JS formatting attempts

Why this approach

Earlier review feedback called out regex preprocessing as fragile (for example formatting inside comments). This version avoids that class of issues by relying on markup_fmt's native parse tree and language-aware formatting behavior.

new function `format_expressions_in_text` handles JS expressions in text, allowing for temporary placeholders during formatting of component files. The `format_html` function preprocesses expressions before markup parsing and restores them after formatting.

…new formatting test for Vue - new `badly_formatted.vue` and its fixed counterpart - updated existing Svelte test file to include dynamic style

CLAassistant · 2025-06-16T12:43:15Z

All committers have signed the CLA.

dsherret · 2025-06-23T17:23:14Z

+        "svelte" | "astro" => (r"\{([^{}]+)\}", "{", "}"),
+        "vue" => (r"\{\{([^{}]+)\}\}", "{{", "}}"),


This seems very error prone to do (ex. it would format within comments). Probably it should be handled by markup_fmt instead and it can call out to format these expressions with the appropriate formatter.

@dsherret thanks for the earlier feedback! sorry for the long radio silence, just remembered i've been meaning to finish this 😅

i reworked this PR to address your concern about the regex-based preprocessing:

dropped local expression placeholder pipeline in cli/tools/fmt.rs:

deleted format_expressions_in_text(...)

removed preprocess/restore logic around format_html(...)

format_html(...) now passes the original text directly to markup_fmt (single source of truth for parsing/formatting), instead of trying to rewrite expressions in Deno first

Upgraded markup_fmt from 0.19.1 to 0.22.0

ready for another review

- remove local placeholder preprocessing in html formatter - rely on markup_fmt parsing for interpolated style expressions - bump markup_fmt to 0.22.0 and update config compatibility

- revert the Svelte style interpolation fixture in fmt testdata to the upstream-supported form - keep the change minimal to unblock flaky integration::fmt::fmt_test failures on CI

- bump markup_fmt to 0.26.0 and update lockfile - add required markup_fmt language config fields in fmt options - keep Svelte style interpolation coverage in existing fmt fixtures

- skip TypeScript formatting for markup_fmt jinja pseudo-extensions - avoid invalid JS parse errors for Nunjucks/Jinja filters and block statements - keep markup_fmt 0.26 upgrade while restoring njk spec and integration behavior

- align badly_formatted_fixed.vue with current formatter spacing output - fix integration::fmt::fmt_test expectation mismatch on CI

janosh · 2026-03-02T12:33:03Z

@bartlomieju the "ensure CI success" error looks like a false positive. anything i need to do here?

bartlomieju · 2026-03-15T08:46:31Z

Review from Claude. No 2 and 4 look concerning :(

1. PR title/description is misleading

The title says "Add expression formatting in CSS strings for components" and the description mentions a format_expressions_in_text function — but the diff contains neither. The actual change is a markup_fmt version bump plus config field additions. The expression formatting likely comes from the library upgrade itself, not from code in this PR.

2. Svelte test file loses trailing newline

-</style>
+</style>
\ No newline at end of file

badly_formatted.svelte now lacks a trailing newline. The fixed counterpart (badly_formatted_fixed.svelte) already ends with a newline, so after formatting, the file gains a newline it didn't have. This is fine functionally but the input file should probably retain the trailing newline for consistency — removing it seems unintentional.

3. New config fields use hardcoded defaults without documentation

vue_component_case: config::VueComponentCase::Ignore,
angular_next_control_flow_same_line: true,

These are set in two places (format_embedded_html and get_resolved_markup_fmt_config) with no user-facing configuration. If markup_fmt 0.26.0 changes behavior based on these, users can't control it. Are these the correct defaults? VueComponentCase::Ignore seems safe, but angular_next_control_flow_same_line: true is an opinionated choice with no way to override.

4. Vue formatted output swaps quote styles

The formatted Vue output changes " to ' for attribute values containing JS strings:

<!-- input -->
:style="{ width: a + b + 'px' }"
<!-- output -->
:style='{ width: a + b + "px" }'

This is technically valid HTML but may surprise users who consistently use double quotes for attributes. This behavior comes from markup_fmt and may not be configurable — worth confirming it's intentional and won't break tools that parse HTML expecting double-quoted attributes.

5. Jinja handler matches on string prefix — fragile

ext if ext.starts_with("markup-fmt-jinja-") => {
    Ok(Cow::from(text))
}

This relies on markup_fmt 0.26.0 passing extensions with exactly this prefix. If the library changes the prefix convention in a future version, this silently breaks. A comment referencing where this convention is documented in markup_fmt would help.

Wire vueComponentCase and angularNextControlFlowSameLine through fmt config and formatter resolution so markup_fmt behavior is explicit and user-configurable, while keeping fixture behavior stable and documenting jinja callback handling.

janosh · 2026-03-15T12:07:27Z

@bartlomieju thanks a lot for the thorough review, all good points!

retitled and rewrote the PR description to reflect the real scope (markup_fmt-driven behavior) since the initial attempt at closing fmt(svelte): formatter error on dynamic style in svelte #26999 was very different
restored the trailing newline in badly_formatted.svelte for fixture consistency.
added user-facing fmt config support for vueComponentCase and angularNextControlFlowSameLine
- wired these through config resolution and formatter code paths (including workspace config merge behavior).
kept expected fixture output aligned with current formatter behavior and clarified this behavior is owned by markup_fmt.
kept the jinja passthrough behavior but added an upstream source reference in code for the markup-fmt-jinja-* extension naming convention.

Align import ordering and match arm formatting with tools/format.js output to satisfy CI formatting checks.

Include the new fmt options in the unknown-field expected list so malformed fmt config specs match current serde error output.

janosh · 2026-03-22T08:35:47Z

would be great to get this merged! i'm currently blocked from upgrading beyond deno 2.3.7 by this. let me know if anything left to address

…e config duplication - Extract `embedded_markup_language_options()` that builds the shared `LanguageOptions` with vue/angular config resolution - `format_embedded_html` now takes a `&LanguageOptions` instead of individual vue_component_case + angular_next_control_flow_same_line params - `get_resolved_markup_fmt_config` uses struct update syntax (`..embedded_markup_language_options()`) to override only the fields that differ for top-level component files - Net reduction of ~30 lines of duplicated struct field assignments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

bartlomieju · 2026-03-24T12:23:00Z

I will land #32949 first, to not confuse crate upgrade with additional changes

- Take upstream's markup_fmt 0.27.0 and malva 0.15.2 versions - Keep PR's embedded_markup_language_options dedup for format_embedded_html - Keep PR's config-resolved vue_component_case and angular_next_control_flow_same_line - Take upstream's Jinja passthrough comment referencing v0.27.0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

bartlomieju

Thanks

janosh added 2 commits June 16, 2025 08:35

feat(tests): more realistic formatting test for Svelte component and …

403b608

…new formatting test for Vue - new `badly_formatted.vue` and its fixed counterpart - updated existing Svelte test file to include dynamic style

janosh added 2 commits June 16, 2025 08:46

revert accidental changes in badly_formatted.svelte

1d9dec5

lint cli/tools/fmt.rs

0fb1bde

janosh changed the title ~~Add expression formatting in CSS strings for components~~ feat: Add expression formatting in CSS strings for components Jun 16, 2025

janosh added 2 commits June 17, 2025 06:19

Merge branch 'main' into fix-gh-26999

5c1a7c6

fix lint.js

bbee7b7

janosh force-pushed the fix-gh-26999 branch from dac2cb3 to bbee7b7 Compare June 17, 2025 10:47

dsherret reviewed Jun 23, 2025

View reviewed changes

janosh added 6 commits February 25, 2026 21:33

Use markup_fmt for component expressions

9afbb37

- remove local placeholder preprocessing in html formatter - rely on markup_fmt parsing for interpolated style expressions - bump markup_fmt to 0.22.0 and update config compatibility

Merge remote-tracking branch 'upstream/main' into fix-gh-26999

92cc99e

Restore stable Svelte fmt fixtures

963c30f

- revert the Svelte style interpolation fixture in fmt testdata to the upstream-supported form - keep the change minimal to unblock flaky integration::fmt::fmt_test failures on CI

Upgrade markup_fmt and Svelte fixtures

7861d3f

- bump markup_fmt to 0.26.0 and update lockfile - add required markup_fmt language config fields in fmt options - keep Svelte style interpolation coverage in existing fmt fixtures

Preserve jinja text in markup formatter callback

5d25184

- skip TypeScript formatting for markup_fmt jinja pseudo-extensions - avoid invalid JS parse errors for Nunjucks/Jinja filters and block statements - keep markup_fmt 0.26 upgrade while restoring njk spec and integration behavior

Update Vue fmt fixture for markup_fmt 0.26

7347e7a

- align badly_formatted_fixed.vue with current formatter spacing output - fix integration::fmt::fmt_test expectation mismatch on CI

Merge branch 'main' into fix-gh-26999

ffb3f90

janosh changed the title ~~feat: Add expression formatting in CSS strings for components~~ feat(fmt): delegate component expression formatting to markup_fmt Mar 15, 2026

janosh added 2 commits March 15, 2026 12:08

chore(fmt): apply formatter output in fmt.rs

c5f3978

Align import ordering and match arm formatting with tools/format.js output to satisfy CI formatting checks.

test(fmt): update malformed config expectation

b3d3a4c

Include the new fmt options in the unknown-field expected list so malformed fmt config specs match current serde error output.

bartlomieju requested a review from dsherret March 16, 2026 08:07

bartlomieju changed the title ~~feat(fmt): delegate component expression formatting to markup_fmt~~ fix(fmt): delegate component expression formatting to markup_fmt Mar 24, 2026

bartlomieju approved these changes Mar 24, 2026

View reviewed changes

bartlomieju merged commit 1c50485 into denoland:main Mar 24, 2026
112 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(fmt): delegate component expression formatting to markup_fmt#29763

fix(fmt): delegate component expression formatting to markup_fmt#29763
bartlomieju merged 18 commits intodenoland:mainfrom
janosh:fix-gh-26999

janosh commented Jun 16, 2025 •

edited

Loading

Uh oh!

CLAassistant commented Jun 16, 2025 •

edited

Loading

Uh oh!

dsherret Jun 23, 2025 •

edited

Loading

Uh oh!

janosh Feb 25, 2026

Uh oh!

janosh commented Mar 2, 2026

Uh oh!

bartlomieju commented Mar 15, 2026 •

edited

Loading

Uh oh!

janosh commented Mar 15, 2026

Uh oh!

janosh commented Mar 22, 2026

Uh oh!

bartlomieju commented Mar 24, 2026

Uh oh!

bartlomieju left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		"svelte" \| "astro" => (r"\{([^{}]+)\}", "{", "}"),
		"vue" => (r"\{\{([^{}]+)\}\}", "{{", "}}"),

Conversation

janosh commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Why this approach

Uh oh!

CLAassistant commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dsherret Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janosh Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

janosh commented Mar 2, 2026

Uh oh!

bartlomieju commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. PR title/description is misleading

2. Svelte test file loses trailing newline

3. New config fields use hardcoded defaults without documentation

4. Vue formatted output swaps quote styles

5. Jinja handler matches on string prefix — fragile

Uh oh!

janosh commented Mar 15, 2026

Uh oh!

janosh commented Mar 22, 2026

Uh oh!

bartlomieju commented Mar 24, 2026

Uh oh!

bartlomieju left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

janosh commented Jun 16, 2025 •

edited

Loading

CLAassistant commented Jun 16, 2025 •

edited

Loading

dsherret Jun 23, 2025 •

edited

Loading

bartlomieju commented Mar 15, 2026 •

edited

Loading