Skip to content

Commit ec59af0

Browse files
committed
fix: clear the removed rule's premise from the surfaces it outlived
Review of the removal turned up five places still asserting, or still built for, the enclosing-form question #1307 deleted. The gallery's submit-todo comment contradicted itself four lines apart, saying the enclosing form does NOT have to be bound and then that it does, and pointed readers at the rule this branch removes. That file is scaffold payload, so it ships into every generated app. The production onError message for WEBJS_FORM_SUBMITTED_AS_GET named the enclosing form as the cause; the dev logger four lines above it was already correct, so the two disagreed on one event. render-server.js still carried the four-state form-scope comment and passed a vestigial third argument to a two-parameter render(), both left over from the same removal. check.js kept two imports whose only uses were inside the deleted function, and the now-callerless scanner's JSDoc still described the consumer it lost.
1 parent e5086c2 commit ec59af0

5 files changed

Lines changed: 27 additions & 45 deletions

File tree

gallery/modules/todo/actions/submit-todo.server.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,12 @@ import { deleteTodo } from './delete-todo.server.ts';
2121
// control's visible label), and a bound submitter cannot carry its own
2222
// `name`/`value`, which is exactly the channel `name="intent"` uses below.
2323
//
24-
// Third thing to know, and the one that fails silently: the enclosing <form>
25-
// has to be bound too, because `method="post"` and the enctype are supplied on
26-
// the form's start tag and a per-button action cannot retrofit them. The
27-
// renderer refuses an unbound form it can see, but a submitter inside a
28-
// COMPONENT is a cannot-tell (the component renders in its own pass with no
29-
// view of the host page) and binds anyway. What happens then depends on that
30-
// form: one still declaring `method="post"` works (the identity rides the
31-
// button's own name/value pair into the body), but one with no method submits as
32-
// a GET, so the identity rides the query string and the action never runs while
33-
// the page returns 200. Run `webjs check`: `submitter-needs-bound-form` finds
34-
// these across modules.
24+
// Third thing to know: no `formaction` url is emitted, because the identity
25+
// travels in the body instead. So the submission targets whatever the FORM
26+
// targets, and a form declaring `action="/x"` sends its buttons there. The
27+
// action still runs when `/x` is a PAGE route; against a `route.ts` or another
28+
// origin the identity is ignored and nothing runs, which the dev-time client
29+
// guard reports at submit time.
3530
//
3631
// With JS the component intercepts the submit and calls the underlying action
3732
// directly for the optimistic path, so this runs only with JS off.

