From 1a08f9bdacf8f05c7987a3bcc89a331002979df7 Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 7 Aug 2026 14:19:30 +0530 Subject: [PATCH 01/13] feat: make a bound form submitter carry its own submission A ` + +`; +``` + +One consequence worth knowing rather than discovering: no `formaction` url is emitted (an empty one is an HTML conformance error), so the submission targets whatever the FORM targets. A form declaring its own `action="/x"` sends its buttons to `/x`, which is native precedence and still runs the action, because the identity travels in the body. Leaving the form's `action` off, the ordinary shape, keeps the submission on the current page. + +The submitter must be a ``, + // #1307: a bound submitter carries its whole submission, so none of these + // depend on the form around it. Each moved here from a refusal table, and + // each must render IDENTICALLY on both renderers, which is what proves the + // two inject the same `formmethod` / `formenctype` pair in the same order. + 'submitter with no form at all': () => html``, + 'submitter inside an unbound form': () => html`
`, + 'submitter inside a form with no method at all': () => html`
`, + 'submitter inside a method=get form': () => html`
`, + // The author's own value wins; only the attribute they did NOT supply is + // injected. Both renderers make that call through `resolveBoundSubmitterAttrs`. + 'bound submitter supplying its own formmethod': () => html``, + 'bound submitter supplying its own formenctype': () => html``, + // #1307 reverses #1207's Part B: a PLAIN button's own override is a legal + // native instruction, so both renderers leave it exactly as written. + 'plain submitter formmethod=get inside a bound form': () => html`
`, + 'plain submitter formenctype=text/plain inside a bound form': () => html`
`, }; for (const [name, tpl] of Object.entries(ACCEPTS)) { @@ -434,12 +450,6 @@ suite('SSR/client parity: form actions (#1155)', () => { 'quoted action hole is a stringify': [() => html`
`, /interpolated into/], 'array-wrapped action': [() => html`
`, /interpolated into/], 'action off a form': [() => html`
`, /interpolated into/], - // #1207: the submitter's form is RIGHT THERE in the same template, so both - // renderers can see it is unbound and both must say so. - 'submitter inside an unbound form': [ - () => html`
`, - /requires the enclosing
to also be bound/, - ], 'submitter that is not a submit control': [ () => html`
`, /submitter control/, @@ -448,16 +458,30 @@ suite('SSR/client parity: form actions (#1155)', () => { () => html`
`, /already carries a "name" attribute/, ], - // #1207 Part B: a submitter that binds nothing, whose own formenctype would - // still defeat the bound form it sits in. - 'unparseable submitter enctype': [ - () => html`
`, + // #1307: same-element contradictions on a BOUND submitter. These replace + // #1207's Part B rows, which asked about the button's NEIGHBOUR and are now + // in ACCEPTS. Here the author bound an action to this very button and then + // told the same button to submit in a way that action could never read. + 'unparseable enctype on a BOUND submitter': [ + () => html``, /formenctype=/, ], - 'non-POST submitter method': [ - () => html`
`, + 'non-POST method on a BOUND submitter': [ + () => html``, /formmethod=/, ], + 'dialog method on a BOUND submitter': [ + () => html``, + /formmethod="dialog"/, + ], + 'padded formmethod on a BOUND submitter': [ + () => html``, + /cannot work/, + ], + 'prop binding on a BOUND submitter': [ + () => html``, + /reflected IDL attribute/, + ], 'a function that is not an action': [() => html`
{}}>
`, /is not a server action/], 'prop binding on a bound form': [() => html`
`, /also binds \./], 'two action holes': [() => html`
`, /two action=/], @@ -488,41 +512,25 @@ suite('SSR/client parity: form actions (#1155)', () => { } /** - * The ONE asymmetry in #1207, stated here rather than left to be discovered. + * There is deliberately NO third table here any more. * - * "Is my enclosing form bound" is a question SSR always answers and the client - * sometimes cannot. SSR reads a linear byte stream, so an open `
` either - * bound an action or did not. The client reconciles a template whose root may - * be a DocumentFragment that is not in the tree yet, and a submitter with no - * form ABOVE IT IN ITS OWN TEMPLATE is genuinely ambiguous there: it looks - * identical whether it is a stray button or a list row about to be inserted - * into a bound form by its parent. Those two are indistinguishable at that - * moment, and the list row is the shape the whole feature exists for. + * #1207 had one, holding a single row: a `formaction=${fn}` submitter with no + * enclosing form. "Is my enclosing form bound" was a question SSR always + * answered and the client sometimes could not, because the client reconciles + * a template whose root may be a DocumentFragment not yet in the tree, where a + * stray button and a list row about to be inserted into a bound form look + * identical. So SSR refused and the client deferred, and that asymmetry was + * documented here as permanent. * - * So the client BINDS when it cannot tell, and SSR refuses. The asymmetry is - * safe in that direction and only in that direction: the server renders every - * page, so a genuinely form-less submitter is still refused loudly before - * anything ships. The reverse (client refuses, SSR accepts) is what would - * render a page on the server and crash it on hydration. + * #1307 removed the question rather than the asymmetry. A bound submitter now + * carries its own `formmethod` and `formenctype`, so no renderer needs to know + * what encloses it, and the row moved to ACCEPTS where both renderers must now + * produce identical bytes for it. + * + * Keep this table absent. A new "SSR refuses, client defers" entry is a signal + * that a cross-element rule has crept back in, which is exactly the shape both + * issues concluded cannot be enforced honestly. */ - const SSR_ONLY_REFUSES = { - 'submitter with no form at all': [ - () => html``, - /requires the enclosing to also be bound/, - ], - }; - - for (const [name, [tpl, pattern]] of Object.entries(SSR_ONLY_REFUSES)) { - test(`SSR refuses, client defers: ${name}`, async () => { - const r = await bothWays(tpl); - assert.ok(r.ssrErr, `SSR must refuse: ${name}`); - assert.ok(pattern.test(r.ssrErr), `SSR reason (${r.ssrErr})`); - assert.ok(!r.clientErr, `client must NOT refuse what it cannot know: ${name} (${r.clientErr})`); - // Deferring the question must not defer the LEAK guard: the client still - // binds an identity rather than stringifying the function. - assert.ok(!/PARITY_SECRET/.test(String(r.ssrErr) + String(r.client)), 'no source on either path'); - }); - } test('the identity field is submitted, which is what all of this is for', () => { // The end state, read the way a browser reads it: `new FormData(form)` is diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index dc10c4662..9b03a4926 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -152,16 +152,19 @@ test('mixed hole action="/x/${fn}" throws', async () => { }); test('formaction=${fn} on a submit button inside an unbound form throws', async () => { + // The enclosing form no longer decides anything (#1307). What still refuses is + // the LEAK guard: `leaky` is not a registered action, so it has no identity to + // bind and stringifying it would write the function's source into the HTML. await assert.rejects( () => renderToString(html`
`, { ssr: true }), - /requires the enclosing
to also be bound/, + NOT_AN_ACTION, ); }); test('camelCase formAction=${fn} throws on an unbound form (React spells it this way)', async () => { await assert.rejects( () => renderToString(html`
`, { ssr: true }), - /requires the enclosing
to also be bound/, + NOT_AN_ACTION, ); await assert.rejects( () => renderToString(html`
`, { ssr: true }), @@ -188,7 +191,7 @@ test('a quoted mixed-case Action="${fn}" throws (sigil strip and case-fold compo test('the streaming renderer folds case too', async () => { await assert.rejects( () => drain(renderToStream(html``, { ssr: false })), - /requires the enclosing
to also be bound/, + NOT_AN_ACTION, ); }); @@ -273,10 +276,10 @@ test('the streaming renderer refuses a mixed hole', async () => { ); }); -test('the streaming renderer refuses formaction on an unbound button', async () => { +test('the streaming renderer refuses a formaction that is not an action', async () => { await assert.rejects( () => drain(renderToStream(html``, { ssr: false })), - /requires the enclosing to also be bound/, + NOT_AN_ACTION, ); await assert.rejects( () => drain(renderToStream(html`
`, { ssr: false })), diff --git a/packages/core/test/rendering/form-action-binding-client.test.js b/packages/core/test/rendering/form-action-binding-client.test.js index 5bab41881..be2ecac8e 100644 --- a/packages/core/test/rendering/form-action-binding-client.test.js +++ b/packages/core/test/rendering/form-action-binding-client.test.js @@ -646,35 +646,52 @@ test('a detached submitter still refuses what the template alone decides', () => ); }); -test('a submitter whose enclosing form is resolvable and UNBOUND is still refused', () => { - // The skip is narrow: it applies when there is no form to ask, not when the - // answer is no. An inline submitter can always reach its form. +test('a submitter whose enclosing form is UNBOUND now binds, carrying its own submission', () => { + // The client used to ask "is my enclosing form bound" and refuse when it could + // reach the form and the answer was no. #1307 removed the question: the button + // supplies `formmethod` / `formenctype` itself, so the form is irrelevant. const rowAction = HOISTED(); const host = document.createElement('div'); - assert.throws( - () => render(html`
`, host), - /requires the enclosing
to also be bound/, - ); + render(html`
`, host); + const btn = host.querySelector('button'); + assert.equal(btn.getAttribute('name'), '__webjs_action'); + assert.equal(btn.getAttribute('formmethod'), 'post'); + assert.equal(btn.getAttribute('formenctype'), 'multipart/form-data'); + assert.equal(btn.hasAttribute('formaction'), false, 'no url is emitted'); }); // --------------------------------------------------------------------------- -// #1207 Part B on the client, so a component-only page (never SSR'd) gets the -// same answer the server would have given. +// The same-element rule on the client, so a component-only page (never SSR'd) +// gets the same answer the server would have given. // --------------------------------------------------------------------------- -test('the client refuses an unparseable submitter enctype inside a bound form', () => { - const formAction = HOISTED(); +test('the client refuses a BOUND submitter contradicting its own binding', () => { + const rowAction = HOISTED(); const host = document.createElement('div'); assert.throws( - () => render(html`
`, host), + () => render(html``, host), /formenctype=/, ); assert.throws( - () => render(html`
`, host), + () => render(html``, host), /formmethod=/, ); }); +test('the client leaves a PLAIN submitter\'s own override alone, matching SSR', () => { + // #1207's Part B refused these on both renderers. #1307 allows them on both, + // which is the half that keeps the two in step. + const formAction = HOISTED(); + const host = document.createElement('div'); + render( + html`
`, + host, + ); + const [a, b] = host.querySelectorAll('button'); + assert.equal(a.getAttribute('formenctype'), 'text/plain'); + assert.equal(b.getAttribute('formmethod'), 'get'); +}); + test('the client leaves dialog and retargeted submitters alone', () => { const formAction = HOISTED(); const host = document.createElement('div'); diff --git a/packages/core/test/rendering/form-action-binding.test.js b/packages/core/test/rendering/form-action-binding.test.js index 2d097fa3c..b4a7f9643 100644 --- a/packages/core/test/rendering/form-action-binding.test.js +++ b/packages/core/test/rendering/form-action-binding.test.js @@ -258,10 +258,10 @@ test('binding is scoped to
and bound submitters: non-action shapes refuse assert.match(msg, /function was interpolated into/, 'refused as a stringify, not bound'); assert.doesNotMatch(msg, /SECRET/); } - // Standalone submitter outside a bound form throws the form-binding requirement error: - let unboundMsg = ''; - try { await renderToString(html``, { ssr: true }); } catch (e) { unboundMsg = String(e.message); } - assert.match(unboundMsg, /requires the enclosing to also be bound/); + // A standalone submitter is NOT a non-action shape. Since #1307 it binds and + // carries its own submission, so it belongs in neither refusal above. + const standalone = await renderToString(html``, { ssr: true }); + assert.match(standalone, /name="__webjs_action"/); }); test('formaction=${fn} on submitter inside a bound form emits submitter action identity', async () => { @@ -270,7 +270,51 @@ test('formaction=${fn} on submitter inside a bound form emits submitter action i html`
`, { ssr: true } ); - assert.match(out, /`, { ssr: true }); + assert.match(bare, /formmethod="post"/, 'the button supplies its own method'); + assert.match(bare, /formenctype="multipart\/form-data"/, 'and its own enctype'); + assert.match(bare, /name="__webjs_action"/); + // No enclosing form at all, and a form that explicitly declares GET. The + // button overrides both, exactly as native HTML says a submitter does. + for (const tpl of [ + html``, + html`
`, + ]) { + const out = await renderToString(tpl, { ssr: true }); + assert.match(out, /formmethod="post"/); + assert.match(out, /formenctype="multipart\/form-data"/); + } + // COUNTERFACTUAL: delete the two injections in `bindSubmitterStartTag` and + // every assertion above fails, because the button falls back to whatever the + // enclosing form declares, which here is a GET carrying no body. +}); + +test('the author\'s own formmethod / formenctype wins; only the missing one is injected', async () => { + withResolver(); + const ownMethod = await renderToString( + html``, { ssr: true }); + assert.equal(ownMethod.match(/formmethod=/g).length, 1, 'not duplicated'); + assert.match(ownMethod, /formenctype="multipart\/form-data"/, 'the missing one is still supplied'); + + const ownEnctype = await renderToString( + html``, + { ssr: true }, + ); + assert.equal(ownEnctype.match(/formenctype=/g).length, 1); + assert.match(ownEnctype, /formenctype="application\/x-www-form-urlencoded"/, 'the author\'s value survives'); + assert.match(ownEnctype, /formmethod="post"/); }); test('a bound form nested among siblings does not disturb them', async () => { @@ -380,10 +424,10 @@ test('formaction=${fn} submitter refusals: name attribute, input type=image, unp }); test('a formaction binding on is refused for its label', async () => { - // `` IS a submitter, so Part B still judges it, but the - // identity has to occupy `value`, which on this control is also the visible - // caption. Binding would render a button captioned with the action id, and - // the only fix (`value="Publish"`) is the channel the identity needs. + // `` IS a submitter, but the identity has to occupy + // `value`, which on this control is also the visible caption. Binding would + // render a button captioned with the action id, and the only fix + // (`value="Publish"`) is the channel the identity needs. withResolver(); await assert.rejects( () => renderToString( @@ -392,13 +436,15 @@ test('a formaction binding on is refused for its label', a ), /also its visible label/, ); - // Part B still reaches it, so the control is not simply ignored. + // The label refusal fires FIRST, before any submission attribute is looked + // at, so a bound `` reports the label conflict whatever + // else it carries. await assert.rejects( () => renderToString( - html`
`, + html`
`, { ssr: true }, ), - /formmethod=/, + /also its visible label/, ); // And a plain labelled one renders untouched. const ok = await renderToString( @@ -459,7 +505,7 @@ test('formaction submitters work when rendered by a nested template', async () = withResolver(); const buttons = () => html``; const out = await renderToString(html`
${buttons()}
`, { ssr: true }); - assert.match(out, /`, html`
`, html`
`, ]) { - await assert.rejects(() => renderToString(tpl, { ssr: true }), /formenctype=/); + const out = await renderToString(tpl, { ssr: true }); + assert.match(out, /formenctype="(text\/plain|TEXT\/PLAIN)"/, 'left exactly as written'); } }); -test('a non-POST formmethod on a plain submitter inside a bound form is refused', async () => { +test('a PLAIN submitter\'s own formmethod inside a bound form renders untouched', async () => { withResolver(); for (const tpl of [ html`
`, html`
`, html`
`, + html`
`, ]) { - await assert.rejects(() => renderToString(tpl, { ssr: true }), /formmethod=/); + const out = await renderToString(tpl, { ssr: true }); + assert.match(out, /formmethod=/, 'left exactly as written'); } }); -test('a padded formmethod is refused, matching the form-level untrimmed rule', async () => { +test('a padded formmethod on a BOUND submitter is refused, matching the form-level untrimmed rule', async () => { // `formmethod` is an enumerated attribute matched against exact keywords with // no whitespace stripping, so `" post "` falls to the invalid-value default // and the button submits as a GET. Trimming here would accept it and ship the - // silently-posts-nowhere submitter the refusal exists to prevent. + // silently-posts-nowhere submitter the refusal exists to prevent. Scoped to a + // BOUND submitter now: on a plain one the author owns the consequence. withResolver(); await assert.rejects( () => renderToString( - html`
`, + html``, { ssr: true }, ), /formmethod=" post "/, ); }); +test('a BOUND submitter contradicting its own binding is still refused', async () => { + // The surviving half of Part B, and the whole of the new rule: the author + // bound an action to THIS button and then told THIS button to submit in a way + // that action could never read. + withResolver(); + for (const [tpl, pattern] of [ + [html``, /formmethod=/], + [html``, /formmethod=/], + [html``, /formenctype=/], + ]) { + await assert.rejects(() => renderToString(tpl, { ssr: true }), pattern); + } +}); + test('parseable submitter enctypes stay fully supported', async () => { - // Part B refuses VALUES that cannot work, never the attribute itself. + // The rule refuses VALUES that cannot work, never the attribute itself. withResolver(); for (const enc of ['multipart/form-data', 'application/x-www-form-urlencoded']) { const out = await renderToString( - html`
`, + html`
`, { ssr: true }, ); assert.match(out, new RegExp(`formenctype="${enc.replace(/[/]/g, '\\/')}"`)); @@ -572,9 +644,9 @@ test('a submitter retargeted by a static formaction keeps its own method', async assert.match(out, /formmethod="get"/); }); -test('Part B ignores controls that do not submit', async () => { - // `formmethod` / `formenctype` are inert on anything that is not a submitter, - // so flagging them there would be a false positive on valid markup. +test('formmethod / formenctype on a non-submitting control are inert and untouched', async () => { + // Inert on anything that is not a submitter, so touching them there would be a + // false positive on valid markup. withResolver(); const out = await renderToString( html`
`, @@ -583,8 +655,7 @@ test('Part B ignores controls that do not submit', async () => { assert.match(out, /name="q"/); }); -test('Part B applies only INSIDE a bound form', async () => { - // An ordinary hand-written form is not this module's business. +test('an ordinary hand-written form is not this module\'s business', async () => { withResolver(); const out = await renderToString( html`
`, @@ -593,9 +664,10 @@ test('Part B applies only INSIDE a bound form', async () => { assert.match(out, /formenctype="text\/plain"/); }); -test('the bound-form scope closes at ', async () => { - // `insideBoundForm` is scoped by the tag stream, so a submitter written after - // the bound form has closed is outside it and judged by nothing. +test('a plain submitter after a bound form closes is untouched', async () => { + // There is no enclosing-form scope left to leak (#1307 deleted it), so this + // pins the absence: a plain button anywhere keeps whatever it was written + // with, before or after any form. withResolver(); const out = await renderToString( html`
`, @@ -604,35 +676,37 @@ test('the bound-form scope closes at ', async () => { assert.match(out, /formmethod="get"/); }); -test('Part B reaches a submitter arriving through a nested template', async () => { - // The flag is threaded into nested renders, so a button rendered by a child - // template inside a bound form is judged exactly like an inline one. +test('a plain submitter arriving through a nested template is untouched too', async () => { + // Nothing is threaded into nested renders any more, which is the point: the + // verdict does not depend on how the button reached the page. withResolver(); const row = () => html``; - await assert.rejects( - () => renderToString(html`
${row()}
`, { ssr: true }), - /formenctype=/, - ); + const out = await renderToString(html`
${row()}
`, { ssr: true }); + assert.match(out, /formenctype="text\/plain"/); }); -test('the streaming machine applies Part B identically', async () => { +test('the streaming machine judges a BOUND submitter identically', async () => { // `streamTemplate` is a SEPARATE state machine that inherits nothing, and // #1154 already shipped a guard in one machine and not the other once. withResolver(); await assert.rejects( () => drain(renderToStream( - html`
`, + html``, { ssr: false }, )), /formenctype=/, ); await assert.rejects( () => drain(renderToStream( - html`
`, + html``, { ssr: false }, )), /formmethod=/, ); + // And INJECTS identically, which is the half a refusal-only test would miss. + const bound = await drain(renderToStream( + html`
`, { ssr: false })); + assert.match(bound, /formmethod="post" formenctype="multipart\/form-data"/); const ok = await drain(renderToStream( html`
`, { ssr: false }, @@ -664,50 +738,37 @@ test('a submitter rendered by a component inside a bound form binds', async () = html`
`, { ssr: true, dev: false }, ); - assert.match(out, /`, { ssr: true }), - /requires the enclosing
to also be bound/, - ); - await assert.rejects( - () => renderToString( - html`
`, - { ssr: true }, - ), - /requires the enclosing
to also be bound/, - 'and the scope really does close at
', - ); -}); - -test('an UNBOUND form is refused, which is a different answer from cannot-tell', async () => { +test('a form-less submitter BINDS now, which is the whole of #1307', async () => { + // This block used to prove the opposite. SSR distinguished cannot-tell from + // conclusively-none and refused the latter, because a submitter could not + // supply `method="post"` for itself. It can now, so every shape below binds. withResolver(); - await assert.rejects( - () => renderToString( - html`
`, - { ssr: true }, - ), - /requires the enclosing
to also be bound/, - ); + for (const tpl of [ + html``, + html`
`, + html`
`, + html`
`, + ]) { + const out = await renderToString(tpl, { ssr: true }); + assert.match(out, /formmethod="post"/); + assert.match(out, /formenctype="multipart\/form-data"/); + } }); -test("the 'unbound' state is what refuses inside a COMPONENT's own form", async () => { - // The test above cannot observe the 'unbound' transition: a top-level scan - // starts at 'none', so that template is refused either way, and deleting the - // transition left the whole suite green. 'unbound' differs from 'none' only - // under an 'unknown' seed, which is the component pass. +test("a component's own unbound form no longer swallows its submitter", async () => { + // The #1307 failure in its purest form. A component renders in a SEPARATE SSR + // pass with no view of the host page, so the old scan seeded 'unknown' and + // could not judge the enclosing form. Where it thought it COULD judge (the + // component's own form), it refused, and because a component's SSR error is + // ISOLATED the button silently vanished from a page that still returned 200. // - // Without it, a component's own GET-defaulting form would happily bind a - // submitter inside it, which is the silently-posts-nowhere shape the guard - // exists to prevent. The component's SSR error is isolated, so the proof is - // that the component renders EMPTY rather than emitting the identity. + // Both halves are fixed by the same change: nothing is judged, so nothing is + // isolated away. withResolver(); const { WebComponent } = await import('../../src/component.js'); class OwnUnbound extends WebComponent({}) { @@ -718,16 +779,12 @@ test("the 'unbound' state is what refuses inside a COMPONENT's own form", async html`
`, { ssr: true, dev: false }, ); - assert.equal((out.match(/name="__webjs_action"/g) || []).length, 1, - "only the page form's identity is emitted; the component's submitter never bound"); - assert.ok(!out.includes(' { - // `` used to hard-reset the scope to 'none', which asserted a fact the - // component scan cannot know: closing a form of its own teaches it nothing - // about the host page. A bound submitter written after it was then refused - // and, being isolated, vanished from a 200. +test('a component that closes its own form still binds a later submitter', async () => { withResolver(); const { WebComponent } = await import('../../src/component.js'); class ClosesOwnForm extends WebComponent({}) { @@ -740,16 +797,20 @@ test("a component that CLOSES its own form keeps deferring, rather than downgrad html`
`, { ssr: true, dev: false }, ); - assert.match(out, /`), })}`); assert.match(shell, /

loading<\/p><\/webjs-boundary>/); - assert.equal(pending[0].formScope, 'bound', 'the shell records the scope'); - assert.match(parts[0], /`) })}`, html`

