Skip to content

Commit d2f80bd

Browse files
committed
fix: keep classifying after a rebuild, and stop claiming an uncommitted swap
The feature turned itself off for every edit after the first in a burst. doRebuild invalidates the lazy analysis, nothing re-warms it until an HTTP request arrives, and the relay defers that request by its 2000ms quiet window while the measured inter-save gap is about a second. So the second save classified analysis-cold and the strongest-verdict rule collapsed the whole batch to a full reload. The gate now reads whether the derived sets are POPULATED rather than whether they are current, which is what the rest of the code already assumed: classifying against the previous build's graph is intended, and it is conservative in the right direction, since a file that graph has never seen falls through to a reload. applySwap returns without committing on four paths (a missing frame, and three degradations to a hard navigation), and all four still reported applied true, which is the hole the flag exists to close. They return a sentinel now and fetchAndApply maps it. Three doc corrections. The gallery copy described a counter that is not on that page. The skill reference kept the direct CLI invocation the website copy had already dropped. And both runtime surfaces listed five watched directories while the supervisor also watches root middleware, so a middleware edit is a full reload rather than the in-place refresh they implied.
1 parent 2fa8552 commit d2f80bd

7 files changed

Lines changed: 98 additions & 19 deletions

File tree

.agents/skills/webjs/references/runtime.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ Three seams pick a runtime-specific implementation, all inside the framework, no
4040

