Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b61da0d
fix: refuse a function in form action=, it leaked server source
vivek7405 Jul 28, 2026
4e93870
fix: close the same form-action leak in the streaming renderer
vivek7405 Jul 28, 2026
d2f302a
refactor: drop the unreleased form-binding machinery from this branch
vivek7405 Jul 28, 2026
edbebb2
refactor: keep isFormActionAttr internal to the guard module
vivek7405 Jul 28, 2026
147b25c
fix: guard the .action property binding, the third leak path
vivek7405 Jul 28, 2026
31ddf50
docs: cover the form-action throw on the docs site, fix the action= a…
vivek7405 Jul 28, 2026
c128743
fix: close four bypasses that walked straight past the action guard
vivek7405 Jul 28, 2026
f53bb09
test: prove the action guard refuses identically on Node and Bun
vivek7405 Jul 28, 2026
9725114
fix: stop the guard crashing on a cyclic array and refusing event bin…
vivek7405 Jul 28, 2026
a8c8247
test: restore the dropped quoted-bool case, name the clause each test…
vivek7405 Jul 29, 2026
5baa930
test: cover the streaming native-prop clause in the rendering suite
vivek7405 Jul 29, 2026
0080dff
test: correct the clause map, and stop hand-rolling a case that fits …
vivek7405 Jul 29, 2026
48fce50
test: isolate the browser hosts so a failure names the right test
vivek7405 Jul 29, 2026
4dce2f3
test: pin the scope boundary, and stop duplicating the clause map
vivek7405 Jul 29, 2026
7b292e7
test: pin the scope boundary on every renderer, not just the buffered…
vivek7405 Jul 29, 2026
71c1d15
test: assert what the stream actually flushed, not what survived the …
vivek7405 Jul 29, 2026
32a23af
test: inline the leak marker so the assertions cannot go tautological
vivek7405 Jul 29, 2026
5af0027
test: say what the streaming test actually proves, and why Bun's sour…
vivek7405 Jul 29, 2026
6abe618
test: state precisely when a pre-refusal chunk reaches the consumer
vivek7405 Jul 29, 2026
cab06a8
fix: read the stream the way a real consumer does, and stop overstati…
vivek7405 Jul 29, 2026
034b936
fix: pin the worst-case stream consumer, and stop calling outer value…
vivek7405 Jul 29, 2026
02e6729
fix: make an undersized drain burst fail loudly instead of reporting …
vivek7405 Jul 29, 2026
bf4a914
fix: correct the stream mechanism, the burst boundary, and the foldin…
vivek7405 Jul 29, 2026
3fb056b
test: say that the streaming drain proves nothing on the shipped path
vivek7405 Jul 29, 2026
1bb0337
docs: stop stating a rule for Bun's constant folding, there isn't a s…
vivek7405 Jul 29, 2026
1e43a3d
fix: contain a throw from an async render commit
vivek7405 Jul 29, 2026
352c348
test: pin the guard's case folding, the one branch nothing covered
vivek7405 Jul 29, 2026
a77bdbe
docs: note the action= carve-out where the binding rules are stated
vivek7405 Jul 29, 2026
f35ca6d
docs: correct the .action carve-out and drop the process narration
vivek7405 Jul 29, 2026
4abec56
test: prove commit containment in a real browser, not just linkedom
vivek7405 Jul 29, 2026
3478071
fix: refuse a function .prop only where the property reflects
vivek7405 Jul 29, 2026
3677176
fix: settle the update cycle when the pending commit rejects
vivek7405 Jul 29, 2026
eed0e7e
docs: state what the isolation swallows, and the comment hole it misses
vivek7405 Jul 29, 2026
9331eef
fix: do not let a superseded cycle's late rejection clobber the DOM
vivek7405 Jul 29, 2026
fbfbf1c
docs: say the stringify leak is not special to comments
vivek7405 Jul 29, 2026
884ac75
fix: release the update cycle even if error reporting throws
vivek7405 Jul 29, 2026
ac9afca
docs: correct two stale counts in the guard's own comments
vivek7405 Jul 29, 2026
1ea8146
docs: drop the tracked-separately claim for the Suspense silence
vivek7405 Jul 29, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/webjs/references/components.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ Three decoupled concerns, do not conflate them.
2. **The client re-fetch default is stale-while-revalidate.** When a prop or dependency change re-runs `async render()`, the previous content stays until the new render resolves. No blank, no flash, no user code.
3. **`renderFallback()` is the OPTIONAL re-fetch loading UI.** Shown ONLY during a client re-fetch, NEVER on first paint, and it does NOT create a server-streaming boundary.