${Suspense({ fallback: html`

l

`, children: Promise.resolve(html``) })}
`, ]) { - await assert.rejects(() => drainSuspense(shell), /requires the enclosing
to also be bound/); + const { parts } = await drainSuspense(shell); + assert.match(parts[0], /formmethod="post" formenctype="multipart\/form-data"/); } }); @@ -801,11 +864,17 @@ test('a Suspense boundary outside a bound form still refuses a submitter', async test('a reflected .prop on a submitter is refused, in both machines', async () => { withResolver(); + // Every row binds its own action (#1307). The rule is same-element: a `.prop` + // on a button the author bound is refused, because SSR drops it and the + // browser reflects it, so the SAME button would submit differently with JS + // than without. On a PLAIN button it is an ordinary native property and is + // left alone, which is the row below this test. const refused = [ - html`
`, - html`
`, - html`
`, - html`
`, + html``, + html``, + html``, + html``, + html``, ]; for (const tpl of refused) { await assert.rejects(() => renderToString(tpl, { ssr: true }), /reflected IDL attribute/); @@ -839,23 +908,29 @@ test('an empty author name after the hole is refused, not shipped as a duplicate } }); -test('a .formAction prop is refused on a submitter that binds nothing', async () => { - // The narrowing that spares `.name` / `.value` on a non-binding submitter must - // NOT spare `.formAction`. SSR drops the prop, so with JS off the button - // submits to the page and runs the bound action; a browser reflects it, so - // with JS on the button posts elsewhere and the action never runs. That is the - // works-one-way-only shape, and it is why the STATIC `formaction="/url"` stays - // fine: both renderers see that one and agree. +test('a .prop on a submitter that binds NOTHING is an ordinary native property', async () => { + // #1307 narrowed this. `.formMethod` / `.formEnctype` / `.formAction` on a + // button that binds no action used to be refused, on the grounds that they + // could defeat the enclosing form's binding. That is a rule about the + // author's OTHER element, and the ordinary native-property behaviour (SSR + // drops a `.prop`, the browser reflects it) is what this codebase already + // accepts everywhere else, including for an unbound form's own `.method`. withResolver(); - const tpl = html`
`; - await assert.rejects(() => renderToString(tpl, { ssr: true }), /reflected IDL attribute/); - await assert.rejects(() => drain(renderToStream(tpl, { ssr: false })), /reflected IDL attribute/); + for (const tpl of [ + html`
`, + html`
`, + html`
`, + ]) { + const out = await renderToString(tpl, { ssr: true }); + assert.match(out, /Save<\/button>/, 'the prop is dropped at SSR, as every native prop is'); + assert.ok(!/formaction|formmethod|formenctype/.test(out), 'and nothing is invented for it'); + } const ok = await renderToString( html`
`, { ssr: true }, ); - assert.match(ok, /formaction="\/search"/, 'the static retarget is still allowed'); + assert.match(ok, /formaction="\/search"/, 'a static retarget is still allowed'); }); test('a falsy boolean name hole leaves the identity channel free', async () => { @@ -866,7 +941,7 @@ test('a falsy boolean name hole leaves the identity channel free', async () => { html`
`, { ssr: true }, ); - assert.match(out, /x<\/button>/); + assert.match(out, /x<\/button>/); // Truthy emits `name=""`, which collides with the identity. await assert.rejects( () => renderToString( diff --git a/packages/server/src/ssr.js b/packages/server/src/ssr.js index b6344773f..d69feb635 100644 --- a/packages/server/src/ssr.js +++ b/packages/server/src/ssr.js @@ -2109,7 +2109,7 @@ function reachedVendorSpecifiers(graph, entryFiles, componentUrls, appDir, elida * @param {string} prefix * @param {string} bodyHtml * @param {string} closer - * @param {{ pending: {id: string, promise: Promise, formScope?: 'none'|'unbound'|'bound'|'unknown'}[], nextId: number }} ctx + * @param {{ pending: {id: string, promise: Promise}[], nextId: number }} ctx * @param {number} status * @param {Request | undefined} req * @param {URL | undefined} url @@ -2165,15 +2165,14 @@ function streamingHtmlResponse(prefix, bodyHtml, closer, ctx, status, req, url, try { const resolved = await p.promise; const sub = { pending: [], nextId: ctx.nextId, dev: ctx.dev }; - // Carry the boundary's form scope (#1207). This is a fresh scan - // that cannot see the shell the boundary sits in, so without it - // a ``, }; -const refusedUnboundSubmitter = { - 'formaction=${fn}': () => html``, - 'camelCase formAction=${fn}': () => html``, -}; /** * The two shapes #1155 turned into a BINDING rather than a stringify: @@ -99,6 +95,12 @@ const refusedAsUnidentified = { 'action=${fn}': () => html`
`, 'upper-case ACTION=${fn}': () => html`
`, 'formaction=${fn} inside bound form': () => html`
`, + // #1307: a bound submitter no longer asks anything of its enclosing form, so + // these two moved here from their own table. They still refuse, because + // `leaky` has no identity to bind, which is the leak guard this file exists + // for. The enclosing form has nothing to do with it either way. + 'formaction=${fn} with no bound form': () => html``, + 'camelCase formAction=${fn} with no bound form': () => html``, }; for (const [name, mk] of Object.entries(refused)) { @@ -123,18 +125,6 @@ for (const [name, mk] of Object.entries(refused)) { `[${runtime}] streaming refusal must not carry the source (${name})`); } -for (const [name, mk] of Object.entries(refusedUnboundSubmitter)) { - let threw = null; - try { await renderToString(mk(), { ssr: true }); } catch (e) { threw = e; } - assert.ok(threw, `[${runtime}] buffered SSR must refuse ${name}`); - assert.match(threw.message, /requires the enclosing
to also be bound/, `[${runtime}] ${name} message`); - - let streamThrew = null; - try { await drain(renderToStream(mk(), { ssr: false })); } catch (e) { streamThrew = e; } - assert.ok(streamThrew, `[${runtime}] streaming SSR must refuse ${name}`); - assert.match(streamThrew.message, /requires the enclosing to also be bound/, `[${runtime}] ${name} message`); -} - for (const [name, mk] of Object.entries(refusedAsUnidentified)) { for (const [machine, run] of [ ['buffered', () => renderToString(mk(), { ssr: true })], diff --git a/test/bun/form-action-submitter-parity.test.mjs b/test/bun/form-action-submitter-parity.test.mjs index bf8716a49..b80d29882 100644 --- a/test/bun/form-action-submitter-parity.test.mjs +++ b/test/bun/form-action-submitter-parity.test.mjs @@ -1,5 +1,11 @@ /** - * Cross-runtime parity test for per-submitter `formaction=${action}` (#1207). + * Cross-runtime parity test for per-submitter `formaction=${action}` (#1207), + * and for the submission attributes a bound submitter carries itself (#1307). + * + * SSR is runtime-sensitive, so the emission has to be proven on Bun as well as + * Node: the whole point of #1307 is that the button ships `formmethod="post"` + * and an enctype in the served HTML, and a runtime that emitted one and not the + * other would produce a form that silently posts nowhere with JS off. */ import { test } from 'node:test'; import assert from 'node:assert/strict'; @@ -20,15 +26,46 @@ test('SSR: formaction=${fn} on submitter inside bound form emits submitter name= `; const out = await renderToString(tpl, { ssr: true }); assert.match(out, //); - assert.match(out, /`; - await assert.rejects( - () => renderToString(tpl, { ssr: true }), - /requires the enclosing to also be bound/, +test('SSR: a bound submitter is self-sufficient on this runtime (#1307)', async () => { + // The headline of #1307, asserted cross-runtime: a form that binds nothing and + // declares no method, whose button still submits a POST the action can read. + // This threw before the change, on both runtimes. + for (const tpl of [ + html`
`, + html``, + html`
`, + ]) { + const out = await renderToString(tpl, { ssr: true }); + assert.match(out, /name="__webjs_action" value="hash\/deleteAction"/); + assert.match(out, /formmethod="post"/, 'the button supplies its own method'); + assert.match(out, /formenctype="multipart\/form-data"/, 'and its own enctype'); + } +}); + +test('SSR: a bound submitter contradicting its own binding refuses on this runtime', async () => { + for (const [tpl, expected] of [ + [html``, /formmethod=/], + [html``, /formenctype=/], + [html``, /dialog/], + ]) { + await assert.rejects(() => renderToString(tpl, { ssr: true }), expected); + } +}); + +test("SSR: a PLAIN submitter's own override is left alone on this runtime", async () => { + // #1307 reverses #1207's Part B, and the reversal has to be identical on both + // runtimes or the same markup would refuse on one and render on the other. + const out = await renderToString( + html`
`, + { ssr: true }, ); + assert.match(out, /formmethod="get"/); }); test('SSR: formaction=${fn} on submitter with name attribute throws refusal', async () => { @@ -72,13 +109,16 @@ test('SSR: nested submitter templates keep the enclosing form binding', async () // runtime and not the other would ship a page that works in dev and 405s in // production, which is the same works-one-way-only failure Part B exists to // close. -test('SSR: Part B refuses an unparseable submitter enctype on both runtimes', async () => { +test('SSR: a BOUND submitter\'s own contradictions refuse on both runtimes', async () => { + // Was "Part B refuses ...". #1307 scoped every row here to a submitter that + // BINDS: the author attached an action to this button and then told this same + // button to submit in a way that action could never read. The rows about a + // PLAIN button's override moved to the carve-out test below, because native + // HTML defines that outcome and the renderer now honours it. const refused = [ - ['plain formenctype', html`
`], - ['plain formmethod', html`
`], - ['padded formmethod', html`
`], - ['submit input', html`
`], - ['nested template', html`
${html``}
`], + ['bound formenctype', html``], + ['bound formmethod', html``], + ['bound padded formmethod', html``], ['bound plus dialog', html`
`], ]; for (const [label, tpl] of refused) { @@ -86,6 +126,22 @@ test('SSR: Part B refuses an unparseable submitter enctype on both runtimes', as } }); +test("SSR: a PLAIN submitter's overrides render untouched on both runtimes", async () => { + // Every row here refused before #1307. The reversal has to be identical on + // Node and Bun, or the same markup would refuse on one runtime and render on + // the other, which is the precise class of bug this file exists to catch. + const allowed = [ + ['plain formenctype', html`
`, /formenctype="text\/plain"/], + ['plain formmethod', html`
`, /formmethod="get"/], + ['submit input', html`
`, /formenctype="text\/plain"/], + ['nested template', html`${html``}
`, /formmethod="get"/], + ]; + for (const [label, tpl, expected] of allowed) { + const out = await renderToString(tpl, { ssr: true }); + assert.match(out, expected, label); + } +}); + test('SSR: Part B carve-outs render on both runtimes', async () => { const allowed = [ ['dialog dismissal', html`
`, /formmethod="dialog"/], From a8606dd00ae18ce3e0631aef51aae5d1f246e84b Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 7 Aug 2026 14:26:38 +0530 Subject: [PATCH 02/13] fix: send a form submission with the enctype the author declared The client router built a FormData for every submission and sent it with no explicit content type, so fetch always derived multipart/form-data and the authored enctype was never read at all. `application/x-www-form-urlencoded` is the HTML default, so a plain `
` MEANS urlencoded. That form sent a urlencoded body with JS off and a multipart body with JS on: one template, two different requests, which is the thing progressive enhancement is supposed to rule out. It is independent of the submitter work but lives in the same function, so it is fixed here rather than left as a known divergence. Resolve the effective enctype with native precedence (a submitter's formenctype over the form's) and encode the body to match. Multipart keeps sending FormData; urlencoded sends URLSearchParams, with a File serialized as its name, which is what the platform's own urlencoded serializer does. Turbo drops file entries here instead, losing a field the no-JS path sends. `text/plain` gets no encoder. The HTML spec calls its payload not reliably interpretable by computer, and `looksLikeFormSubmission` accepts multipart and urlencoded only, so such a POST is answered by a bare 405 before its body is read. The router declines the submission and lets the browser perform it natively, so both paths do the same thing. Turbo enumerates this encoding and then sends FormData anyway, which is the divergence being avoided. The value is matched UNTRIMMED, like every other enumerated attribute here: a padded `enctype=" multipart/form-data "` falls to the invalid-value default and a browser sends urlencoded for it, so trimming would have reintroduced the same disagreement one level down. A test caught that. The real proof is in the browser suite, reading the encoded body off a stubbed fetch, because the linkedom unit harness cannot drive onSubmit as far as preventDefault (`new FormData(formElement)` throws there). That limit is now recorded beside the bail tests it weakens, since they can show a submission was not routed but not that it bailed for the stated reason. --- .../webjs/references/data-and-actions.md | 2 +- AGENTS.md | 2 +- packages/core/src/router-client.js | 92 +++++++++++++- .../browser/form-action-submit.test.js | 113 ++++++++++++++++++ .../core/test/routing/router-client.test.js | 93 ++++++++++++++ .../app/docs/progressive-enhancement/page.ts | 5 +- 6 files changed, 302 insertions(+), 5 deletions(-) diff --git a/.agents/skills/webjs/references/data-and-actions.md b/.agents/skills/webjs/references/data-and-actions.md index f6772bd63..a94342116 100644 --- a/.agents/skills/webjs/references/data-and-actions.md +++ b/.agents/skills/webjs/references/data-and-actions.md @@ -135,7 +135,7 @@ import { createPost } from '#modules/posts/actions/create-post.server.ts'; html`
`; ``` -The renderer omits the `action` attribute so the form posts to the page's own url, supplies `method="post"` and an enctype, and emits a hidden `__webjs_action` field carrying the action's `/` identity, the same identity the RPC endpoint resolves. Nothing about the action's source reaches the browser. With JS off this is an ordinary HTML submission; with JS the client router posts the same body to the same url, so the two paths are identical by construction. +The renderer omits the `action` attribute so the form posts to the page's own url, supplies `method="post"` and an enctype, and emits a hidden `__webjs_action` field carrying the action's `/` identity, the same identity the RPC endpoint resolves. Nothing about the action's source reaches the browser. With JS off this is an ordinary HTML submission; with JS the client router posts the same body to the same url, encoded per the declared `enctype` (#1307: multipart stays `FormData`, urlencoded, which is the HTML default, is sent as `URLSearchParams`), so the two paths are identical by construction. **A form-bound action always receives the `FormData`**, which is where it differs from the same function called over RPC (rich arguments) or server-to-server. `validate` is the typing seam: it takes the `FormData` and its transform-return becomes the action's typed input. diff --git a/AGENTS.md b/AGENTS.md index 5b7a021c4..6da30576e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -367,7 +367,7 @@ Derive the type at every boundary: a DB row from the schema (`typeof todos.$infe ## Client navigation: automatic, nothing to opt into -The router auto-enables when `@webjsdev/core` loads (any page with a component), so there is nothing to opt INTO. An app that wants plain full-page (MPA) navigation can opt OUT app-wide with `{ "webjs": { "clientRouter": false } }` (#629), or per-moment at runtime with `disableClientRouter()`. SSR auto-emits KEYED boundary comment pairs around each layout's children AND the page itself (open ``, close ``; the route-key is the resolved path with param values percent-encoded, #1015). The router strictly scans both DOMs (any truncated, mispaired, or duplicated boundary poisons the scan) and applies a two-tier swap with Next.js remount parity: a changed route-key REPLACES (remounts) at the PARENT of the shallowest changed boundary (the range that contains the changed layout's own markup, exact Next remount scope), an unchanged one MORPHS the deepest shared boundary in place (a searchParams-only nav preserves hydrated component state). A poisoned or disjoint scan degrades to a full page load, never a guessed recovery, so silent DOM corruption is structurally impossible; outer-layout DOM identity is preserved on every soft path. **Every degradation dispatches `webjs:navigation-fallback` on `document` in ALL environments** (detail `{ cause, href, willReload }`, not cancelable), so a full page load on a click is observable in production rather than silent (#1114). Form submissions ride the same pipeline (`data-no-router` opts out). Wire bytes are minimized via the `X-Webjs-Have` header (`segment:route-key` entries, so a dynamic layout held for OTHER params is re-rendered rather than short-circuited; the server returns only the divergent fragment, served `private` so a shared cache can never store the reduced body and serve it to a full-page navigation, and additionally marked `Vary: X-Webjs-Have` for caches that honour it (#1140; `Vary` alone was not enough, since Cloudflare honours only `Accept-Encoding`)); scroll is restored on back/forward. **The link-prefetch cache is ANCHOR-VALIDATED** (#1114): a reduced fragment begins at the boundary the server short-circuited on, and on consume the router checks that boundary is still live with the same route-key. A root-anchored fragment therefore survives an unrelated navigation (still a cache hit), while one anchored deeper is discarded once that layout is gone, because applying it would share no boundary with the live DOM and force a full page load. The router also never prefetches the page it is already on (#1106): that request can never serve a later navigation and only occupies a capped cache slot. A non-GET `
` that BINDS a server action (`action=${fn}`) is the no-JS write-path (with JS the router posts the same body to the same url and applies the response in place: a `422` swaps without reload, a `303` is followed via fetch). A failed navigation recovers in place (a cancelable `webjs:navigation-error` event, else a minimal in-place alert), never a destructive full reload. +The router auto-enables when `@webjsdev/core` loads (any page with a component), so there is nothing to opt INTO. An app that wants plain full-page (MPA) navigation can opt OUT app-wide with `{ "webjs": { "clientRouter": false } }` (#629), or per-moment at runtime with `disableClientRouter()`. SSR auto-emits KEYED boundary comment pairs around each layout's children AND the page itself (open ``, close ``; the route-key is the resolved path with param values percent-encoded, #1015). The router strictly scans both DOMs (any truncated, mispaired, or duplicated boundary poisons the scan) and applies a two-tier swap with Next.js remount parity: a changed route-key REPLACES (remounts) at the PARENT of the shallowest changed boundary (the range that contains the changed layout's own markup, exact Next remount scope), an unchanged one MORPHS the deepest shared boundary in place (a searchParams-only nav preserves hydrated component state). A poisoned or disjoint scan degrades to a full page load, never a guessed recovery, so silent DOM corruption is structurally impossible; outer-layout DOM identity is preserved on every soft path. **Every degradation dispatches `webjs:navigation-fallback` on `document` in ALL environments** (detail `{ cause, href, willReload }`, not cancelable), so a full page load on a click is observable in production rather than silent (#1114). Form submissions ride the same pipeline (`data-no-router` opts out). Wire bytes are minimized via the `X-Webjs-Have` header (`segment:route-key` entries, so a dynamic layout held for OTHER params is re-rendered rather than short-circuited; the server returns only the divergent fragment, served `private` so a shared cache can never store the reduced body and serve it to a full-page navigation, and additionally marked `Vary: X-Webjs-Have` for caches that honour it (#1140; `Vary` alone was not enough, since Cloudflare honours only `Accept-Encoding`)); scroll is restored on back/forward. **The link-prefetch cache is ANCHOR-VALIDATED** (#1114): a reduced fragment begins at the boundary the server short-circuited on, and on consume the router checks that boundary is still live with the same route-key. A root-anchored fragment therefore survives an unrelated navigation (still a cache hit), while one anchored deeper is discarded once that layout is gone, because applying it would share no boundary with the live DOM and force a full page load. The router also never prefetches the page it is already on (#1106): that request can never serve a later navigation and only occupies a capped cache slot. A non-GET `` that BINDS a server action (`action=${fn}`) is the no-JS write-path (with JS the router posts the same body to the same url and applies the response in place: a `422` swaps without reload, a `303` is followed via fetch). **The router ENCODES that body per the declared `enctype`** (#1307), resolved with native precedence (a submitter's `formenctype` over the form's): `multipart/form-data` sends `FormData`, and `application/x-www-form-urlencoded`, the HTML default and therefore what a plain `` means, sends `URLSearchParams` (a `File` serializes as its name, as the platform does). It previously built `FormData` for everything, so an ordinary POST form sent a urlencoded body with JS off and a multipart body with JS on. A `text/plain` POST is the one encoding the server cannot parse, so the router declines it and lets the browser submit natively, which keeps both paths doing the same thing. A failed navigation recovers in place (a cancelable `webjs:navigation-error` event, else a minimal in-place alert), never a destructive full reload. The advanced client-router surface is in `references/client-router-and-streaming.md`: **link prefetch** (on by default, device-adaptive default: `intent` on a hover pointer, `viewport` (dwell-gated, cancel-on-scroll-out) on touch, per-link `data-prefetch` override), **``** partial-swap regions, **View Transitions** (opt-in via ``, where the router checks that exact `content` value and a bare tag enables nothing, plus `data-webjs-permanent` to persist a live element), **stream actions** (`` element-level updates, #248), and the **opt-in nav-loading indicator** (`` exposes a `data-navigating` attribute during a nav so you can style a CSS-only progress affordance, off by default because toggling a root attribute re-resolves `oklch()` tokens to a one-frame repaint flash on iOS WebKit, #610). Production benefits from HTTP/2 at the edge; `npm run start` speaks plain HTTP/1.1 (put a reverse proxy in front for TLS + HTTP/2). diff --git a/packages/core/src/router-client.js b/packages/core/src/router-client.js index e39904853..e3d8fe9ed 100644 --- a/packages/core/src/router-client.js +++ b/packages/core/src/router-client.js @@ -866,6 +866,18 @@ function onSubmit(e) { const method = getSubmitMethod(form, submitter); if (method === 'dialog') return; + // #1307: `text/plain` is a legal native encoding the server cannot parse + // (`looksLikeFormSubmission` accepts multipart and urlencoded only), and + // there is no honest way to send it over `fetch` and have the response mean + // anything. Bail to the browser so BOTH paths do the same thing, rather than + // silently sending multipart, which is what made the same form behave one way + // with JS and another way without it. Turbo enumerates this encoding and then + // sends FormData anyway, which is the divergence being avoided here. A safe + // method ignores the enctype entirely, per the form-submission algorithm. + const enctype = getSubmitEnctype(form, submitter); + const isSafeMethod = method === 'get' || method === 'head'; + if (!isSafeMethod && enctype === 'text/plain') return; + const action = getSubmitAction(form, submitter); /** @type {URL} */ let url; try { url = new URL(action, location.href); } @@ -873,7 +885,7 @@ function onSubmit(e) { if (url.origin !== location.origin) return; if (NON_HTML_EXTENSIONS.test(url.pathname)) return; - const body = buildSubmitFormData(form, submitter); + const body = encodeSubmitBody(buildSubmitFormData(form, submitter), enctype); e.preventDefault(); // Resolve the target frame for the submit, same precedence as a link: @@ -911,6 +923,76 @@ function getSubmitAction(form, submitter) { return form.getAttribute('action') || form.action || location.href; } +/** + * The three `enctype` keywords, plus the normalization a browser applies. + * + * Both the missing-value AND the invalid-value default of the `enctype` + * enumerated attribute are `application/x-www-form-urlencoded`, so + * `enctype="nonsense"` really does mean urlencoded and has to be sent as such. + * Only an exact, ASCII-case-insensitive match on one of the other two keywords + * means anything else. + * + * @param {string | null | undefined} raw + * @returns {'application/x-www-form-urlencoded' | 'multipart/form-data' | 'text/plain'} + */ +function normalizeEnctype(raw) { + // Compared UNTRIMMED, the same rule `assertSubmittableForm` applies in + // `form-action.js`. An enumerated attribute is matched against exact + // keywords with no whitespace stripping, so `enctype=" multipart/form-data "` + // falls to the invalid-value default and a BROWSER sends urlencoded for it. + // Trimming here would send multipart, so the router would disagree with the + // no-JS path on exactly the shape this function exists to keep in step. + const v = String(raw || '').toLowerCase(); + if (v === 'multipart/form-data') return 'multipart/form-data'; + if (v === 'text/plain') return 'text/plain'; + return 'application/x-www-form-urlencoded'; +} + +/** + * Enctype resolution: the submitter's `formenctype` wins over the form's + * `enctype`, exactly as `getSubmitMethod` resolves the method (#1307). Turbo + * resolves it the same way, in `core/drive/form_submission.js`. + * + * @param {HTMLFormElement} form + * @param {HTMLElement | null} submitter + */ +function getSubmitEnctype(form, submitter) { + return normalizeEnctype( + (submitter && submitter.getAttribute('formenctype')) || form.getAttribute('enctype'), + ); +} + +/** + * Encode a submission body the way the DECLARED enctype says to (#1307). + * + * The router used to build a `FormData` and send it with no explicit content + * type, so `fetch` always derived `multipart/form-data` and the authored + * `enctype` was never read at all. An author writing + * `enctype="application/x-www-form-urlencoded"`, which is also the HTML + * DEFAULT and therefore what a plain `` means, got + * urlencoded with JS off and multipart with JS on. Same form, two different + * request bodies, which is exactly what progressive enhancement rules out. + * + * A `File` entry serializes as its NAME under urlencoded, which is what the + * platform's own urlencoded serializer does. (Turbo drops file entries here + * entirely, in `http/fetch_request.js`, which loses a field the no-JS path + * sends.) + * + * A bound form is unaffected: it carries an explicit + * `enctype="multipart/form-data"`, and since #1307 a bound submitter carries + * `formenctype="multipart/form-data"`, so both resolve to multipart as before. + * + * @param {FormData} formData + * @param {'application/x-www-form-urlencoded' | 'multipart/form-data' | 'text/plain'} enctype + * @returns {FormData | URLSearchParams} + */ +function encodeSubmitBody(formData, enctype) { + if (enctype === 'multipart/form-data') return formData; + const params = new URLSearchParams(); + for (const [k, v] of formData) params.append(k, typeof v === 'string' ? v : v.name); + return params; +} + /** * Build FormData honoring the submitter's name=value (per HTML5 form * submission algorithm). Modern browsers + the `FormData(form, submitter)` @@ -1827,7 +1909,11 @@ async function performNavigation(href, isPopState, frameId) { * * @param {string} href Absolute target URL. * @param {string} method Lowercased HTTP verb. - * @param {FormData} body + * @param {FormData | URLSearchParams} body Encoded per the declared enctype + * (#1307): `FormData` for multipart, `URLSearchParams` for urlencoded. Both + * iterate as `[name, value]` pairs, which is all the safe-method query-string + * promotion below needs, and `fetch` derives the right content type from + * either without an explicit header. * @param {string | null} frameId * @param {HTMLFormElement | null} [form] The submitted form, for busy + events. */ @@ -5116,6 +5202,8 @@ export { getSubmitMethod as _getSubmitMethod, getSubmitAction as _getSubmitAction, buildSubmitFormData as _buildSubmitFormData, + getSubmitEnctype as _getSubmitEnctype, + encodeSubmitBody as _encodeSubmitBody, restoreOptimistic as _restoreOptimistic, eligibleAnchorHref as _eligibleAnchorHref, viewTransitionsEnabled as _viewTransitionsEnabled, diff --git a/packages/core/test/routing/browser/form-action-submit.test.js b/packages/core/test/routing/browser/form-action-submit.test.js index 70e8e7646..03ada250e 100644 --- a/packages/core/test/routing/browser/form-action-submit.test.js +++ b/packages/core/test/routing/browser/form-action-submit.test.js @@ -185,4 +185,117 @@ suite('Client router: bound form submissions (#1155)', () => { teardown(); } }); + + // ------------------------------------------------------------------------- + // #1307: the router honours the DECLARED enctype. + // + // It used to build a `FormData` for every submission and send it with no + // explicit content type, so `fetch` always derived `multipart/form-data` and + // the authored `enctype` was never read at all. A plain `` + // MEANS `application/x-www-form-urlencoded` in HTML, so the same form sent a + // urlencoded body with JS off and a multipart body with JS on. Two different + // requests from one template is exactly what progressive enhancement rules + // out, and it is why these assertions read the real RequestInit rather than + // trusting the resolver in isolation. + // ------------------------------------------------------------------------- + + const okHtml = () => new Response( + '

ok

', + { headers: { 'content-type': 'text/html', 'x-webjs-build': '' } }, + ); + + test('a form declaring no enctype sends URLENCODED, the HTML default (#1307)', async () => { + setup(okHtml); + try { + render(html` + + + + + `, container); + container.querySelector('button').click(); + await tick(); + const post = calls[0]; + assert.ok(post, 'router issued the submission fetch'); + assert.ok(post.init.body instanceof URLSearchParams, + 'a form with no enctype must NOT be sent as multipart'); + assert.equal(post.init.body.get('email'), 'a@b.com', 'and the field survives the encoding'); + // COUNTERFACTUAL: revert `encodeSubmitBody` to return the FormData + // unconditionally and this goes red, which is what pins the fix. + } finally { teardown(); } + }); + + test('a form declaring multipart still sends FormData', async () => { + setup(okHtml); + try { + render(html` +
+ + +
+ `, container); + container.querySelector('button').click(); + await tick(); + assert.ok(calls[0].init.body instanceof FormData, 'multipart is still FormData'); + } finally { teardown(); } + }); + + test("a submitter's formenctype overrides the form's, as native precedence says", async () => { + setup(okHtml); + try { + render(html` +
+ + +
+ `, container); + container.querySelector('button').click(); + await tick(); + assert.ok(calls[0].init.body instanceof URLSearchParams, + "the button's own formenctype decides the encoding"); + } finally { teardown(); } + }); + + test('an invalid enctype is urlencoded, not passed through or treated as text/plain', async () => { + // `enctype` is an enumerated attribute whose invalid-value default is + // urlencoded, so a browser sends urlencoded for this and so must the router. + setup(okHtml); + try { + render(html` +
+ + +
+ `, container); + container.querySelector('button').click(); + await tick(); + assert.ok(calls[0], 'the submission was still routed, not bailed'); + assert.ok(calls[0].init.body instanceof URLSearchParams); + } finally { teardown(); } + }); + + test('a text/plain POST is NOT routed, so both paths do the same native thing', async () => { + // The server parses multipart and urlencoded only, so there is no honest + // way to send this over fetch. Bailing means the browser performs exactly + // the submission it would have without JS. The nav guard would catch the + // real navigation, so what is asserted is simply that no fetch was issued. + setup(okHtml); + try { + render(html` +
+ +
+ `, container); + const btn = container.querySelector('button'); + // Stop the native submission the bail deliberately allows, so the test + // page is not navigated away. The router runs on the BUBBLE phase, so a + // capture-phase listener here would pre-empt what is being measured. + const stop = (e) => e.preventDefault(); + container.addEventListener('submit', stop); + btn.click(); + await tick(); + container.removeEventListener('submit', stop); + assert.equal(calls.length, 0, 'the router did not take it'); + } finally { teardown(); } + }); }); diff --git a/packages/core/test/routing/router-client.test.js b/packages/core/test/routing/router-client.test.js index 7c3a6c913..06ac6717b 100644 --- a/packages/core/test/routing/router-client.test.js +++ b/packages/core/test/routing/router-client.test.js @@ -30,6 +30,7 @@ let _collect, _plan, _keyOf, _diffEl, _reconcile, _applySwap, _prefetchCache, _snapshotCache, _LIVE_ATTRS, _blurOutgoingFocus, _onSubmit, _getSubmitMethod, _getSubmitAction, _buildSubmitFormData, + _getSubmitEnctype, _encodeSubmitBody, _restoreOptimistic, _navToken, _bumpNavToken, _currentPageUrl, _setCurrentPageUrl, _resetWarnOnce, _eligibleAnchorHref, _prefetchSuppressed, _prefetchMode, _prefetchHasHoverPointer, _prefetch, _prefetchTake, _prefetchAnchor, @@ -101,6 +102,8 @@ before(async () => { _getSubmitMethod, _getSubmitAction, _buildSubmitFormData, + _getSubmitEnctype, + _encodeSubmitBody, _restoreOptimistic, _navToken, _bumpNavToken, @@ -3006,6 +3009,66 @@ test('getSubmitMethod: tolerates null submitter (programmatic submit)', () => { assert.equal(_getSubmitMethod(form, null), 'post'); }); +test('getSubmitEnctype: submitter formenctype overrides form enctype', () => { + // Native precedence, the same rule `getSubmitMethod` follows one line up. + const form = formFrom('
'); + assert.equal(_getSubmitEnctype(form, form.querySelector('button')), 'multipart/form-data'); +}); + +test('getSubmitEnctype: the missing-value default is urlencoded, per HTML', () => { + // This is the case that mattered (#1307). A plain `
` + // MEANS urlencoded, and the router used to send multipart for it, so the + // same form produced a different request body with JS on than with JS off. + const form = formFrom('
'); + assert.equal(_getSubmitEnctype(form, form.querySelector('button')), 'application/x-www-form-urlencoded'); +}); + +test('getSubmitEnctype: an INVALID value is urlencoded too, not passed through', () => { + // `enctype` is an enumerated attribute whose invalid-value default is also + // urlencoded, so `nonsense` really does mean urlencoded. Treating an + // unrecognised value as text/plain would bail a form that submits perfectly. + for (const raw of ['nonsense', 'TEXT/HTML', '', ' multipart/form-data ']) { + const form = formFrom(`
`); + assert.equal( + _getSubmitEnctype(form, form.querySelector('button')), + 'application/x-www-form-urlencoded', + `${raw || '(empty)'} normalizes to the invalid-value default`, + ); + } +}); + +test('getSubmitEnctype: the two other keywords are matched case-insensitively', () => { + for (const [raw, want] of [ + ['MULTIPART/FORM-DATA', 'multipart/form-data'], + ['Text/Plain', 'text/plain'], + ]) { + const form = formFrom(`
`); + assert.equal(_getSubmitEnctype(form, form.querySelector('button')), want); + } +}); + +test('encodeSubmitBody: urlencoded sends URLSearchParams, multipart sends FormData', () => { + const fd = new FormData(); + fd.append('email', 'a@b.com'); + fd.append('note', 'hi there'); + const params = _encodeSubmitBody(fd, 'application/x-www-form-urlencoded'); + assert.ok(params instanceof URLSearchParams, 'urlencoded must not send FormData'); + assert.equal(params.get('email'), 'a@b.com'); + assert.equal(params.get('note'), 'hi there'); + assert.equal(_encodeSubmitBody(fd, 'multipart/form-data'), fd, 'multipart passes the FormData through'); +}); + +test('encodeSubmitBody: a File under urlencoded is sent as its NAME, matching the platform', () => { + // The platform's urlencoded serializer writes the file's name. Turbo drops + // file entries entirely here, which loses a field the no-JS path sends. + const fd = new FormData(); + fd.append('avatar', new File(['x'], 'portrait.png', { type: 'image/png' })); + fd.append('email', 'a@b.com'); + const params = _encodeSubmitBody(fd, 'application/x-www-form-urlencoded'); + assert.equal(params.get('avatar'), 'portrait.png'); + assert.equal(params.get('email'), 'a@b.com', 'and the sibling text field survives'); +}); + test('getSubmitAction: submitter formaction overrides form action', () => { const form = formFrom('
'); const submitter = form.querySelector('button'); @@ -3070,6 +3133,36 @@ test('onSubmit: ignores submissions with method="dialog"', () => { assert.equal(e._wasPrevented(), false, 'native dialog dismissal not routed'); }); +// NOTE on these two bail tests, and on every `onSubmit: ignores ...` test +// around them: this harness is linkedom, where `new FormData(formElement)` +// throws, so `onSubmit` cannot be driven all the way to `preventDefault()` +// here. A bail assertion therefore proves that the submission was NOT routed, +// but cannot prove it bailed for the stated REASON. The positive control (an +// ordinary POST still being intercepted, and the body actually encoded per the +// declared enctype) lives in the browser suite, in +// `packages/core/test/routing/browser/form-action-submit.test.js`, against a +// real DOM and a stubbed fetch. +test('onSubmit: an unsafe text/plain submission bails to the browser (#1307)', () => { + // The server parses multipart and urlencoded only, so there is no honest way + // to send text/plain over fetch and have the response mean anything. Bailing + // makes the JS-on and JS-off paths do the SAME thing (both a native + // text/plain POST, both answered the same way), which is the requirement. + const form = formFrom('
'); + const e = fakeSubmitEvent(form); + _onSubmit(e); + assert.equal(e._wasPrevented(), false, 'the browser performs the submission'); +}); + +test('onSubmit: a submitter formenctype="text/plain" bails too', () => { + // Native precedence: the submitter's override decides the encoding, so the + // bail has to read it there as well or a per-button text/plain would be sent + // as multipart under JS and natively without it. + const form = formFrom('
'); + const e = fakeSubmitEvent(form, form.querySelector('button')); + _onSubmit(e); + assert.equal(e._wasPrevented(), false); +}); + test('onSubmit: ignores cross-origin actions', () => { const form = formFrom('
'); const e = fakeSubmitEvent(form); diff --git a/website/app/docs/progressive-enhancement/page.ts b/website/app/docs/progressive-enhancement/page.ts index ef923652f..a07318850 100644 --- a/website/app/docs/progressive-enhancement/page.ts +++ b/website/app/docs/progressive-enhancement/page.ts @@ -177,7 +177,10 @@ export default function NewPost({ actionData }: {

- "Identical by construction" is a claim about the whole submission, submitter included. A button's own formmethod / formenctype can defeat the form it sits in, so the renderers read those on EVERY submitter inside a bound form, whether or not that button binds an action of its own: <form action=\${fn}><button formenctype="text/plain"> would submit fine with JS (the router posts FormData and ignores the attribute) and be a bare 405 without it, so it is refused at render instead. The same goes for formmethod="get", which sends no body. Two shapes are deliberately left alone, because neither submits to the bound action: formmethod="dialog" is a native <dialog> dismissal, and a plain formaction="/url" points the submission somewhere else entirely. + "Identical by construction" is a claim about the whole submission, encoding included, and it is enforced in two places. The router resolves the effective enctype with native precedence (a submitter's formenctype over the form's) and ENCODES the body accordingly: multipart/form-data sends FormData, and application/x-www-form-urlencoded, which is the HTML default and therefore what a plain <form method="post"> means, sends URLSearchParams. Before that the router built FormData for everything, so an ordinary POST form sent a urlencoded body without JS and a multipart body with it. A text/plain POST is the one encoding the server cannot parse, so the router declines it and lets the browser submit natively, which makes both paths do the same thing rather than one of them appear to work. +

+

+ A submitter that BINDS its own action is refused when it also declares a formmethod other than post, an unparseable formenctype, or formmethod="dialog", because those contradict the action attached to that same button. A button that binds nothing is left alone: its formmethod / formenctype is a legal native override, the author wrote it deliberately, and the form's action simply does not run, exactly as the same markup behaves anywhere else. In dev the client logs a console error at submit time when a submission is carrying an identity it cannot deliver.

3. Make components render correctly on the server

From 50d2d7614447dd64f034f3e3116369ddbd522e2d Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 7 Aug 2026 14:31:50 +0530 Subject: [PATCH 03/13] test: prove a bound submitter runs from an unbound form with JS off Adds the dogfood route the headline claim needs. `/feedback/triage-split` renders a form with NO action and NO method, so a browser defaults it to GET, and both of its buttons bind their own action. Both shapes are present because they exercise different renderer paths. "Save draft" is inline in the page template, which SSR scans in one pass, and was the case the renderer used to REFUSE outright. "Publish" comes from a component, whose template SSR renders in a separate pass with no view of the host page, and was the case it bound anyway because it could not tell. The second is why the fix had to remove the question rather than answer it better: no scan can see a page from inside a component's own render. The e2e asserts the served markup carries `formmethod` and `formenctype` on the button itself, and then that pressing it actually RUNS the action, in a real browser with scripting disabled. Markup alone would not prove the second. Note for anyone re-checking this: e2e resolves `@webjsdev/core` through the BUILT dist, so `npm run --prefix packages/core build:dist` has to run before the suite or it exercises the previous build. The counterfactual for this test passed vacuously until that was done, and fails correctly after it. --- .../blog/app/feedback/triage-split/page.ts | 58 +++++++++++++++ .../feedback/components/publish-button.ts | 38 ++++++++++ test/e2e/e2e.test.mjs | 73 +++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 examples/blog/app/feedback/triage-split/page.ts create mode 100644 examples/blog/modules/feedback/components/publish-button.ts diff --git a/examples/blog/app/feedback/triage-split/page.ts b/examples/blog/app/feedback/triage-split/page.ts new file mode 100644 index 000000000..8c188f70e --- /dev/null +++ b/examples/blog/app/feedback/triage-split/page.ts @@ -0,0 +1,58 @@ +import { html } from '@webjsdev/core'; +import { saveDraft } from '#modules/feedback/actions/save-draft.server.ts'; +import '#modules/feedback/components/publish-button.ts'; + +/** + * The #1307 shape, as an e2e fixture: a COMPLETELY UNBOUND form whose buttons + * each bind their own action. + * + * Note what this `
` does NOT have. No `action=${...}`, so it binds + * nothing. No `method`, so a browser would default it to GET. Before #1307 + * that was the silent failure: the renderer refused a bound submitter where it + * could see the form was unbound, and bound one anyway where it could not see + * (a button inside a component, which is the ordinary shape). The latter + * submitted as a GET, put the identity in the query string, re-rendered this + * page, and ran nothing. A 200 with no log and no visible symptom. + * + * It works now because a bound submitter carries its whole submission: the + * renderer puts `formmethod="post"` and `formenctype="multipart/form-data"` on + * the button itself, alongside the identity, so the button needs nothing from + * the form around it. + * + * Both shapes are here on purpose, because they exercise different renderer + * paths. "Save draft" is written INLINE in this page's template, which SSR + * scans in one pass. "Publish" is rendered by ``, a COMPONENT, + * whose template SSR renders in a separate pass with no view of this page. + * That second one is the case no scan could ever resolve, and it is why the + * fix had to remove the question rather than answer it better. + * + * `/feedback/triage` keeps the bound-form-plus-bound-submitter shape, so the + * two routes together cover a bound and an unbound host form. + */ + +type PageCtx = { + actionData?: { fieldErrors?: Record; values?: Record }; +}; + +export const metadata = { title: 'Triage (split) - WebJs Blog' }; + +export default function TriageSplitPage({ actionData }: PageCtx) { + const err = actionData?.fieldErrors?.note; + const val = actionData?.values?.note || ''; + return html` +
+

Triage a note (split)

+ + + ${err ? html`

${err}

` : ''} +
+ + +
+ +
+ `; +} diff --git a/examples/blog/modules/feedback/components/publish-button.ts b/examples/blog/modules/feedback/components/publish-button.ts new file mode 100644 index 000000000..fa59935bf --- /dev/null +++ b/examples/blog/modules/feedback/components/publish-button.ts @@ -0,0 +1,38 @@ +import { WebComponent, html } from '@webjsdev/core'; +import { publishDraft } from '#modules/feedback/actions/publish-draft.server.ts'; + +/** + * A bound submitter rendered by a COMPONENT (#1307). + * + * This is the case the old enclosing-form check could never resolve. SSR + * renders a component's template in a SEPARATE pass, walking the + * already-emitted HTML, so this template's scan has no view of the page that + * placed the tag and cannot see whether the surrounding `
` bound + * anything. The scan therefore had a third answer, cannot-tell, and had to + * bind on it: refusing would have rejected a per-row button in a list, and a + * refused component is ISOLATED at SSR, so production would have returned 200 + * with the button silently missing. + * + * Binding on cannot-tell is what produced the silent failure, because the + * button then carried an identity with no way to send it as a POST. + * + * Nothing here asks about the form any more. The renderer gives this button + * `formmethod="post"` and `formenctype="multipart/form-data"` alongside the + * identity, so it submits correctly inside `/feedback/triage-split`'s unbound, + * method-less form, with JavaScript on or off. + * + * The component itself is display-only, so elision drops its module from the + * browser. That is the point rather than an oversight: the button is a plain + * HTML submitter once SSR has run, and the no-JS e2e proves it needs no + * script at all. + */ +class PublishButton extends WebComponent({}) { + render() { + return html` + + `; + } +} +PublishButton.register('publish-button'); diff --git a/test/e2e/e2e.test.mjs b/test/e2e/e2e.test.mjs index 6323abdf8..2655fcbef 100644 --- a/test/e2e/e2e.test.mjs +++ b/test/e2e/e2e.test.mjs @@ -3463,6 +3463,79 @@ describe('E2E: form actions (no-JS + enhanced)', { skip: !process.env.WEBJS_E2E } finally { await p.close(); } }); + test('JS DISABLED: a bound submitter runs its action inside a COMPLETELY UNBOUND form (#1307)', async () => { + // THE headline assertion of #1307, and the one-line proof the whole change + // works. `/feedback/triage-split` renders a `` with NO action and NO + // method, so a browser defaults it to GET. Both of its buttons bind their + // own action, and "Publish" is rendered by a COMPONENT, which SSR renders + // in a separate pass that cannot see this page at all. + // + // Before the change this submitted as a GET: the identity rode the query + // string, the page re-rendered, and nothing ran. A 200 with no log, which + // is exactly why detection was the original plan and making it WORK is the + // better one. + // + // COUNTERFACTUAL: delete the `formmethod` / `formenctype` injection from + // `bindSubmitterStartTag` and this test goes red, because the button falls + // back to the form's GET default and the action never runs. + const p = await paBrowser.newPage(); + await p.setJavaScriptEnabled(false); + try { + await p.goto(`${paBase}/feedback/triage-split`, { waitUntil: 'domcontentloaded', timeout: 10000 }); + + // The served markup carries the whole submission on the button itself. + const shape = await p.evaluate(() => { + const btn = document.getElementById('publish'); + const form = btn.closest('form'); + return { + formMethodAttr: form.getAttribute('method'), + formActionAttr: form.getAttribute('action'), + name: btn.getAttribute('name'), + value: btn.getAttribute('value'), + formmethod: btn.getAttribute('formmethod'), + formenctype: btn.getAttribute('formenctype'), + formaction: btn.getAttribute('formaction'), + }; + }); + assert.equal(shape.formMethodAttr, null, 'the host form declares no method'); + assert.equal(shape.formActionAttr, null, 'and binds no action'); + assert.equal(shape.name, '__webjs_action', 'the component-rendered button carries the identity'); + assert.ok(/\/publishDraft$/.test(shape.value || ''), `identity value, got "${shape.value}"`); + assert.equal(shape.formmethod, 'post', 'and supplies its own method'); + assert.equal(shape.formenctype, 'multipart/form-data', 'and its own enctype'); + assert.equal(shape.formaction, null, 'no formaction url is emitted'); + + // And it actually RUNS, which is the part markup alone cannot prove. + await p.type('#note', 'ship it'); + await Promise.all([ + p.waitForNavigation({ waitUntil: 'domcontentloaded', timeout: 10000 }), + p.click('#publish'), + ]); + const ran = await p.evaluate(() => document.getElementById('ran')?.textContent || ''); + assert.equal(ran, 'publishDraft', + `the action must RUN from an unbound form, got "${ran}"`); + } finally { await p.close(); } + }); + + test('JS DISABLED: the INLINE bound submitter in the same unbound form runs too', async () => { + // The sibling path. "Save draft" is written inline in the page template, so + // SSR sees it in the same scan as the form and knew that form was unbound. + // That is the case the renderer used to REFUSE outright, as distinct from + // the component case it bound anyway. Both now bind and both work. + const p = await paBrowser.newPage(); + await p.setJavaScriptEnabled(false); + try { + await p.goto(`${paBase}/feedback/triage-split`, { waitUntil: 'domcontentloaded', timeout: 10000 }); + await p.type('#note', 'later'); + await Promise.all([ + p.waitForNavigation({ waitUntil: 'domcontentloaded', timeout: 10000 }), + p.click('#save'), + ]); + const ran = await p.evaluate(() => document.getElementById('ran')?.textContent || ''); + assert.equal(ran, 'saveDraft', `the inline submitter's action must run, got "${ran}"`); + } finally { await p.close(); } + }); + test('JS DISABLED: a failing submitter action re-renders THIS page at 422', async () => { // The per-button path has to reach the same 422 re-render as the form-level // one, or a validation failure on a submitter action would lose the page. From dcbdb5ddfe640140b84b6f7922aae8545eb64fd4 Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 7 Aug 2026 14:33:27 +0530 Subject: [PATCH 04/13] fix: balance a template hole in matchClosingBrace The scanner kept one flat depth counter and incremented it at `${`, but the matching `}` arrived while it was still in template state, so nothing ever decremented it. Depth could not return to zero, and a class body containing an interpolated template was unmatchable. A template hole is a code context nested INSIDE a template, not a brace in the enclosing block, so it gets its own frame on a stack. The `}` that returns that frame to zero pops back into the template rather than counting toward the block being matched. Latent rather than live: every caller passes a position-preserving mask in which holes are already blanked, so no caller has ever fed it a `${`. Fixed independently of the form-submitter work it was found alongside, and committed on its own because it is its own defect in a shared lexer. The new test's counterfactual is the flat counter: it returns -1 for a balanced body and the template-hole case goes red. --- packages/server/AGENTS.md | 2 +- packages/server/src/js-scan.js | 54 +++++++++--- .../test/scanner/match-closing-brace.test.js | 85 +++++++++++++++++++ 3 files changed, 127 insertions(+), 14 deletions(-) create mode 100644 packages/server/test/scanner/match-closing-brace.test.js diff --git a/packages/server/AGENTS.md b/packages/server/AGENTS.md index 846f0c677..6cbd5264b 100644 --- a/packages/server/AGENTS.md +++ b/packages/server/AGENTS.md @@ -86,7 +86,7 @@ with metadata, Suspense, streaming) for HTML, or `api.js` / | `component-elision.js` | Static analyser deciding which display-only component modules can be elided from the browser, plus the serve-time side-effect-import stripper. Conservative denylist of interactivity signals (single source of truth). `analyzeElision` also returns `shippedRouteModules` (#646): for each page/layout that ships whole (neither inert nor import-only), the first client-effecting blocker that pins it (a non-component on a component-free path from the module, #963, or `null` when the module's own code is the cause) plus a human `reason`. It further returns `componentVerdicts` (#1308): per component FILE, its sorted tag list, whether it ships, and the EVIDENCE that forced it (`own` / `observed` / `closure` / `render` / `import` / `unreadable`, first match wins) plus the module that did the forcing. Both are projections of what the passes already computed, never a second analysis. A reporting layer over the existing verdict, consumed by the `webjs doctor` advisory | | `elision-report.js` | `analyzeAppElision(appDir)` (#646, #1308): builds the module graph + runs `analyzeElision` ONCE, returning the WHOLE verdict as a sorted, app-relative, JSON-serializable `{ analysed, skipped, components, routeModules, orphans, summary }`. Both directions: which component modules the browser never downloads and why each shipped one ships, which page/layout is inert / import-only / shipped, and every orphan class that gets no verdict at all. Consumed by `webjs elision` (plus `--json`), the MCP `list_elision` tool, and BOTH `webjs doctor` elision checks, which share one call so the graph is built once. `skipped` names why nothing was analysed (`no-app` / `elide-off` / `unanalysable`), and the report caches nothing (every consumer runs once and exits). A reporting layer over the analysis, NOT a build (webjs is no-build) | | `elision-differential.js` | The differential primitives shared by the framework's own guard and the app-facing one (#1308): `maskJsSet(html)` (the ONE definition of the JS-loaded set, so `test/elision/differential-elision.test.js` and `webjs elision --verify` can never disagree about what the invariant means) and `staticPageRoutes(table)` (the dynamic-free render corpus). A leaf: no filesystem, no module graph | -| `js-scan.js` | Shared lexical scanners (`redactStringsAndTemplates`, `redactToPlaceholders`, `extractWebComponentClassBodies`, `matchClosingBrace`) used by `check.js`, `component-scanner.js`, and `component-elision.js`. `redactToPlaceholders` (#634) masks comments and replaces each string / template body with a `__STR___` placeholder (originals returned in a `literals` array, `${...}` holes scanned as code), so the component scanner and the elision import / side-effect scanners see a real top-level `register(...)` / `import` while an identical token shown inside a code-sample string is inert | +| `js-scan.js` | Shared lexical scanners (`redactStringsAndTemplates`, `redactToPlaceholders`, `extractWebComponentClassBodies`, `matchClosingBrace`) used by `check.js`, `component-scanner.js`, and `component-elision.js`. **`matchClosingBrace` keeps a STACK of frames, not one flat depth counter** (#1307): a `${` hole is a code context nested inside a template, so it pushes its own frame and the `}` that closes it pops back into the template instead of counting toward the block being matched. The flat counter incremented at `${` and never decremented (the closing `}` arrived while still in template state), so a class body containing an interpolated template was unmatchable. It stayed latent because every caller passes a position-preserving MASK in which holes are already blanked, so none ever fed it a `${`. `redactToPlaceholders` (#634) masks comments and replaces each string / template body with a `__STR___` placeholder (originals returned in a `literals` array, `${...}` holes scanned as code), so the component scanner and the elision import / side-effect scanners see a real top-level `register(...)` / `import` while an identical token shown inside a code-sample string is inert | | `fs-walk.js` | Async recursive directory walker | | `logger.js` | `defaultLogger` (JSON-shaped in prod, pretty in dev) | diff --git a/packages/server/src/js-scan.js b/packages/server/src/js-scan.js index 7b4e6e2a0..d307f5771 100644 --- a/packages/server/src/js-scan.js +++ b/packages/server/src/js-scan.js @@ -632,28 +632,46 @@ export function matchClosingParenthesis(s, start) { * `}` inside `'…'`, `"…"`, or backtick templates don't decrement depth. * Returns -1 if no balanced brace is found. * + * A template hole is a CODE context nested inside a template, not a brace in + * the enclosing block, so it gets its own frame on the stack: `${` pushes, + * and the `}` that returns that frame to depth zero pops back into the + * template rather than counting toward the block being matched. An earlier + * version incremented the outer depth at `${` and then never decremented it + * (the closing `}` arrived while still in template state), so depth could + * never return to zero and a class body holding `` html`…${x}…` `` was + * unmatchable. Every caller passes a masked source in which holes are already + * blanked, so the bug is invisible until one passes raw source. + * * @param {string} s * @param {number} start */ export function matchClosingBrace(s, start) { - let depth = 1; + // Innermost first. `tpl` frames are template literals (no brace counting); + // `!tpl` frames are code, each with its own depth. + /** @type {Array<{ tpl: boolean, depth: number }>} */ + const stack = [{ tpl: false, depth: 1 }]; let i = start; - let str = ''; // '', "'", '"', or backtick while (i < s.length) { + const top = stack[stack.length - 1]; const c = s[i]; - if (str) { + if (top.tpl) { if (c === '\\') { i += 2; continue; } - if (c === str) str = ''; - else if (str === '`' && c === '$' && s[i + 1] === '{') { - // template hole, count its closing `}` toward our brace depth. - depth++; - i += 2; - continue; - } + if (c === '`') { stack.pop(); i++; continue; } + if (c === '$' && s[i + 1] === '{') { stack.push({ tpl: false, depth: 1 }); i += 2; continue; } i++; continue; } - if (c === "'" || c === '"' || c === '`') { str = c; i++; continue; } + if (c === "'" || c === '"') { + i++; + while (i < s.length) { + if (s[i] === '\\') { i += 2; continue; } + const d = s[i]; + i++; + if (d === c || d === '\n') break; // closed, or unterminated at EOL + } + continue; + } + if (c === '`') { stack.push({ tpl: true, depth: 0 }); i++; continue; } if (c === '/' && s[i + 1] === '/') { // line comment while (i < s.length && s[i] !== '\n') i++; continue; @@ -664,8 +682,18 @@ export function matchClosingBrace(s, start) { i += 2; continue; } - if (c === '{') depth++; - else if (c === '}') { depth--; if (depth === 0) return i; } + if (c === '{') { top.depth++; i++; continue; } + if (c === '}') { + top.depth--; + // Depth zero closes this frame: the outermost one is the answer, an inner + // one is a template hole ending and hands control back to its template. + if (top.depth === 0) { + if (stack.length === 1) return i; + stack.pop(); + } + i++; + continue; + } i++; } return -1; diff --git a/packages/server/test/scanner/match-closing-brace.test.js b/packages/server/test/scanner/match-closing-brace.test.js new file mode 100644 index 000000000..653bec6bf --- /dev/null +++ b/packages/server/test/scanner/match-closing-brace.test.js @@ -0,0 +1,85 @@ +/** + * `matchClosingBrace`: balance a `{` against its `}` across strings, comments, + * regex-free JS, and nested template literals. + * + * The template-hole case is the one worth a dedicated file. A hole is a CODE + * context nested inside a template, not a brace in the enclosing block. The + * previous implementation kept ONE flat depth counter and incremented it at + * `${`, but the matching `}` arrived while the scanner was still in template + * state, so nothing ever decremented it. Depth could not return to zero and a + * class body containing an interpolated template was unmatchable. + * + * It stayed invisible because every caller passes a position-preserving MASK + * in which holes are already blanked, so no caller ever fed it a `${`. That is + * exactly the kind of latent defect a shared lexer should not be carrying, and + * the counterfactual below pins the fix rather than the symptom. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { matchClosingBrace } from '../../src/js-scan.js'; + +/** Index of the `}` that closes the `{` at index 0. */ +function close(src) { + assert.equal(src[0], '{', 'fixture must start at the opening brace'); + return matchClosingBrace(src, 1); +} + +test('matches a plain nested block', () => { + const src = '{ a { b } }'; + assert.equal(close(src), src.length - 1); +}); + +test('walks PAST a template hole, which the flat depth counter could not', () => { + // The regression case. With one counter this returns -1, because `${` + // incremented a depth that its own `}` never decremented. + const src = '{ render() { return html`

${x}

`; } }'; + assert.equal(close(src), src.length - 1); +}); + +test('handles several holes, and a hole holding braces of its own', () => { + for (const src of [ + '{ html`${a}${b}` }', + '{ html`${ { k: 1 } }` }', + '{ html`${ items.map((i) => html`
  • ${i}
  • `) }` }', + ]) { + assert.equal(close(src), src.length - 1, src); + } +}); + +test('a brace inside a string or a template TEXT run is not counted', () => { + for (const src of [ + '{ const a = "}"; }', + "{ const a = '}'; }", + '{ const a = `}`; }', + '{ const a = "${x}"; }', + ]) { + assert.equal(close(src), src.length - 1, src); + } +}); + +test('a brace inside a comment is not counted', () => { + for (const src of [ + '{ // }\n }', + '{ /* } */ }', + '{ /* ` */ }', + ]) { + assert.equal(close(src), src.length - 1, src); + } +}); + +test('an escaped quote does not end the string early', () => { + const src = '{ const a = "\\"}"; }'; + assert.equal(close(src), src.length - 1); +}); + +test('an unterminated single-quoted string stops at the newline, not at EOF', () => { + // A JS string cannot span a raw newline, so treating one as still-open would + // swallow the rest of the file and report no match for a perfectly balanced + // block. + const src = "{ const a = 'oops\n }"; + assert.equal(close(src), src.length - 1); +}); + +test('returns -1 when there is genuinely no balanced brace', () => { + assert.equal(close('{ a { b }'), -1); +}); From 58ae261a1e763a2c779bd0790b0ea2a0b0b1a22c Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 7 Aug 2026 14:36:44 +0530 Subject: [PATCH 05/13] feat: report at submit time when a submission cannot deliver its action The renderer deliberately stopped refusing a plain submitter's own formmethod and formenctype, because native HTML defines what those mean and an author who typed one meant it. That leaves an honest gap: the shape is also what a mistake looks like, and nothing else in the pipeline can tell the two apart. Submit time can, because it is the only moment the resolved method, the resolved enctype and whether a bound identity is actually in the body all exist together. So a dev-only console.error reports it there, once per shape, and is silent in production where the visitor could not act on it anyway. Placed BEFORE the text/plain bail rather than after. Putting it after made the text/plain branch dead code, which a test caught: that is precisely the case where the router declines the submission, both paths are answered with a 405, and the author gets no other signal. The enctype test is `text/plain` alone, NOT the renderer's parseable-enctype allowlist. `enctype` is an enumerated attribute whose missing and invalid value defaults are both urlencoded, so `enctype="nonsense"` submits a parseable body and the action runs; testing the allowlist would report that working form as broken. Two of the new tests initially passed for the wrong reason. A `submit` listener on the container bubbles BEFORE the router's document-level one, so it set defaultPrevented and onSubmit returned at its first line, meaning the router never made the decision being measured. The suite's nav guard already handles this correctly by listening on window bubble, and its own docs warn about the capture-phase version of the same mistake. --- packages/core/src/router-client.js | 72 ++++++++++- .../browser/form-action-submit.test.js | 114 ++++++++++++++++-- 2 files changed, 175 insertions(+), 11 deletions(-) diff --git a/packages/core/src/router-client.js b/packages/core/src/router-client.js index e3d8fe9ed..67cfcf788 100644 --- a/packages/core/src/router-client.js +++ b/packages/core/src/router-client.js @@ -8,6 +8,7 @@ import './webjs-frame.js'; // live-channel `connectWS` handler. import './webjs-stream.js'; import { renderStream } from './webjs-stream.js'; +import { FORM_ACTION_FIELD } from './form-action.js'; // Register (the element-level streaming boundary, #471) so it // is layout-neutral and available for the progressive soft-nav streaming apply. import './webjs-suspense.js'; @@ -876,6 +877,15 @@ function onSubmit(e) { // method ignores the enctype entirely, per the form-submission algorithm. const enctype = getSubmitEnctype(form, submitter); const isSafeMethod = method === 'get' || method === 'head'; + + // The dev report runs BEFORE the bail below, not after it. `text/plain` is + // precisely the case where the router declines the submission, so a guard + // placed after the bail would be dead code for the one shape that most needs + // reporting: both paths are then answered with a 405 and the author gets no + // other signal. Observational, and silent in production. + const rawBody = buildSubmitFormData(form, submitter); + warnIfActionSubmissionCannotDeliver(form, submitter, method, rawBody); + if (!isSafeMethod && enctype === 'text/plain') return; const action = getSubmitAction(form, submitter); @@ -885,7 +895,7 @@ function onSubmit(e) { if (url.origin !== location.origin) return; if (NON_HTML_EXTENSIONS.test(url.pathname)) return; - const body = encodeSubmitBody(buildSubmitFormData(form, submitter), enctype); + const body = encodeSubmitBody(rawBody, enctype); e.preventDefault(); // Resolve the target frame for the submit, same precedence as a link: @@ -1134,10 +1144,66 @@ function resolveTargetFrameId(trigger) { */ const warnedKeys = new Set(); /** @param {string} key @param {string} message */ -function warnOnce(key, message) { +function warnOnce(key, message, level = 'warn') { if (warnedKeys.has(key)) return; warnedKeys.add(key); - if (typeof console !== 'undefined' && console.warn) console.warn(message); + if (typeof console === 'undefined') return; + const fn = level === 'error' ? console.error : console.warn; + if (fn) fn.call(console, message); +} + +/** + * DEV-ONLY: report at submit time when a submission is carrying a bound + * action's identity it cannot actually deliver (#1307). + * + * This is the backstop for the shapes the renderer deliberately stopped + * refusing. A PLAIN ` `, container); - const btn = container.querySelector('button'); - // Stop the native submission the bail deliberately allows, so the test - // page is not navigated away. The router runs on the BUBBLE phase, so a - // capture-phase listener here would pre-empt what is being measured. - const stop = (e) => e.preventDefault(); - container.addEventListener('submit', stop); - btn.click(); + // The native submission this bail deliberately allows is cancelled by + // the suite's nav guard, which listens on WINDOW bubble, i.e. after the + // router's own document-bubble listener. A listener on `container` would + // run BEFORE the router and set `defaultPrevented`, so `onSubmit` would + // return at its first line and this test would pass without the router + // ever making the decision it claims to measure. + container.querySelector('button').click(); await tick(); - container.removeEventListener('submit', stop); assert.equal(calls.length, 0, 'the router did not take it'); } finally { teardown(); } }); + + // ------------------------------------------------------------------------- + // #1307: the dev-time submit guard. + // + // The renderer deliberately stopped refusing a PLAIN submitter's own + // `formmethod` / `formenctype`, because native HTML defines what those mean + // and the author wrote them on purpose. That leaves one honest gap: the + // shape is also what a mistake looks like. Submit time is the only moment + // the resolved method, the resolved enctype, and whether a bound identity is + // actually in the body all exist together, so the report happens there. + // + // Observational by construction: it runs before `preventDefault` and changes + // nothing about the submission. + // ------------------------------------------------------------------------- + + function captureErrors(fn) { + const orig = console.error; + const seen = []; + console.error = (...a) => { seen.push(a.join(' ')); }; + try { return fn(seen); } finally { console.error = orig; } + } + + test('a bound identity submitted as GET logs once, and still submits', async () => { + setup(okHtml); + try { + await captureErrors(async (seen) => { + render(html` +
    + + +
    + `, container); + container.querySelector('button').click(); + await tick(); + assert.ok( + seen.some((m) => /never runs/.test(m)), + `expected a submit-time console.error, saw: ${JSON.stringify(seen)}`, + ); + }); + } finally { teardown(); } + }); + + test('the guard stays silent for a form carrying no bound identity', async () => { + // An ordinary hand-written form is not this feature's business, and a + // console error on every plain GET form would be pure noise. + setup(okHtml); + try { + await captureErrors(async (seen) => { + render(html` +
    + + +
    + `, container); + container.querySelector('button').click(); + await tick(); + assert.equal(seen.length, 0, `expected silence, saw: ${JSON.stringify(seen)}`); + }); + } finally { teardown(); } + }); + + test('the guard fires for text/plain but NOT for an invalid enctype', async () => { + // `enctype` is an enumerated attribute whose invalid-value default is + // urlencoded, so `nonsense` submits a perfectly parseable body and the + // action runs. Testing against the renderer's parseable-enctype allowlist + // instead of `text/plain` alone would report that working form as broken. + setup(okHtml); + try { + await captureErrors(async (seen) => { + render(html` +
    + + +
    + `, container); + container.querySelector('button').click(); + await tick(); + assert.equal(seen.length, 0, `an invalid enctype is urlencoded and works, saw: ${JSON.stringify(seen)}`); + }); + } finally { teardown(); } + + setup(okHtml); + try { + await captureErrors(async (seen) => { + render(html` +
    + + +
    + `, container); + container.querySelector('button').click(); + await tick(); + assert.ok( + seen.some((m) => /cannot parse/.test(m)), + `expected the text/plain report, saw: ${JSON.stringify(seen)}`, + ); + }); + } finally { teardown(); } + }); }); From aab7b1e46e9149e8c3951f682ad5ec9765a6f49e Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 7 Aug 2026 14:37:50 +0530 Subject: [PATCH 06/13] fix: build the submission body after the cheap bails, not before The previous commit moved `buildSubmitFormData` ahead of the origin and file-extension checks so the dev report could precede the text/plain bail. That reached the FormData construction for submissions the router ignores, and two pre-existing unit tests went red: `new FormData(formElement)` throws under the linkedom harness those tests run in. Move the text/plain bail down instead. The order is now the cheap bails, then the body, then the report, then the bail, which keeps the report ahead of the bail (the reason for the original move) without building a body for a submission that was never going to be routed. I committed the previous change with those two tests already failing, because the verifying command was chained with `&&` on a grep that succeeded whatever the tally said. The tally was in the output and I did not read it. --- packages/core/src/router-client.js | 38 ++++++++++++++++-------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/core/src/router-client.js b/packages/core/src/router-client.js index 67cfcf788..456aec28a 100644 --- a/packages/core/src/router-client.js +++ b/packages/core/src/router-client.js @@ -867,27 +867,9 @@ function onSubmit(e) { const method = getSubmitMethod(form, submitter); if (method === 'dialog') return; - // #1307: `text/plain` is a legal native encoding the server cannot parse - // (`looksLikeFormSubmission` accepts multipart and urlencoded only), and - // there is no honest way to send it over `fetch` and have the response mean - // anything. Bail to the browser so BOTH paths do the same thing, rather than - // silently sending multipart, which is what made the same form behave one way - // with JS and another way without it. Turbo enumerates this encoding and then - // sends FormData anyway, which is the divergence being avoided here. A safe - // method ignores the enctype entirely, per the form-submission algorithm. const enctype = getSubmitEnctype(form, submitter); const isSafeMethod = method === 'get' || method === 'head'; - // The dev report runs BEFORE the bail below, not after it. `text/plain` is - // precisely the case where the router declines the submission, so a guard - // placed after the bail would be dead code for the one shape that most needs - // reporting: both paths are then answered with a 405 and the author gets no - // other signal. Observational, and silent in production. - const rawBody = buildSubmitFormData(form, submitter); - warnIfActionSubmissionCannotDeliver(form, submitter, method, rawBody); - - if (!isSafeMethod && enctype === 'text/plain') return; - const action = getSubmitAction(form, submitter); /** @type {URL} */ let url; try { url = new URL(action, location.href); } @@ -895,6 +877,26 @@ function onSubmit(e) { if (url.origin !== location.origin) return; if (NON_HTML_EXTENSIONS.test(url.pathname)) return; + // Built once, after the cheap bails (a submission the router ignores should + // not pay for a FormData) and BEFORE the text/plain bail below. That order + // matters for the dev report: `text/plain` is precisely the case where the + // router declines the submission, so reporting after the bail would be dead + // code for the one shape that most needs it, since both paths are then + // answered with a 405 and the author gets no other signal. + const rawBody = buildSubmitFormData(form, submitter); + // Observational, and silent in production. Runs before `preventDefault`. + warnIfActionSubmissionCannotDeliver(form, submitter, method, rawBody); + + // #1307: `text/plain` is a legal native encoding the server cannot parse + // (`looksLikeFormSubmission` accepts multipart and urlencoded only), and + // there is no honest way to send it over `fetch` and have the response mean + // anything. Bail to the browser so BOTH paths do the same thing, rather than + // silently sending multipart, which is what made the same form behave one + // way with JS and another way without it. Turbo enumerates this encoding and + // then sends FormData anyway, which is the divergence being avoided here. A + // safe method ignores the enctype entirely, per the submission algorithm. + if (!isSafeMethod && enctype === 'text/plain') return; + const body = encodeSubmitBody(rawBody, enctype); e.preventDefault(); From c93b9eb0d60c7c7ac95fdb164227a7ffdaa5bdab Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 7 Aug 2026 14:46:24 +0530 Subject: [PATCH 07/13] feat: report a form that submitted nowhere through onError Two detect-only diagnostics, each with an err.code an app can group on. WEBJS_FORM_ACTION_MISSING: a parseable form body carrying no identity, which is answered with a 405 and otherwise leaves nothing but an anonymous status in an access log. WEBJS_FORM_SUBMITTED_AS_GET: a page GET carrying the reserved field in its query string, meaning a submission holding a bound action's identity went out as a GET, so the action never ran and the page still returned 200. Nothing in the framework puts that field in a url. A bound submitter now carries its own formmethod="post", so what reaches this is an explicit formmethod="get" or method="get" the author wrote and the renderer deliberately honours. It is the production counterpart to the dev-time console error the router logs at submit time, and it is why honouring the override rather than refusing it does not mean shipping a silent failure. Both keep rendering their normal response. Answering a GET differently because of a query parameter would hand any visitor a way to turn any page into an error. Both carry field NAMES and never values, and both dedupe per process on code plus method plus matched ROUTE with a 256-entry cap, because either is reachable unauthenticated and an uncapped report is a free amplifier into a paid sink. Keying on the route pattern rather than the pathname is what stops crafted urls on a dynamic route from filling the cap and silencing the diagnostics. Applying the original patch with `git apply --3way` silently reverted unrelated newer work in dev.js, including #1308's orphan warning and the elision verdict fields, because it was computed against an older main. One test caught it. The hunk is hand-applied instead, threading hasOnError and logger through handleCore's context rather than reaching for a binding that is not in scope there. --- .agents/skills/webjs/references/built-ins.md | 4 +- packages/server/AGENTS.md | 2 +- packages/server/src/dev.js | 23 ++- packages/server/src/form-dispatch.js | 172 +++++++++++++++++- .../server/test/routing/form-dispatch.test.js | 137 ++++++++++++++ website/app/docs/deployment/page.ts | 4 +- website/app/docs/server-actions/page.ts | 3 +- website/app/docs/troubleshooting/page.ts | 11 +- 8 files changed, 341 insertions(+), 15 deletions(-) diff --git a/.agents/skills/webjs/references/built-ins.md b/.agents/skills/webjs/references/built-ins.md index 3aadfb86c..65edf7396 100644 --- a/.agents/skills/webjs/references/built-ins.md +++ b/.agents/skills/webjs/references/built-ins.md @@ -231,9 +231,9 @@ Two guarantees worth knowing. A result that could not check (a network or toolch Wired at the single response funnel, covering pages, routes, actions, and assets uniformly. -- **Access log.** One structured `info` line per handled request (`method`, `path`, `status`, `durationMs`, `requestId`, plus a dev-only `seed` field on a page render carrying the SSR action-seeding counters, #1309). Never logs bodies or secrets; framework `/__webjs/*` traffic is suppressed. +- **Access log.** One structured `info` line per handled request (`method`, `path`, `status`, `durationMs`, `requestId`). Never logs bodies or secrets; framework `/__webjs/*` traffic is suppressed. - **Request id.** Each request gets a `crypto.randomUUID()` correlation id, set as `X-Request-Id` (honoring a trusted inbound one) and readable server-side with `requestId()` from `@webjsdev/server` (returns `null` outside a request scope). -- **`onError` hook.** Register via `createRequestHandler({ onError })` or `startServer({ onError })`. Called with `(error, { request, requestId, phase })` on any caught pipeline error, before the sanitized response is sent. Best-effort (a throwing hook is ignored), purely additive (the sanitized 500 / action digest is unchanged). Point it at Sentry or an APM. +- **`onError` hook.** Register via `createRequestHandler({ onError })` or `startServer({ onError })`. Called with `(error, { request, requestId, phase })` on any caught pipeline error, before the sanitized response is sent. Best-effort (a throwing hook is ignored), purely additive (the sanitized 500 / action digest is unchanged). Point it at Sentry or an APM. It also carries two framework DIAGNOSTICS that are not request failures, each with an `err.code` to group or filter on, both under `phase: 'action'`: `WEBJS_FORM_SUBMITTED_AS_GET` (a page GET carrying the reserved `__webjs_action` field in its query string, so a submission holding a bound action's identity went out as a GET and the action never ran, #1307; a bound submitter carries its own `formmethod="post"`, so what reaches this is an explicit `formmethod="get"` / `method="get"` the author wrote and the renderer honours rather than refuses) and `WEBJS_FORM_ACTION_MISSING` (a PARSEABLE form body carrying no identity, the 405; an `enctype="text/plain"` submission is answered before its body is read, so it stays a bare 405). Both are detect-only, so the 200 and the 405 are unchanged; both carry `method`, `pathname`, and for the second the submitted field NAMES, never the values; and both are deduplicated per process on the code, the method, and the matched ROUTE (not the request pathname, so crafted urls on a dynamic route cannot exhaust the 256-entry cap and silence the diagnostics), since either is reachable by an unauthenticated request and an uncapped report would be a free amplifier into a paid sink. ```ts const app = await createRequestHandler({ diff --git a/packages/server/AGENTS.md b/packages/server/AGENTS.md index 6cbd5264b..119278edd 100644 --- a/packages/server/AGENTS.md +++ b/packages/server/AGENTS.md @@ -37,7 +37,7 @@ with metadata, Suspense, streaming) for HTML, or `api.js` / | `dev-error.js` | Dev error overlay frame builder (#264). `buildDevErrorFrame(error, { kind, appDir, file?, line?, hint?, url? })` returns a JSON-serializable frame (message, parsed `file`/`line`/`column`, a source `codeFrame`, an optional `hint`, and an optional `url`); `parseStackLocation(stack, appDir)` finds the first app frame (preferring non-`node_modules`, splitting off the dev loader's `?t=` cache-bust query); `readCodeFrame(file, line, column)` reads the source excerpt with a `>` line marker + a caret. `url` (#1047) is the request URL that produced the frame, `null` when absent: only a `render` frame carries one, and it is what lets the browser overlay refuse a frame for a page the tab is not viewing. A `ts-strip` / `rebuild` frame passes none and stays unscoped, because it describes a still-broken build rather than one page. PURE (the only side effect is a guarded source read) and DEV-ONLY by the caller's contract, so no path / source is built in prod | | `dev-overlay.js` | The BROWSER half of the dev error overlay (#264), a browser-safe module (no node imports) that `reloadClientJs` inlines verbatim after an `export`-keyword strip, so the browser test drives the exact shipping code. `renderDevOverlay(frame, currentPath?)` builds the card with `createElement` + `textContent` ONLY (never `innerHTML`, so a hostile message / path / code frame is inert text); `dismissDevOverlay()` takes it down and forgets any held frame, which is what the Dismiss button calls. **URL scope (#1047):** a url-stamped `render` frame for a different path than `currentPath` (default `location.pathname + location.search`, compared encoding-tolerantly) is NOT rendered. The gate runs BEFORE any removal, so a refused frame neither paints nor wipes a live `rebuild` / `ts-strip` overlay. It cannot simply drop the frame either, because the SSE frame is pushed during the render, before the navigation response is sent, so it arrives while `location` is still the old page: a refused frame goes to a PENDING slot that `syncDevOverlayToLocation(currentPath?)` re-evaluates on the next navigation, which renders it only if the url matches AND it arrived during the navigation now finishing, and drops it otherwise (consume-once, so retention is bounded with no timer). The second half of that condition is what stops a frame held from an IDLE render (another tab's page, a background fetch of some other url, never a link prefetch since that reports nothing) painting on a later visit to that url, when the page may render fine; `markDevOverlayNavStart()` is the nav-start marker it counts. `installDevOverlayNavSync(opts?)` wires all of it to the client router's `webjs:navigate` (applied nav) + `popstate` (a snapshot-cache restore returns before `webjs:navigate`) + `webjs:before-cache` (the nav START, since `snapshotCurrent` runs at the top of every navigation and form submission), and returns an uninstall thunk. Two things keep a snapshot copy of the overlay from becoming an undismissable card (the router's tier-4 popstate restore `replaceChildren`s the body from cached HTML, and a parsed copy is not the node this module holds, so nothing could remove it and its Dismiss button has no listener). `before-cache` DETACHES the overlay across the router's synchronous `outerHTML` read and re-attaches it a microtask later, so the cached HTML carries none. That is what covers the hard ordering: under an opt-in view transition the router defers the body swap PAST the sync, so a copy the swap inserts would arrive after any sweep had already run. The sync additionally SWEEPS any `[data-webjs-error-overlay]` element it does not own, as the backstop for a copy from anywhere else (a snapshot cached before this client installed its listeners, or any other wholesale body replacement). What `before-cache` must NOT do is strip the overlay for good, which the event's own contract invites: it fires on EVERY navigation, so that would tear a `rebuild` / `ts-strip` overlay off the page on any link click while the build was still broken. `packages/core` is untouched: the fix rides existing events | | `frame-render.js` | Server-side `` subtree extraction (#253). `requestedFrameId(req)` reads the `x-webjs-frame` header (null when absent, the normal full-page path); `extractFrameSubtree(html, id)` returns the `...` slice from rendered HTML verbatim (so byte-equivalent by construction), balancing nested `` tags and reading the `id` attribute (not a substring match), or null when the id is absent. Used by `ssr.js`'s frame-render branch | -| `form-dispatch.js` | Form-submission dispatch (#1155, replacing the page `action` export of #244): `runFormAction` verifies Origin, parses the bounded form body, resolves the `__webjs_action` identity, runs the action's declared `validate` + `middleware`, evicts its `invalidates` tags, and maps the `ActionResult` to a response (303 PRG on success, 422 re-render with `actionData` on failure, honoring thrown `redirect()`/`notFound()`/`forbidden()`/`unauthorized()`). An action that returns a `Response` DIRECTLY (e.g. a content-negotiated `streamResponse`, #248) is honored verbatim; a STREAMED return is refused (a submission has no frame consumer). A submission carrying no identity is a 405, an unresolvable hash is a 422 re-render with `ACTION_SKEW_MESSAGE` plus the typed values, and a module that throws at import is a sanitized 500 (never mislabelled as skew). `dev.js` routes EVERY non-GET/HEAD page request here, wrapped in the page's segment middleware | +| `form-dispatch.js` | Form-submission dispatch (#1155, replacing the page `action` export of #244): `runFormAction` verifies Origin, parses the bounded form body, resolves the `__webjs_action` identity, runs the action's declared `validate` + `middleware`, evicts its `invalidates` tags, and maps the `ActionResult` to a response (303 PRG on success, 422 re-render with `actionData` on failure, honoring thrown `redirect()`/`notFound()`/`forbidden()`/`unauthorized()`). An action that returns a `Response` DIRECTLY (e.g. a content-negotiated `streamResponse`, #248) is honored verbatim; a STREAMED return is refused (a submission has no frame consumer). A submission carrying no identity is a 405, an unresolvable hash is a 422 re-render with `ACTION_SKEW_MESSAGE` plus the typed values, and a module that throws at import is a sanitized 500 (never mislabelled as skew). `dev.js` routes EVERY non-GET/HEAD page request here, wrapped in the page's segment middleware. **Two detect-only `onError` signals (#1307):** `WEBJS_FORM_ACTION_MISSING` for a parseable form body carrying no identity (the 405 above), and `WEBJS_FORM_SUBMITTED_AS_GET` for a page GET carrying the reserved field in its QUERY STRING, which means a submission holding a bound action's identity went out as a GET so the action never ran. Nothing in the framework puts that field in a url, and a bound submitter now carries its own `formmethod="post"`, so what reaches the second signal is an explicit `formmethod="get"` / `method="get"` the author wrote and the renderer deliberately honours; it is the production counterpart to the dev-time client guard. Both keep rendering their normal response (answering differently on a query parameter would let any visitor turn any page into a different status), carry field NAMES and never values, and dedupe per process on code plus method plus matched ROUTE with a 256-entry cap. Keying on the route PATTERN rather than the pathname is load-bearing: keyed on the pathname a dynamic route yields unbounded keys, so a few hundred crafted requests would fill the cap and permanently silence the diagnostics | | `form-action-identity.js` | The `/` identity a bound form submits (#1155). `resolveActionIdentity` maps a real action function to its identity via the always-on `'use server'` load-hook registry (`action-seed.js`), with a module-scan fallback only on a runtime that installs no hook; `lookupActionIdentity` maps a submitted identity back to `{ file, fnName, module }`, distinguishing an unknown hash (skew) from an unknown export (404) and from an import failure (`load-failed`). The scheme is the RPC endpoint's, so both transports resolve the same string to the same function | | `action-seed.js` | SSR action-result seeding (#472, Bun-enabled #529). `registerActionHooks({ seed, dev })` (async, `dev` gating only the determinism assertion below) installs a load hook chosen by `serverRuntime()`: Node's synchronous `module.registerHooks`, or a `Bun.plugin` `onLoad` on Bun (the glue is in `action-seed-bun.js`, dynamically imported so `Bun.*` never loads on Node). For a `'use server'` `*.server.*` module the hook returns a transparent FACADE re-exporting each function wrapped in a `Proxy` (`__actionWrap`); the faceting decision (`isSeedCandidate`) + facade source (`buildSeedFacade`) are runtime-neutral, so both runtimes emit the identical seed. (Bun's `onLoad` must return an object for every filter match, so the non-facet cases serve the raw source.) The Proxy records `(file, fn, args) -> result` into an ambient `AsyncLocalStorage` collector when one is active (a pure passthrough otherwise, so the RPC endpoint path is untouched, and any metadata attached to the function forwards through the Proxy). `collectSeeds(fn)` runs the SSR render inside a fresh collector; `buildSeedScript(collector, { dev, reason })` serializes it into an HTML-escaped `