4141
**The in-place dev refresh (#1398) needs the server process to SURVIVE the edit,** which is the whole of the Node-versus-Bun difference in that row. A page or layout never hydrates, so a freshly rendered page is the complete truth for it and the client router can swap it in without a reload, keeping scroll and (for a page edit) the hydrated state of components outside the changed region. The server classifies the changed file and puts the verdict on the live-reload event, so this needs a process that is still alive to do the classifying.
4242

43-
Bun's `bun --hot` invalidates modules in place without restarting, so it gets the refresh. Node's `bun --hot` equivalent is `node --watch`, which RESTARTS the process on a change under `app`, `components`, `modules`, `lib`, or `actions`, and a fresh process holds no record of what changed, so those edits are always a full reload. Two Node cases still refresh in place: an edit OUTSIDE those five dirs (`db/schema.server.ts`, a `webjs.dev.watch` content dir), and running `webjs dev --no-hot`, which keeps the server in one process on either runtime. A component edit is a full reload everywhere by design, because `customElements.define` is once-per-tag and swapping fresh markup onto the old class would be worse than the reload.
43+
Bun's `bun --hot` invalidates modules in place without restarting, so it gets the refresh. Node's `bun --hot` equivalent is `node --watch`, which RESTARTS the process on a change under `app`, `components`, `modules`, `lib`, or `actions`, or to a root `middleware.{ts,js,mts,mjs}`, and a fresh process holds no record of what changed, so those edits are always a full reload. Two Node cases still refresh in place: an edit OUTSIDE that watched set (`db/schema.server.ts`, a `webjs.dev.watch` content dir), and running `npm run dev -- --no-hot`, which keeps the server in one process on either runtime. A component edit is a full reload everywhere by design, because `customElements.define` is once-per-tag and swapping fresh markup onto the old class would be worse than the reload.
4444

4545
The 103 Early Hints gap costs only a small first-load latency edge where an edge proxy forwards the 103, never correctness. The `modulepreload` hints still ship in the document head on both runtimes.
4646

gallery/modules/client-router/components/router-controls.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ export class RouterControls extends WebComponent {
4949
</div>
5050
<p class="text-sm text-muted-foreground">
5151
refreshPage() re-renders THIS url on the server and swaps it in.
52-
The server time above updates, the counter below keeps counting, and
53-
your scroll position does not move.
52+
The server time above updates, and your scroll position does not
53+
move. On a page with hydrated components outside the swapped region,
54+
their state survives too.
5455
</p>
5556
<p class="text-sm text-muted-foreground">
5657
Plain link:

packages/core/src/router-client.js

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2958,9 +2958,11 @@ function handleNavigationError(href, status, error) {
29582958
* behaviour, so a page rendered through `notFound()`, `forbidden()`, or an
29592959
* `error.ts` boundary has `ok:false` and `applied:true`. It is `false`
29602960
* wherever no swap committed: a transport failure, a non-HTML body, an
2961-
* unparseable one, a 204/205, a discarded revalidation, and an abort. A
2962-
* caller deciding whether to fall back to a full page load wants `applied`;
2963-
* one reporting the submission's success wants `ok`.
2961+
* unparseable one, a 204/205, a discarded revalidation, an abort, and every
2962+
* `applySwap` path that returns without committing (a missing frame, or a
2963+
* degradation to a hard navigation from an importmap mismatch or a poisoned
2964+
* boundary scan). A caller deciding whether to fall back to a full page load
2965+
* wants `applied`; one reporting the submission's success wants `ok`.
29642966
*/
29652967
async function fetchAndApply(href, frameId, recordHistory, optimisticState, method, body, signal, token, revalidating, refresh) {
29662968
method = method || 'GET';
@@ -3163,6 +3165,12 @@ async function fetchAndApply(href, frameId, recordHistory, optimisticState, meth
31633165
if (!doc) { restoreOptimistic(optimisticState); handleNavigationError(href, null, new Error('navigation response did not parse as HTML')); return { ok: false, status: respStatus, aborted: false, applied: false }; }
31643166

31653167
const disposition = applySwap(doc, frameId, !!revalidating, finalUrl, incomingBuild, incomingSrc, refresh);
3168+
// `'none'` means applySwap returned WITHOUT committing anything: the frame the
3169+
// response was for is missing, or it degraded to a hard navigation (an
3170+
// importmap/build mismatch, a poisoned boundary scan). The page is not left
3171+
// in a bad state either way, but nothing was applied IN PLACE, and `applied`
3172+
// has to say so or it repeats the hole it exists to close.
3173+
if (disposition === 'none') return { ok: respOk, status: respStatus, aborted: false, applied: false };
31663174
// A discarded revalidation must be discarded OUTRIGHT: a streamed response's
31673175
// boundary templates must not splice into the restored snapshot afterward
31683176
// (boundary ids are per-render sequential, so a reduced render's numbering
@@ -3694,6 +3702,11 @@ let _swapCommit = Promise.resolve();
36943702
* @param {string | null} href
36953703
* @param {string | null} [incomingBuild]
36963704
* @param {string | null} [incomingSrc]
3705+
* @returns {'discard' | 'none' | undefined} `'discard'` when a background
3706+
* revalidation was thrown away, `'none'` when it returned without committing
3707+
* anything (a missing frame, or a degradation to a hard navigation), and
3708+
* `undefined` when a swap committed. `fetchAndApply` maps the first two to
3709+
* `applied: false`.
36973710
* @param {'page' | 'shell' | undefined} [refresh] same-URL in-place refresh
36983711
* mode (#1398). `'shell'` takes the full-body tier directly, because the
36993712
* layout's OWN markup changed and that lives outside every children range.
@@ -3836,14 +3849,14 @@ function applySwap(doc, frameId, revalidating, href, incomingBuild, incomingSrc,
38363849
if (sessionStorage) sessionStorage.setItem(flag, '1');
38373850
reportFallback('deploy-mismatch', href);
38383851
hardNavigate(href);
3839-
return;
3852+
return 'none';
38403853
}
38413854
} catch {
38423855
// sessionStorage unavailable (private mode w/ quota etc.):
38433856
// fall through to a single reload like before.
38443857
reportFallback('deploy-mismatch', href);
38453858
hardNavigate(href);
3846-
return;
3859+
return 'none';
38473860
}
38483861
} else if (!mismatch) {
38493862
// No importmap/build mismatch, so no hard reload. But the app-source
@@ -3925,7 +3938,7 @@ function applySwap(doc, frameId, revalidating, href, incomingBuild, incomingSrc,
39253938
if (!evt.defaultPrevented) {
39263939
console.warn(`[webjs] frame "${frameId}" was not in the navigation response, leaving it unchanged. Handle "webjs:frame-missing" (preventDefault) to override.`);
39273940
}
3928-
return;
3941+
return 'none';
39293942
}
39303943

39313944
// 1b. Same-URL refresh in `shell` mode (#1398). A boundary morph can only
@@ -4001,7 +4014,7 @@ function applySwap(doc, frameId, revalidating, href, incomingBuild, incomingSrc,
40014014
: !there ? 'incoming-boundaries-malformed'
40024015
: 'no-shared-boundary', href);
40034016
hardNavigate(href);
4004-
return;
4017+
return 'none';
40054018
}
40064019

40074020
// A BACKGROUND revalidation (revalidating + href) with no trustworthy plan

packages/server/src/dev-classify.js

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,19 @@ function verdictRank(v) {
8585
*
8686
* The ladder, first match wins:
8787
*
88-
* 1. The analysis is cold, so nothing is known yet. Fail safe. This mirrors
89-
* Vite's pessimistic seed (`needFullReload = modules.length === 0`). A
90-
* rebuild invalidates the lazy analysis, so this rung is live between a
91-
* rebuild and the next request. In practice the reload or in-place refresh
92-
* each verdict produces IS that request, so the analysis is warm again well
93-
* before the next edit lands, and the rung only catches an edit that beat
94-
* the browser to it.
88+
* 1. The analysis has NEVER completed, so nothing is known yet. Fail safe. This
89+
* mirrors Vite's pessimistic seed (`needFullReload = modules.length === 0`).
90+
* In practice this is the window between boot and the first request.
91+
*
92+
* `analysisReady` deliberately means "the sets are POPULATED", not "they are
93+
* current". A rebuild invalidates the lazy analysis and nothing re-warms it
94+
* until an HTTP request arrives, while the relay defers that request by its
95+
* 2000ms quiet window (#1397), which is longer than the measured inter-save
96+
* gap. Reading currency here would therefore turn the feature off for every
97+
* edit after the first in a burst. Classifying against the previous build's
98+
* graph is the intended behaviour, and it is conservative in the right
99+
* direction: a file the stale graph has never seen falls to rung 6 and
100+
* reloads.
95101
* 2. The path is outside `appDir`. The watcher only fires for `appDir` plus the
96102
* opt-in `webjs.dev.watch` roots (#894), and a file outside `appDir` is
97103
* content the server reads at render time, never a browser module. It can

packages/server/src/dev.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,16 @@ export async function createRequestHandler(opts) {
907907
// platform's traffic and probes are the retry loop. `readyError` holds a
908908
// propagating analysis failure so /__webjs/ready can report it.
909909
let analysisDone = false; // deterministic analysis complete (readiness gate)
910+
// Whether the analysis has EVER completed (#1398). Distinct from
911+
// `analysisDone`, which `doRebuild` flips false on every edit: the live-reload
912+
// classifier needs to know the derived sets are POPULATED, not that they are
913+
// current, because it deliberately classifies against the previous build's
914+
// graph (see the note in `doRebuild`). Gating it on `analysisDone` turned the
915+
// feature off for every edit after the first in a burst, since nothing
916+
// re-warms the analysis until an HTTP request arrives and the relay defers
917+
// that request by its 2000ms quiet window (#1397), which is longer than the
918+
// measured inter-save gap. Never reset.
919+
let analysisEverDone = false;
910920
// A pinned app applied its FULL vendor map and published the build id at boot
911921
// (above). The deferred vendor stage still runs once (and after every rebuild)
912922
// to PRUNE that map to the elision-reachable specifiers, so a pinned app serves
@@ -1102,6 +1112,7 @@ export async function createRequestHandler(opts) {
11021112
);
11031113
}
11041114
analysisDone = true;
1115+
analysisEverDone = true;
11051116
ranAnalysis = true;
11061117
}
11071118
readyError = null;
@@ -1753,7 +1764,7 @@ export async function createRequestHandler(opts) {
17531764
shippedFiles: state.shippedFiles,
17541765
graphFiles: state.graphFiles,
17551766
pageFiles: state.pageFiles,
1756-
analysisReady: analysisDone,
1767+
analysisReady: analysisEverDone,
17571768
sep,
17581769
}),
17591770
appDir,

packages/server/test/dev/classify-live.test.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,54 @@ Counter.register('my-counter');
138138
assert.equal(v.why, 'ships-to-browser');
139139
});
140140

141+
// THE burst regression. A rebuild invalidates the lazy analysis, and nothing
142+
// re-warms it until an HTTP request arrives, which the relay defers by its
143+
// 2000ms quiet window (#1397) while the measured inter-save gap is about a
144+
// second. So gating the classifier on "the analysis is CURRENT" turned the
145+
// feature off for every edit after the first in a burst: the second save
146+
// classified `analysis-cold` and the strongest-verdict rule collapsed the whole
147+
// batch to a full reload. It is gated on "the sets are POPULATED" instead.
148+
//
149+
// Every other case here warms with a fetch before its single edit, so none of
150+
// them can see this; the second edit has to land with no request in between.
151+
test('a SECOND edit with no request in between still classifies (the burst case)', async () => {
152+
const appDir = scaffold();
153+
const srv = await startServer({ appDir, port: 0, dev: true });
154+
const port = srv.server.address().port;
155+
try {
156+
const res = await fetch(`http://127.0.0.1:${port}/`);
157+
assert.equal(res.status, 200);
158+
await res.text();
159+
160+
const first = waitForReloadFrame(port, 6000);
161+
await sleep(150);
162+
writeFileSync(join(appDir, 'app/page.js'), `
163+
import { html } from '@webjsdev/core';
164+
import '../components/counter.js';
165+
export default function Page() {
166+
return html\`<main>EDIT_ONE<my-counter></my-counter></main>\`;
167+
}
168+
`);
169+
assert.equal(JSON.parse(await first).v, 'page', 'the first edit classifies');
170+
171+
// No fetch here on purpose: this is what a burst looks like.
172+
const second = waitForReloadFrame(port, 6000);
173+
await sleep(150);
174+
writeFileSync(join(appDir, 'app/page.js'), `
175+
import { html } from '@webjsdev/core';
176+
import '../components/counter.js';
177+
export default function Page() {
178+
return html\`<main>EDIT_TWO<my-counter></my-counter></main>\`;
179+
}
180+
`);
181+
const v = JSON.parse(await second);
182+
assert.equal(v.v, 'page', 'and so does the second, against the previous build\'s graph');
183+
assert.equal(v.why, 'page-module', 'rather than falling back to analysis-cold');
184+
} finally {
185+
await srv.close();
186+
}
187+
});
188+
141189
test('a LAYOUT edit rides a `shell` verdict, because its own markup is outside every children range', async () => {
142190
const appDir = scaffold();
143191
const v = await verdictForEdit(appDir, 'app/layout.js', `

website/app/docs/runtime/page.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export default function Runtime() {
3232
</tbody>
3333
</table>
3434
<p>The in-place dev refresh needs the server process to <strong>survive</strong> the edit, which is the whole of that last row. A page or layout never hydrates, so a freshly rendered page is the complete truth for it and the client router can swap it in without a reload, keeping your scroll position and the hydrated state of components outside the changed region. The server classifies the changed file and puts the verdict on the live-reload event, so it needs a process that is still alive to do the classifying.</p>
35-
<p>Bun's <code>bun --hot</code> invalidates modules in place without restarting, so it gets the refresh. On Node, <code>node --watch</code> restarts the process on a change under <code>app</code>, <code>components</code>, <code>modules</code>, <code>lib</code>, or <code>actions</code>, and a fresh process holds no record of what changed, so those edits are a full reload. Two Node cases still refresh in place: an edit outside those five directories (<code>db/schema.server.ts</code>, a <code>webjs.dev.watch</code> content directory), and <code>npm run dev -- --no-hot</code>, which keeps the server in one process on either runtime. A component edit is a full reload everywhere by design, because <code>customElements.define</code> is once-per-tag and swapping fresh markup onto the old class would be worse than the reload.</p>
35+
<p>Bun's <code>bun --hot</code> invalidates modules in place without restarting, so it gets the refresh. On Node, <code>node --watch</code> restarts the process on a change under <code>app</code>, <code>components</code>, <code>modules</code>, <code>lib</code>, or <code>actions</code>, or to a root <code>middleware</code> file, and a fresh process holds no record of what changed, so those edits are a full reload. Two Node cases still refresh in place: an edit outside that watched set (<code>db/schema.server.ts</code>, a <code>webjs.dev.watch</code> content directory), and <code>npm run dev -- --no-hot</code>, which keeps the server in one process on either runtime. A component edit is a full reload everywhere by design, because <code>customElements.define</code> is once-per-tag and swapping fresh markup onto the old class would be worse than the reload.</p>
3636
3737
<p>Either way the <code>.ts</code> stripping is position-preserving with no sourcemap, and the bytes the browser fetches are identical. The 103 Early Hints gap only costs a small first-load latency edge where your edge forwards 103, never correctness (the modulepreload hints still ship in the document head).</p>
3838

0 commit comments

Comments
 (0)