Errors are isolated per component by default (no user code): a thrown `await` renders a component-scoped error state while siblings render, never bubbling to the route `error.ts`. Override `renderError(error)` only to customize it (dev shows the message, prod stays silent).
Errors are isolated per component by default (no user code): a thrown `await` renders a component-scoped error state while siblings render, never bubbling to the route `error.ts`. Override `renderError(error)` only to customize it (dev shows the message, prod stays silent). The boundary covers the COMMIT as well as the fetch, so a template that throws while being applied (a refused binding, a value whose `toString` throws) reaches `renderError()` too, and `updateComplete` still settles. Those two halves used to disagree: a fetch rejection was contained and a commit throw escaped as an unhandled rejection that also left `updateComplete` pending forever.

Decision rules. Use `async render()` for request-time server data that should be in the first paint (the default). Add `renderFallback()` when a client re-fetch's stale content would mislead. Use `Task` / signals for genuinely client-only data (a click, viewport, live updates). For SLOW data where blocking the first byte hurts, wrap the region in `<webjs-suspense .fallback=${html\`Loading...\`}>` to stream it (the only way to show a first-paint fallback; see `client-router-and-streaming.md`). Do NOT fetch in `connectedCallback` for data knowable server-side, and do NOT prop-drill what a leaf can fetch itself.

Expand Down
61 changes: 61 additions & 0 deletions .agents/skills/webjs/references/muscle-memory-gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,67 @@ const users = await getUsers();

There is no React `cache()`, `use()`, or `unstable_cache`. Caching is the `cache()` query helper, `export const revalidate` on a page, or `export const cache` on a GET action.

### A function in `<form action=${fn}>` is refused, not stringified

Next binds a Server Action with `<form action={createTodo}>` and React serializes the binding into hidden fields. WebJs does not read that shape. A function interpolated into `action=` is a hard render error.

The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body into the HTML every visitor downloads, including any literal inside it. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike.

What escapes is the SOURCE the runtime reports, and how much that includes depends on the runtime. The body always goes: your query shapes, your table and column names, your internal paths, and any credential written inline.

Whether an OUTER value goes with it is not something to rely on either way. `Function.prototype.toString` returns source text, so on Node a module-scope `const` the body reads appears as its identifier. Bun transpiles the module before the engine sees it and can fold that literal into the body, so the same action reports the VALUE:

```
// const VENDOR_API_KEY = 'sk_live_…'; then used as `Bearer ${VENDOR_API_KEY}`
node 26 Authorization: `Bearer ${VENDOR_API_KEY}` identifier only
bun 1.3 Authorization: "Bearer sk_live_…" the key itself
```

Do not go looking for the rule that decides when it folds. Export status, read count, declaration position, and whether the module has an import have each been measured as the deciding factor and each produced a counterexample on the same bun version, so whatever the optimizer keys on is finer than any of them. The two rows above are one measurement on two specific versions, not a per-runtime guarantee: read them as proof that the boundary moves, never as a promise that Node keeps an outer binding private.

So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, it is the only one that holds across runtimes, and it is the only one that stays true when the transpiler changes.

The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. Casing does not help either: `formAction=` and `ACTION=` fold to the same rule.

**Commenting the form out does not disable the hole.** A comment is HTML, the interpolation is JavaScript, and the renderer emits a comment's holes raw, so `<!-- <form action=${createTodo}> -->` still ships the whole action body with no throw and no log. Delete the binding instead of commenting around it. This is the one shape in this section that leaks silently, which is exactly why it is worth knowing.

It is not special to comments. `String(fn)` returns source text wherever it runs, so a bare function in a text child (`<div>${fn}</div>`) or any unclaimed attribute (`title=${fn}`) writes the same body out. Only the two form-action attribute names are refused today; treat a function anywhere else in a template as a mistake that ships, and reach for `@event=${fn}` or a custom element's `.prop=${fn}`, neither of which stringifies.

The refused and allowed shapes in full. Every "no" row is a binding that stringifies nothing, so refusing it would break working code rather than close a leak:

| Written as | Refused? | Why |
|---|---|---|
| `action=` / `formaction=` | yes | ordinary attribute, stringified into the HTML |
| `.action=` on a native form | yes | the property reflects, so the source lands in the DOM on the client |
| `.formAction=` on a button or input | yes | same reason, that is where `formAction` reflects |
| `.action=` on any other native tag | **no** | a plain expando (`<div .action=${fn}>`, `<button .action=${fn}>`), reflecting nothing, so nothing reaches the markup |
| `.action=` on a custom element | **no** | an author-defined property, not a reflected IDL attribute; a function is a legitimate value. Holds for a PLAIN prop: one declared `reflect: true` writes `String(value)` to the attribute on a path outside these commit sites, so it still emits the source |
| `?action=` | yes | never leaked, but it is meaningless, so it is refused rather than silently emitting a bare `action=""` |
| `@action=` unquoted | **no** | an event listener, and a function is exactly what one takes |
| `@action="${fn}"` quoted | yes | quoting makes it an ordinary attribute again, so it leaks |