packages/core/src/render-server.js

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -182,23 +182,6 @@ async function renderTemplate(tr, ctx) {
182182
let pendingPropAttrs = [];
183183
/** @type {string[]} */
184184
let pendingSubmitterProps = [];
185-
// Whether the tag stream is currently inside a form that BOUND an action
186-
// (#1207), as THIS scan can see it. Three states, and the third is the point:
187-
// 'bound' an enclosing <form> opened here and bound an action
188-
// 'unbound' an enclosing <form> opened here and bound nothing
189-
// 'none' there is conclusively no enclosing <form>
190-
// 'unknown' there may be one, but this scan cannot see it
191-
//
192-
// The last two look alike and must not be merged, which is what a boolean did.
193-
// A component's template is rendered by a SEPARATE pass (`injectDSD` calls
194-
// `render` on it), so a `<button formaction=${fn}>` inside a component inside
195-
// a bound form read as "no form", was refused, and in production vanished
196-
// from a page that still returned 200. That pass now says 'unknown' and the
197-
// boundness question is skipped, exactly as the client skips it when it
198-
// cannot reach the form, so both renderers are best effort in the same place
199-
// and for the same reason. A top-level scan that simply contains no form
200-
// stays 'none' and is still refused, because there the answer IS known.
201-
//
202185
let isCloseTag = false;
203186

204187
// A bound `action=${fn}` is committed at its hole, but the edits it implies
@@ -1037,14 +1020,12 @@ async function injectDSD(html, ctx, ancestors = [], dev) {
10371020
// Render the template to HTML. injectDSD recurses on the result so
10381021
// nested custom elements (e.g. <theme-toggle> inside <blog-shell>)
10391022
// get their own DSD pass.
1040-
// 'unknown' for the form scope (#1207): this is a SEPARATE render pass
1041-
// over one component's own template, driven by walking the already-emitted
1042-
// HTML, so it has no idea whether the host tag sits inside a bound
1043-
// `<form>`. Passing the default 'none' claimed there was no form at all,
1044-
// which refused a perfectly good `<button formaction=${fn}>` in a
1045-
// component inside a bound form and, because component SSR errors are
1046-
// isolated, made the button vanish from a page that still returned 200.
1047-
const rawInner = await render(tpl, ctx, 'unknown');
1023+
// This is a SEPARATE render pass over one component's own template,
1024+
// driven by walking the already-emitted HTML, so it has no idea whether
1025+
// the host tag sits inside a `<form>`. It does not need to: a bound
1026+
// submitter carries its whole submission (#1307), so nothing here asks
1027+
// about an enclosing form.
1028+
const rawInner = await render(tpl, ctx);
10481029

10491030
if (isShadow) {
10501031
// Shadow DOM: native <slot> stays as-is in the DSD template. The

packages/server/src/check.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@ import {
66
redactToPlaceholders,
77
extractWebComponentClassBodies,
88
matchClosingBrace,
9-
matchClosingParenthesis,
109
parsePropEntries,
1110
classifyActionHole,
1211
} from './js-scan.js';
1312
import { buildModuleGraph, transitiveDeps, resolveImport } from './module-graph.js';
14-
import { scanComponents, extractComponents } from './component-scanner.js';
13+
import { scanComponents } from './component-scanner.js';
1514
import { buildRouteTable } from './router.js';
1615
import { analyzeElision } from './component-elision.js';
1716
import { RESERVED_CONFIG } from './action-config.js';

packages/server/src/form-dispatch.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ export function reportFormSubmittedAsGet(url, req, onError, logger, dev, route)
359359
}
360360
if (!willReport) return;
361361
const err = new Error(
362-
`A form submission reached ${url.pathname} as a GET with the \`${FORM_ACTION_FIELD}\` identity in the query string, so no server action ran. The submitter's enclosing <form> binds no action.`,
362+
`A form submission reached ${url.pathname} as a GET with the \`${FORM_ACTION_FIELD}\` identity in the query string, so no server action ran. A bound submitter carries its own formmethod="post", so look for a formmethod="get" on the button that was pressed, or a method="get" on its form: a submitter's own formmethod wins by native precedence and WebJs honours it rather than refusing it.`,
363363
);
364364
/** @type {any} */ (err).code = 'WEBJS_FORM_SUBMITTED_AS_GET';
365365
/** @type {any} */ (err).method = req.method;

packages/server/src/js-scan.js

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -895,13 +895,20 @@ function isInlineStartTagHole(tagName, literalBefore) {
895895
* submitter action hole (`<button|input formaction=${...}>`) and each
896896
* custom-element start tag, the enclosing `<form>` scope at that point (#1307).
897897
*
898+
* NOTE: this currently has NO production caller. It was written for the
899+
* `submitter-needs-bound-form` check rule, which #1384 removed because its
900+
* premise was false: a bound submitter is self-sufficient, so the enclosing
901+
* form's boundness does not decide whether the action runs. The scan itself is
902+
* correct about what it reports and is kept for a future consumer, but nothing
903+
* reads it today, so treat it as unproven against real-world input.
904+
*
898905
* Only an `html`-tagged literal is entered, so `const s = '<form>'` and a `css`
899906
* or `sql` template are never read as markup. That carve-out matters: the
900907
* framework's own website renders `<form action=${fn}>` as a code SAMPLE.
901908
*
902909
* A template nested inside a CHILD-position hole INHERITS the enclosing scope,
903-
* because that is what the renderer does (`render` threads `formScope` through
904-
* arrays, `repeat`, and nested templates).
910+
* because that is where the renderer places it (through arrays, `repeat`, and
911+
* nested templates).
905912
*
906913
* One in a START-TAG hole is `'handed'`: an attribute or property value whose
907914
* placement this scan cannot speak for. Worth being exact about why, because the
@@ -922,10 +929,10 @@ function isInlineStartTagHole(tagName, literalBefore) {
922929
* the scan started in, mirroring `handleTagEnd` in `render-server.js`.
923930
*
924931
* `opensForm` reports whether ANY `<form` start tag was seen anywhere in `src`.
925-
* A caller attributing a scope-`'none'` site to this file needs it: a fragment
926-
* built into a local and spliced into a form the same file opens inherits the
927-
* SPLICE point's scope, not the file's own call-site scope, so a file that
928-
* opens a form cannot have its `'none'` sites attributed safely.
932+
* It exists for a caller that attributes a scope-`'none'` site to this file: a
933+
* fragment built into a local and spliced into a form the same file opens
934+
* inherits the SPLICE point's scope, not the file's own call-site scope, so a
935+
* file that opens a form cannot have its `'none'` sites attributed safely.
929936
*
930937
* @param {string} src
931938
* @returns {{ submitters: FormScopeSite[], tagUses: FormScopeSite[], opensForm: boolean }}

0 commit comments

Comments
 (0)