Skip to content

Commit e27dc29

Browse files
feat: validate compiles malformed HTML as an error, not a warning (#3099)
Once the validator fires, the emitted positional walk is guaranteed not to match the browser-built DOM — crashed or silently misplaced bindings, and desynced hydration under SSR — so warn-and-emit shipped certain breakage with the diagnostic buried in server logs. Both compilers now fail the build pointing at the offending JSX (code frame in Babel, line:col in the native compiler); validate: false remains the opt-out. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 3e3676b commit e27dc29

8 files changed

Lines changed: 145 additions & 66 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@solidjs/babel-plugin-jsx": minor
3+
"@solidjs/compiler": minor
4+
---
5+
6+
`validate` now fails the compile instead of warning when a template's markup would be restructured by the browser's HTML parser (#3099). Once the validator fires the emitted positional walk is guaranteed not to match the browser-built DOM (crashed or silently misplaced bindings; desynced hydration under SSR), so warn-and-emit shipped certain breakage with the diagnostic buried in server logs. Errors now point at the offending JSX (code frame in Babel, line:col in the native compiler). `validate: false` remains the opt-out.

packages/babel-plugin/src/dom/template.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,11 @@ function registerTemplate(path: NodePath, results: TransformResult) {
129129
templateWithClosingTags: results.templateWithClosingTags as string,
130130
isImportNode: results.isImportNode,
131131
isWrapped: results.isWrapped,
132-
renderer: "dom"
132+
renderer: "dom",
133+
// templates dedupe on markup, so the FIRST site carries the blame
134+
// for a validate failure (#3099) — good enough: every site with
135+
// this markup has the same problem
136+
path
133137
});
134138
}
135139
}

packages/babel-plugin/src/shared/postprocess.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,22 @@ export default (path: NodePath<t.Program>, state: PluginPass) => {
3232
if (typeof html === "string") {
3333
const result = isInvalidMarkup(html);
3434
if (result) {
35+
// A compile ERROR, not a warning (#3099): once the validator has
36+
// fired, the emitted template is guaranteed not to match its own
37+
// positional walk — the browser rebuilds the DOM, and the walk
38+
// binds against nodes that moved (crash or silent wrong-node
39+
// bindings; under SSR the restructuring desyncs hydration too).
40+
// Warn-and-emit put this diagnostic in server stdout while the
41+
// browser failed with an unrelated-looking runtime crash. The
42+
// error throws from the template's registration site, so
43+
// bundlers surface it at the right file and line. `validate:
44+
// false` remains the opt-out.
3545
const message =
36-
"\nThe HTML provided is malformed and will yield unexpected output when evaluated by a browser.\n";
37-
console.warn(message);
38-
console.warn("User HTML:\n", result.html);
39-
console.warn("Browser HTML:\n", result.browser);
40-
console.warn("Original HTML:\n", html);
41-
// throw path.buildCodeFrameError();
46+
"The HTML provided is malformed and will yield unexpected output when evaluated by a browser.\n" +
47+
`User HTML:\n ${result.html}\n` +
48+
`Browser HTML:\n ${result.browser}\n` +
49+
`Original HTML:\n ${html}`;
50+
throw (template.path ?? path).buildCodeFrameError(message);
4251
}
4352
}
4453
}

packages/babel-plugin/src/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ export interface TemplateRecord {
1414
isImportNode?: boolean;
1515
isWrapped?: boolean;
1616
renderer: RendererName;
17+
/** First registration site, so `validate` failures point at the JSX (#3099). */
18+
path?: NodePath;
1719
}
1820