That last row is the one to remember: quoting a binding hole turns it back into a plain attribute, which is why invariant 4 requires `@`, `.` and `?` holes to be unquoted.

`.action=${fn}` on a native form is refused during SSR too, even though the property is dropped there and nothing could leak, so a page cannot render clean on the server and then throw on hydration.

**Inside a component you may never see the error.** Per-component SSR error isolation contains the throw, so development shows an error box in place of the component and production renders it empty with the page still returning 200. A form that has silently vanished in production is this bug wearing a disguise; the message is in the server log. Nothing leaks either way.

Two things that "renders it empty" understates, both worth knowing before you go looking:

- **Anything slotted into the failing component goes with it.** The isolation replaces the element from its opening tag through its matching close, so a shell or layout component whose template holds the bad form takes the page's whole authored body with it. Put `action=${fn}` in a shared header and every page renders a 200 with an empty body, not one missing header.
- **On a route with a `loading.{js,ts}`, there is no log line either.** That wraps the page in a `Suspense` boundary, so the page body renders AFTER the 200 and the shell have been flushed, and a boundary that throws there is currently swallowed with no server log, no `onError`, and no error boundary. The visitor gets chrome and an empty body; with JS off the skeleton simply stays. That silence is a known framework gap rather than intended behaviour, so do not read the missing log line as evidence the render succeeded.

```ts
// WRONG: throws at render; it would have leaked the action's body.
import { submitFeedback } from '#modules/feedback/actions/submit-feedback.server.ts';
html`<form method="post" action=${submitFeedback}>`;
Comment thread
vivek7405 marked this conversation as resolved.
// RIGHT: omit action entirely to post to the page's own url, and handle the
// submission in that page's `action` export.
html`<form method="post">`;
```

A string stays a string: `action="/search"` and `action=${'/search'}` are unchanged. Other attributes keep their existing stringify behaviour; only `action` and `formaction` are claimed.

### `params` and `searchParams` are awaitable AND synchronously readable

Next 15/16 made `params` / `searchParams` Promises. WebJs supports BOTH, so either muscle memory is correct.
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ jobs:
# so this is the cross-shell parity proof.
- name: webjs listener parity on Bun
run: bun test/bun/listener.mjs
# The form-action leak guard (#1154) on Bun: a divergence here is a
# divergence in whether a server action's SOURCE reaches the served HTML,
# so it cannot be left to the Node suite alone. Covers both SSR state
# machines, which are independent and must be kept in step by hand.
- name: WebJs form-action guard parity on Bun
run: bun test/bun/form-action-guard.mjs
# Listener overhead reductions on Bun (#756): the out-of-band IP stamp (no
# Request clone), the buffered sync-compression fast path, the streamed-head
# non-blocking classifier, and the basePath rebuild spoof guard. Run as the
Expand Down
10 changes: 7 additions & 3 deletions examples/blog/.agents/rules/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,13 @@ self-review loop.
is never called on the server, so anything there only runs after
hydration. Initial data for components comes from the page function
(server-side fetch plus pass as attribute/property), NOT from `fetch` calls
in `connectedCallback`. For write-paths, prefer `<form action=...>` plus
server action over `fetch` plus click handler. The framework upgrades plain
forms to partial-swap submissions automatically.
in `connectedCallback`. For write-paths, prefer a plain `<form method="post">`
posting to the page's own URL, handled by that page's `action` export, over
`fetch` plus a click handler. The framework upgrades plain forms to
partial-swap submissions automatically. Never interpolate a server action
into the attribute (`<form action=${createPost}>`): that Next binding is
refused at render time, since stringifying the function would leak its
source into the HTML.
- **Client navigation is auto-magic.** Real `<a href>` and `<form action>`
get partial-swap behavior with no opt-in. Because layouts persist across
navigation, put shared chrome (sidenav, header) in `layout.ts` and
Expand Down
10 changes: 7 additions & 3 deletions examples/blog/CONVENTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -698,9 +698,13 @@ state. Only the *interactivity itself* (the +/- click, the open/close
toggle, the tab switch) requires JS.

