feat: strip template and style comments in parser and support legal comments#326
Open
mohamedmansour wants to merge 7 commits into
Open
feat: strip template and style comments in parser and support legal comments#326mohamedmansour wants to merge 7 commits into
mohamedmansour wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes CSR hydration “comment bleed” by stripping HTML comments from parser output and compiled template metadata, and adds a configurable CSS legal-comment preservation policy (inline default, none to strip all). The new --legal-comments option is wired through Rust core APIs, the CLI, Node bindings, and corresponding DESIGN/docs updates.
Changes:
- Strip HTML comment nodes during parsing so bindings/directives inside comments never produce fragments or hydration metadata.
- Strip CSS block/line comments during CSS parsing while optionally preserving “legal” comments (
inlinevsnone) end-to-end via a newLegalCommentsoption. - Propagate the new option through
webui(Rust),webui-cli, andwebui-node, with updated DESIGN and docs.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/guide/integrations/node.md | Documents legalComments option in Node build API. |
| docs/guide/concepts/css-tokens.md | Updates token guidance to reflect comment stripping. |
| docs/guide/cli/index.md | Adds --legal-comments to build/serve docs and explains comment handling. |
| docs/ai.md | Updates AI reference for CLI flag and token injection guidance. |
| DESIGN.md | Specifies LegalComments behavior and comment-handling contract. |
| crates/webui/src/lib.rs | Exposes LegalComments via BuildOptions and adds tests around CSS comment stripping/preservation. |
| crates/webui/README.md | Updates public Rust usage example and explains LegalComments. |
| crates/webui-parser/src/plugin/webui.rs | Ensures compiled-template metadata ignores bindings inside comments and strips comments in <style>. |
| crates/webui-parser/src/lib.rs | Implements HTML/CSS comment stripping in parser/component-template processing; threads LegalComments. |
| crates/webui-parser/src/css_parser.rs | Adds one-pass token+definition extraction with removable-comment stripping and comment-range helpers. |
| crates/webui-parser/src/component_registry.rs | Applies CSS comment policy during component CSS registration/token extraction. |
| crates/webui-parser/src/comment_policy.rs | Adds shared helpers for comment classification and range stripping. |
| crates/webui-parser/benches/parser_bench.rs | Adjusts benchmark templates to updated attribute patterns. |
| crates/webui-node/src/lib.rs | Adds legal_comments option parsing/wiring in N-API build path + tests. |
| crates/webui-cli/src/commands/serve.rs | Updates CLI tests/fixtures to include new build option field. |
| crates/webui-cli/src/commands/common.rs | Adds --legal-comments CLI argument and wires into BuildOptions. |
| crates/webui-cli/src/commands/build.rs | Updates CLI tests/fixtures to include new build option field. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
521f249 to
9ad838d
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Strips all comments in styles and templates but allows the preservation of legal comments. This fixes CSR hydration comment bleed from component templates while keeping the existing CSS signal-fragment escape hatch for dynamic CSS inside
<style>tags.What changed and why
HTML comments
HTML comments are authoring notes. Treating comment bodies as template syntax caused hidden
{{...}},@event, and?attrexamples to leak into CSR-created components as visible text or hydration metadata.CSS comments and CSS signal fragments
legal_comments/--legal-commentsisinline.noneto strip all non-signal comments, including legal comments./*{{...}}*/and/*{{{...}}}*/as valid CSS signal fragments in<style>tags.CSS comments need syntax-aware handling so token extraction ignores commented
var(...)usages and output does not keep authoring comments. CSS signal fragments are a deliberate WebUI escape hatch: dynamic CSS must be wrapped as a CSS comment so raw handlebars are not interpreted as normal CSS text.Parser and plugin architecture
HtmlParserand the WebUI compiled-template metadata generator.SSR/parser output and CSR metadata need the same comment semantics. The WebUI plugin has its own metadata compiler, so it still emits plugin-specific metadata, but it now shares the recognition rule with parser core to avoid drift.
Public API and CLI
LegalCommentsand madeParserOptions::try_new(...)take the legal-comment mode explicitly.legal_commentsto RustBuildOptionsand NodeJsBuildOptions.--legal-comments <inline|none>towebui buildandwebui serve.Comment preservation is build output behavior, so it needs to be explicit and consistent across Rust, Node, and CLI entry points. We intentionally did not keep a compatibility constructor so new parser options require callers to choose the policy.
Tests and benchmarks
:config/:datato normal attributes.The bug spans parser output and CSR metadata, so tests cover both paths. Benchmarks should continue measuring complex attribute parsing, not avoid it.
Micro-benchmarks
Compared
HEADagainstorigin/mainwith the samemicrosoft-webui-parserCriterion benchmark harness.-3.38%-3.45%21 / 210 / 21Selected mean-time deltas:
origin/mainHEADparser_parse_reuse/reuse/attributes_100-1.29%parser_parse_reuse/reuse/styles_40-1.41%parser_css_strategy/external_css-1.77%parser_css_strategy/inline_css-2.01%parser_realistic/parse/todo_app-4.97%parser_realistic/parse/dashboard-14.65%Validation
cargo test -p microsoft-webui-parser -p microsoft-webui-nodecd docs && pnpm buildcargo bench -p microsoft-webui-parser --bench parser_bench -- --testcargo xtask checkCloses #323