1921
export interface ProgramScopeData {

packages/compiler/__tests__/validate-parity.test.js

Lines changed: 63 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,99 @@
1-
// Parity checks for the `validate` option's malformed-HTML warnings.
1+
// Parity checks for the `validate` option's malformed-HTML compile errors.
22
//
3-
// Babel warns through `console.warn`; Oxc's port warns from Rust directly to
4-
// process stderr, so both compiles run in child processes and the captured
5-
// stderr is compared — both whether a warning fires and its exact content.
3+
// Since #3099 `validate` is a hard compile error in both compilers (the
4+
// emitted walk is guaranteed broken once the browser re-parses the markup
5+
// differently). Both compiles run in child processes; the runners catch the
6+
// thrown error and print its message to stderr so the harness can compare
7+
// whether each compiler fired and that the DOM diff content matches.
68

79
const { spawnSync } = require("child_process");
810
const path = require("path");
911

1012
const compilerDir = path.resolve(__dirname, "..");
1113

1214
const cases = {
13-
pInDiv: { code: "const t = <p><div>bad</div></p>;", warns: true },
14-
nestedA: { code: "const t = <a><a>x</a></a>;", warns: true },
15-
tableNoTbody: { code: "const t = <table><tr><td>1</td></tr></table>;", warns: true },
16-
formInForm: { code: "const t = <form><form>x</form></form>;", warns: true },
17-
buttonInButton: { code: "const t = <button><button>x</button></button>;", warns: true },
18-
dynamicHole: { code: "const t = <p>{x()}<div>bad</div></p>;", warns: true },
15+
pInDiv: { code: "const t = <p><div>bad</div></p>;", throws: true },
16+
nestedA: { code: "const t = <a><a>x</a></a>;", throws: true },
17+
tableNoTbody: { code: "const t = <table><tr><td>1</td></tr></table>;", throws: true },
18+
formInForm: { code: "const t = <form><form>x</form></form>;", throws: true },
19+
buttonInButton: { code: "const t = <button><button>x</button></button>;", throws: true },
20+
dynamicHole: { code: "const t = <p>{x()}<div>bad</div></p>;", throws: true },
1921
hydratableMarkers: {
2022
code: "const t = <p>{x()}<div>bad</div></p>;",
2123
options: { hydratable: true },
22-
warns: true
24+
throws: true
2325
},
2426
// Table partials are wrapped in the right context before validation.
25-
tdPartial: { code: "const t = <td>cell</td>;", warns: false },
26-
trPartial: { code: "const t = <tr><td>c</td></tr>;", warns: false },
27-
colPartial: { code: "const t = <col />;", warns: false },
28-
theadPartial: { code: "const t = <thead><tr><th>h</th></tr></thead>;", warns: false },
29-
emptyTbody: { code: "const t = <tbody></tbody>;", warns: false },
27+
tdPartial: { code: "const t = <td>cell</td>;", throws: false },
28+
trPartial: { code: "const t = <tr><td>c</td></tr>;", throws: false },
29+
colPartial: { code: "const t = <col />;", throws: false },
30+
theadPartial: { code: "const t = <thead><tr><th>h</th></tr></thead>;", throws: false },
31+
emptyTbody: { code: "const t = <tbody></tbody>;", throws: false },
3032
// Escaped text must not be re-interpreted as markup.
31-
scriptEscape: { code: 'const t = <div>{"<script>a();</script>"}<b>ok</b></div>;', warns: false },
32-
liOrphan: { code: "const t = <li>item</li>;", warns: false },
33-
goodDiv: { code: "const t = <div><span>fine</span></div>;", warns: false },
34-
disabled: { code: "const t = <p><div>bad</div></p>;", options: { validate: false }, warns: false }
33+
scriptEscape: { code: 'const t = <div>{"<script>a();</script>"}<b>ok</b></div>;', throws: false },
34+
liOrphan: { code: "const t = <li>item</li>;", throws: false },
35+
goodDiv: { code: "const t = <div><span>fine</span></div>;", throws: false },
36+
disabled: {
37+
code: "const t = <p><div>bad</div></p>;",
38+
options: { validate: false },
39+
throws: false
40+
}
3541
};
3642

3743
const babelRunner = `
3844
const babel = require("@babel/core");
3945
const plugin = require("../babel-plugin");
40-
babel.transformSync(process.argv[1], {
41-
filename: "a.jsx",
42-
parserOpts: { plugins: ["jsx"] },
43-
plugins: [[plugin, JSON.parse(process.argv[2])]]
44-
});
46+
try {
47+
babel.transformSync(process.argv[1], {
48+
filename: "a.jsx",
49+
parserOpts: { plugins: ["jsx"] },
50+
plugins: [[plugin, JSON.parse(process.argv[2])]]
51+
});
52+
} catch (error) {
53+
console.error(error.message);
54+
process.exitCode = 42;
55+
}
4556
`;
4657

4758
const oxcRunner = `
4859
const { transform } = require("./index.js");
49-
transform(process.argv[1], { filename: "a.jsx", ...JSON.parse(process.argv[2]) });
60+
try {
61+
transform(process.argv[1], { filename: "a.jsx", ...JSON.parse(process.argv[2]) });
62+
} catch (error) {
63+
console.error(error.message);
64+
process.exitCode = 42;
65+
}
5066
`;
5167

52-
function stderrOf(runner, code, options) {
68+
function runCompile(runner, code, options) {
5369
const result = spawnSync("node", ["-e", runner, code, JSON.stringify(options)], {
5470
cwd: compilerDir,
5571
encoding: "utf8"
5672
});
57-
expect(result.status).toBe(0);
58-
return result.stderr;
73+
expect([0, 42]).toContain(result.status);
74+
return { threw: result.status === 42, stderr: result.stderr };
75+
}
76+
77+
// The compilers format locations differently (Babel appends a code frame;
78+
// Oxc embeds line:col), so parity is asserted on the DOM diff itself.
79+
function domDiff(stderr) {
80+
const match = stderr.match(/User HTML:\n[^\n]*\n\s*Browser HTML:\n[^\n]*/);
81+
expect(match).not.toBeNull();
82+
return match[0].replace(/\n\s+/g, "\n ");
5983
}
6084

61-
describe("validate warning parity", () => {
62-
for (const [name, { code, options = {}, warns }] of Object.entries(cases)) {
85+
describe("validate error parity", () => {
86+
for (const [name, { code, options = {}, throws }] of Object.entries(cases)) {
6387
test(name, () => {
6488
const fullOptions = { moduleName: "r-dom", ...options };
65-
const babelErr = stderrOf(babelRunner, code, fullOptions);
66-
const oxcErr = stderrOf(oxcRunner, code, fullOptions);
67-
expect(babelErr.includes("malformed")).toBe(warns);
68-
expect(oxcErr.includes("malformed")).toBe(warns);
69-
if (warns) {
70-
expect(oxcErr.trim()).toBe(babelErr.trim());
89+
const babel = runCompile(babelRunner, code, fullOptions);
90+
const oxc = runCompile(oxcRunner, code, fullOptions);
91+
expect(babel.threw).toBe(throws);
92+
expect(oxc.threw).toBe(throws);
93+
expect(babel.stderr.includes("malformed")).toBe(throws);
94+
expect(oxc.stderr.includes("malformed")).toBe(throws);
95+
if (throws) {
96+
expect(domDiff(oxc.stderr)).toBe(domDiff(babel.stderr));
7197
}
7298
});
7399
}

packages/compiler/src/dom/element.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ impl<'a, 'source> AstDomTransform<'a, 'source> {
412412
let template_id = if skip_template {
413413
None
414414
} else {
415-
Some(self.template_id_with_options(template, template_flag))
415+
Some(self.template_id_with_options(template, template_flag, element.span))
416416
};
417417
let has_hydratable_event = self.has_hydratable_event;
418418
self.has_hydratable_event = saved_hydratable_event;

packages/compiler/src/dom/template.rs

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ pub(crate) struct DomTemplate {
5959
pub(crate) flag: Option<u8>,
6060
/// Generated `_tmpl$N` local (collision-checked against source names).
6161
pub(crate) name: String,
62+
/// First registration site (templates dedupe on markup), so a `validate`
63+
/// failure names the JSX element's source location (#3099).
64+
pub(crate) span: Span,
6265
}
6366

6467
/// A template under construction: the emitted markup (`html`, with omitted
@@ -93,6 +96,15 @@ pub(crate) struct InsertMarker<'a> {
9396
pub(crate) initial: Option<Expression<'a>>,
9497
}
9598

99+
/// 1-based line and column for a byte offset, for diagnostics.
100+
fn line_and_column(source: &str, offset: u32) -> (usize, usize) {
101+
let offset = (offset as usize).min(source.len());
102+
let prefix = &source[..offset];
103+
let line = prefix.bytes().filter(|byte| *byte == b'\n').count() + 1;
104+
let column = prefix.rfind('\n').map_or(offset, |at| offset - at - 1) + 1;
105+
(line, column)
106+
}
107+
96108
impl DomTemplateState {
97109
pub(crate) fn new() -> Self {
98110
Self {
@@ -240,22 +252,31 @@ impl<'a> AstDomTransform<'a, '_> {
240252
// against the top-level module, not the renderer config.
241253
statements.push(self.import_wrapper_helper(built_in, &format!("_${built_in}")));
242254
}
243-
// Babel's postprocess `validate` pass: warn (stderr, like
244-
// `console.warn`) when a browser would re-parse a template's markup
245-
// differently. Only DOM templates carry the closing-tags variant —
246-
// Babel skips SSR templates for the same reason (theirs are AST
247-
// nodes, not strings).
255+
// Babel's postprocess `validate` pass: a compile ERROR (#3099) when a
256+
// browser would re-parse a template's markup differently. Once the
257+
// validator has fired, the emitted template is guaranteed not to
258+
// match its own positional walk — the browser rebuilds the DOM and
259+
// the walk binds against nodes that moved (crash or silent wrong-node
260+
// bindings; under SSR the restructuring desyncs hydration too), so
261+
// warn-and-emit shipped certain breakage with the explanation buried
262+
// in server stderr. The message carries the registration site's
263+
// line:col so bundler overlays land on the JSX. `validate: false`
264+
// remains the opt-out. Only DOM templates carry the closing-tags
265+
// variant — Babel skips SSR templates for the same reason (theirs
266+
// are AST nodes, not strings).
248267
if self.validate {
249268
for template in &self.template_state.templates {
250269
if let Some(result) =
251270
crate::shared::validate::is_invalid_markup(&template.closed_html)
252271
{
253-
eprintln!(
254-
"\nThe HTML provided is malformed and will yield unexpected output when evaluated by a browser.\n"
255-
);
256-
eprintln!("User HTML:\n {}", result.html);
257-
eprintln!("Browser HTML:\n {}", result.browser);
258-
eprintln!("Original HTML:\n {}", template.closed_html);
272+
let (line, column) = line_and_column(self.source, template.span.start);
273+
return Err(crate::error::CompileError::transform(format!(
274+
"The HTML provided is malformed and will yield unexpected output when evaluated by a browser. ({line}:{column})\n\
275+
User HTML:\n {}\n\
276+
Browser HTML:\n {}\n\
277+
Original HTML:\n {}",
278+
result.html, result.browser, template.closed_html
279+
)));
259280
}
260281
}
261282
}
@@ -277,6 +298,7 @@ impl<'a> AstDomTransform<'a, '_> {
277298
&mut self,
278299
template: TemplateHtml,
279300
flag: Option<u8>,
301+
span: Span,
280302
) -> String {
281303
self.template_state.uses_template = true;
282304
// Templates dedupe on markup alone (the first registration's flag
@@ -298,6 +320,7 @@ impl<'a> AstDomTransform<'a, '_> {
298320
closed_html: template.closed,
299321
flag,
300322
name: name.clone(),
323+
span,
301324
});
302325
name
303326
}

packages/web/test/hydration/diagnostics.spec.tsx

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -392,18 +392,25 @@ describe("Phase 2: Walk validation", () => {
392392
}
393393
});
394394

395-
test("getFirstChild warns on browser-corrected structure (tbody insertion)", () => {
395+
// The historical poster child for this warn — JSX `<table><tr>` whose
396+
// missing tbody the browser inserts during parse — is a compile error
397+
// since #3099, so the walk mismatch is staged with valid JSX against
398+
// server DOM whose table section differs. The runtime diagnostic still
399+
// matters for `validate: false` users and handwritten server markup.
400+
test("getFirstChild warns on mismatched table structure", () => {
396401
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
397402
const [text] = createSignal("Cell");
398403

399-
container.innerHTML = '<table _hk="0"><tbody><tr><td>Cell</td></tr></tbody></table>';
404+
container.innerHTML = '<table _hk="0"><thead><tr><td>Cell</td></tr></thead></table>';
400405

401406
dispose = hydrate(
402407
() => (
403408
<table>
404-
<tr>
405-
<td>{text()}</td>
406-
</tr>
409+
<tbody>
410+
<tr>
411+
<td>{text()}</td>
412+
</tr>
413+
</tbody>
407414
</table>
408415
),
409416
container
@@ -413,7 +420,7 @@ describe("Phase 2: Walk validation", () => {
413420
c => typeof c[0] === "string" && c[0].includes("Hydration structure mismatch")
414421
);
415422
expect(structureWarns.length).toBeGreaterThanOrEqual(1);
416-
expect(structureWarns[0][0]).toContain("expected <tr>");
423+
expect(structureWarns[0][0]).toContain("expected <tbody>");
417424
warn.mockRestore();
418425
});
419426

@@ -496,14 +503,16 @@ describe("Phase 2: Walk validation", () => {
496503
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
497504
const [text] = createSignal("Cell");
498505

499-
container.innerHTML = '<table _hk="0"><tbody><tr><td>Cell</td></tr></tbody></table>';
506+
container.innerHTML = '<table _hk="0"><thead><tr><td>Cell</td></tr></thead></table>';
500507

501508
dispose = hydrate(
502509
() => (
503510
<table>
504-
<tr>
505-
<td>{text()}</td>
506-
</tr>
511+
<tbody>
512+
<tr>
513+
<td>{text()}</td>
514+
</tr>
515+
</tbody>
507516
</table>
508517
),
509518
container

0 commit comments

Comments
 (0)