**Default rules:**
- **Forms must work as plain HTML POSTs.** Use `<form action=…>` bound
to a server action. Never `fetch` + a JS click handler for the
happy path. The framework upgrades the form to a partial-swap
- **Forms must work as plain HTML POSTs.** Post to the page's own URL
(omit `action` entirely) and handle the submission in that page's
`action` export. Never `fetch` + a JS click handler for the happy
path. Do NOT interpolate a server action into the attribute
(`<form action=${createPost}>`): that is the Next binding, it is not
how WebJs reaches an action, and it is refused at render time because
stringifying the function would write its source into the HTML. The framework upgrades the form to a partial-swap
submission automatically when the client router is active, and with
JS disabled the same form does a full-page POST and works identically
end-to-end.
Expand Down
70 changes: 68 additions & 2 deletions packages/core/src/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,25 @@ function defaultHasChanged(a, b) {
return a !== b;
}

/**
* `String(v)` that cannot itself throw.
*
* A rejection reason is whatever the author rejected with, and not every value
* converts to a primitive: `Object.create(null)` has no `toString`, and a
* revoked Proxy throws on any operation. Coercing one in an error-reporting
* path turned a reportable failure into a second, unreported one.
*
* @param {unknown} v
* @returns {string}
*/
function safeString(v) {
try {
return String(v);
} catch {
return Object.prototype.toString.call(v);
}
}

/**
* A minimal base for HTML Custom Elements that mirrors Lit's ergonomics
* while staying JSDoc-only and no-build.
Expand Down Expand Up @@ -1176,7 +1195,7 @@ class WebComponentBase extends Base {
// (shouldUpdate=false) running during the fetch does NOT resolve
// updateComplete early: the pending commit owns the resolution.
this.__pendingAsyncCommits = (this.__pendingAsyncCommits || 0) + 1;
pendingCommit.then(() => {
const settle = () => {
// Always decrement first, even when superseded, so the in-flight
// count never leaks (a superseded cycle returns below without
// committing, but it is no longer pending).
Expand All @@ -1189,6 +1208,37 @@ class WebComponentBase extends Base {
}
// --- 7-8 + updateComplete ---
this._postCommit(changedProperties);
};
// A rejection has to settle the cycle too, or the counter stays >= 1
// forever: that both wedges `updateComplete` for this instance and
// disables the `!this.__pendingAsyncCommits` escape below for every
// later non-committing cycle. `_commitAsync` is written not to reject,
// but it is not the only thenable that reaches here: `update()` is a
// documented override point and may return one, so the handler belongs
// at this site rather than only at the one implementation of it.
pendingCommit.then(settle, (error) => {
// The token check gates the DOM write, NOT the release. A superseded
// cycle rejecting late (a discarded fetch, or the #492 abort of its own
// in-flight action surfacing as an AbortError) must not commit its
// error state over the newer render that already replaced it, which is
// the same rule both handlers inside _commitAsync follow. `settle()`
// still runs either way: it does its own token check before touching
// anything, and the decrement it owns has to happen regardless or the
// counter wedges exactly as it did before this handler existed.
// `finally`, not a trailing statement: the release must survive
// anything the reporting path throws. `String(error)` is evaluated in
// the argument expression, OUTSIDE _handleRenderError's own try/catch,
// so a rejection reason that cannot be coerced (a null-prototype
// object, a revoked Proxy) threw a TypeError straight out of this
// handler and skipped the decrement, wedging the counter in exactly
// the way this handler exists to prevent.
try {
if (token === this.__renderToken) {
this._handleRenderError(error instanceof Error ? error : new Error(safeString(error)));
}
} finally {
settle();
}
});
return;
}
Expand Down Expand Up @@ -1399,7 +1449,23 @@ class WebComponentBase extends Base {
return Promise.resolve(pending).then(
(tpl) => {
if (token !== this.__renderToken) return; // superseded by a newer render
clientRender(tpl, this._renderRoot);
// The COMMIT throws too, not just the fetch, and a sibling rejection
// handler cannot see it: `.then(onFulfil, onRejected)` never routes
// onFulfil's own throw to onRejected. Left uncaught, the returned
// promise rejected and _performRender's `.then` had no rejection
// handler, so the error escaped as an unhandled rejection,
// renderError() never ran, __pendingAsyncCommits never decremented
// (wedging it >= 1 forever, which also disables the non-committing
// cycle's _resolveUpdate escape), and updateComplete never settled.
// The sync path routes an identical throw to the same boundary, so
// this is what makes the two paths agree.
try {
clientRender(tpl, this._renderRoot);
} catch (commitError) {
this._handleRenderError(
commitError instanceof Error ? commitError : new Error(String(commitError)),
);
}
},
(error) => {
if (token !== this.__renderToken) return;
Expand Down
Loading