Call the builder with every node when the output is escaped by firefoxic · Pull Request #2175 · postcss/postcss

firefoxic · 2026-10-18T15:05:01Z

Closes #2174.

Since 8.5.29 the stringifier buffers the output from the first < on and, where it escaped anything, hands the builder all of it as a single piece. A builder is documented to be called with every node's string and node instance, so a stylesheet holding <style, </style or <!-- no longer tells a custom syntax which part of the output belongs to which node.

The escaping is what the buffer is for, so the text cannot be escaped piece by piece. It can be cut back into the pieces it was built from: escaping only ever replaces a < with the three characters longer \3c, so the escaped text holds every part in the order the chunks were buffered in, each around its own nodes. This PR does that, so the builder calls come back.

The source map of the escaping stays as precise as it is in 8.5.30: each piece is mapped where its node stands, rather than the whole output standing at the root. The mapping added by #2169 is not added again, as test/map.test.ts shows.

let { parse } = require('postcss')
let stringify = require('postcss/lib/stringify')

let root = parse('a{}b{color:red}</style>c{}')

stringify(root, (chunk, node, type) => {
  console.log(JSON.stringify([chunk, node && node.type, type]))
})

On 8.5.30:

["a{}b{color:red}\\3c /style>c{}", "root", "start"]

Here:

["a{", "rule", "start"]
["}", "rule", "end"]
["b{", "rule", "start"]
["color:red", "decl", undefined]
["}", "rule", "end"]
["\\3c /style>c{", "rule", "start"]
["}", "rule", "end"]

Summary by CodeRabbit

Bug Fixes
- Improved escaping of HTML-sensitive text in style content, including closing style tags. Output chunks retain their associated nodes and boundary labels, and combining the chunks produces the same result as stringifying the root. Builders are invoked for each node when style content is escaped, keeping generated output and per-node information consistent.

Since 8.5.29 the stringifier of ours buffers the output from the first `<` on and, where it escaped anything, hands the builder all of it as a single piece — of the root's own, marked with `start` since 8.5.30. A builder is documented to be called with every node's string and node instance, and ours is how a custom syntax gets to know which part of the output belongs to which node, so the escaping took that away from every builder that is given a stylesheet holding `<style`, `</style` or `<!--`. The escaping is what the buffer is for, so the text cannot be escaped piece by piece. It can be cut back into the pieces it was built from: escaping only ever replaces a `<` with the three characters longer `\3c `, so the escaped text holds every part in the order the chunks were buffered in, each around its own nodes. Doing that restores the builder calls, and the source map of the escaping stays as precise as it is now, since each piece is mapped where its node stands rather than the whole output standing at the root.

coderabbitai · 2026-10-18T15:05:13Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 284a98f9-3d5a-49e0-926b-d30d5e4c0419

📥 Commits

Reviewing files that changed from the base of the PR and between 80ff0bc and 3274524.

📒 Files selected for processing: 1

lib/stringify.js

💤 Files with no reviewable changes: 1

lib/stringify.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

When stringification escapes output, SafeStringifier splits the escaped text across its original chunks and calls the builder for each chunk with its associated child and type. A test checks the callback data and confirms that the chunks join to root.toString().

Changes: Escaped builder callbacks

Layer / File(s)	Summary
Chunked escaping and callback coverage `lib/stringify.js`, `test/stringifier.test.js`	`SafeStringifier.escapeHTML` accepts a print callback, splits escaped output across the original chunks, and sends each chunk to the builder with its child and type. The test checks the emitted chunks, node types, labels, and joined output.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: ai, rufflet, choudhryfrompak

Merge Risk: ⚪ Minimal · up to 32745

The change appears ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: invoking the builder for every node when escaped output is generated.
Linked Issues check		Issue `#2174` requires per-node builder calls when escaping occurs. `SafeStringifier.escapeHTML` buffers the original chunks, escapes the combined CSS, splits the escaped text with `cutEscaped`, and res…
Out of Scope Changes check		The reviewed changes are limited to escaped stringification in `lib/stringify.js` and its focused regression coverage in `test/stringifier.test.js`. Document-root handling and precise chunk mapping su…

Full details: Linked Issues check

Explanation

Issue #2174 requires per-node builder calls when escaping occurs. SafeStringifier.escapeHTML buffers the original chunks, escapes the combined CSS, splits the escaped text with cutEscaped, and restores each chunk with its node and type. The focused test verifies rule and declaration chunks, start and end markers, and output equivalence with root.toString(). The implementation also preserves separate chunk handling when no escaping occurs.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to escaped stringification in lib/stringify.js and its focused regression coverage in test/stringifier.test.js. Document-root handling and precise chunk mapping support the stringifier behavior described by issue #2174. No unrelated product behavior is demonstrated.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

ai · 2026-10-18T19:36:27Z

Thanks. Released in 8.5.31

firefoxic · 2026-10-18T19:52:25Z

Thank you for accepting this fix!

Bumps `postcss` from 8.5.30 to 8.5.31, both in the root resolution and in the `@dnb/eufemia` dev dependency. 8.5.31 is a bug fix with no security fix ([release](https://github.com/postcss/postcss/releases/tag/8.5.31)). `npm diff` shows only two code changes: - `lib/stringify.js`: when `SafeStringifier` escapes `</style>` or `<!--` in CSS, it now builds a correct source map for every node. Before, it mapped only the start of the node (postcss/postcss#2175). - `lib/processor.js`: the version string. The escaped CSS text itself is unchanged, so the CSS we build cannot change. No other dependency changes. In the lockfile, only the `postcss` entry changes. **Verified locally** - I built all library CSS with `makeLibStyles` and `makeMainStyle` in production mode, so the `--isolated` variants are included. That is 500 files: 125 plain, 125 `.min.css` and 250 isolated. - The files are byte-identical on 8.5.30 and 8.5.31, both before and after rebasing onto the current `main`. - Two `main` builds also matched each other (control, 0 differing). - Old `main` against new `main` showed 24 differing files, from the style change in #10032 (positive control). So the comparison does see real changes. - `vitest run src/plugins scripts/prebuild` passes (297 tests). This covers the repo's own PostCSS plugins and the style build. `lint:styles` passes. - `yarn install --immutable`, `yarn dedupe --check`, `lint:resolutions` and the three `audit:ci` workspaces pass.

ai reviewed Oct 18, 2026

View reviewed changes

Comment thread lib/stringify.js Outdated

fixup! Call the builder with every node when the output is escaped

3274524

ai merged commit 6abd674 into postcss:main Oct 18, 2026
11 checks passed

firefoxic deleted the fix-builder-chunks-on-escape branch October 18, 2026 16:12

langz mentioned this pull request Oct 22, 2026

chore(deps-dev): bump postcss to 8.5.31 dnbexperience/eufemia#10123

Merged

Navigation Menu

Repository navigation

Call the builder with every node when the output is escaped - #2175

Call the builder with every node when the output is escaped#2175
ai merged 2 commits into
postcss:mainfrom
firefoxic:fix-builder-chunks-on-escape

firefoxic commented Oct 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

ai commented Oct 18, 2026

Uh oh!

firefoxic commented Oct 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

firefoxic commented Oct 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ai commented Oct 18, 2026

Uh oh!

firefoxic commented Oct 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

firefoxic commented Oct 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading