Skip to content

dogfood: factory-form extends WebComponent({...}) defeats display-only elision #604

@vivek7405

Description

@vivek7405

Problem

A display-only component written in the now-mandatory declare-free factory DX (class X extends WebComponent({ ... })) is never elided, even when it carries no interactivity signal. It ships its module to the browser unnecessarily.

The cause is in the elision analyzer. hasModuleScopeSideEffect (packages/server/src/component-elision.js) scans depth-0 source for top-level calls and ships the module if it finds one. It exempts the registration calls register() / define() (line 276) but does not exempt the WebComponent({ ... }) factory call that appears in the class extends clause. That factory call sits at depth 0 (before the class body brace), so analyzeComponentSource returns interactive: true with reason runs code at module scope (a top-level call, new, or dynamic import()) (line 404) for ANY factory-form component, before the per-class analysis ever runs.

The callless form class X extends WebComponent { ... } (no parens) has no such call and elides correctly.

Consequence: the hasNonStateFactoryProperty(factoryArg) branch (line 450, helper at line 581), written specifically to let a state-only factory component ({ n: prop({ state: true }) }) be elided, is unreachable dead code, because every factory-form component is shipped by the side-effect guard first.

This is safe-direction: it only ever over-ships (never wrongly drops a page), so it is a missed-optimization regression, not a correctness break. But it defeats explicit code and is freshly relevant: #599 enforced the factory DX and made static properties throw at runtime, so the factory form is now the ONLY supported way to declare reactive props.

Repro (verified in a sandbox app, prod webjs start)

// components/display-badge.js
import { html, WebComponent } from '@webjsdev/core';
class DisplayBadge extends WebComponent({}) {            // factory form ships
  render() { return html`<span class="db-badge">static</span>`; }
}
DisplayBadge.register('display-badge');

An inert route importing only this component still boots its module plus emits a modulepreload. Rewriting to class DisplayBadge extends WebComponent { ... } (callless) makes the same route drop to zero application JS (page module, inert layout, and component all elided) while the SSR'd HTML is byte-identical. Direct analyzer probe confirms: WebComponent({}) and WebComponent() give interactive: true (reason: module-scope side effect); callless extends WebComponent gives interactive: false.

Fastest verification path (no app boot)

The bug reproduces at the unit level by calling the analyzer directly, so an agent can confirm the fix without running a server:

import { analyzeComponentSource } from '@webjsdev/server/src/component-elision.js';
const factory = "import { html, WebComponent } from '@webjsdev/core';\nclass B extends WebComponent({}) { render() { return html`<span>x</span>`; } }\nB.register('b-x');";
analyzeComponentSource(factory).interactive; // BUG: true (reason: module-scope side effect). After fix: false

true is the bug, false is fixed. Add the assertion forms in the acceptance criteria to packages/server/test/elision/analyze.test.js (the existing analyze.test.js already drives analyzeComponentSource this way). The full sandbox repro (route drops to zero application JS) is the integration-level confirmation, but the unit probe is the inner loop.

Design / approach

Exempt the WebComponent(...) factory call from hasModuleScopeSideEffect: it is part of a class extends clause (a declaration), not an independent side-effecting statement, the same justification as the existing register / define exemption. After exempting it, the per-class path (lines 419 to 455) runs and hasNonStateFactoryProperty correctly ships a non-state factory prop while letting a state-only or no-prop factory component elide.

Keep the conservative bias: only the bare identifier WebComponent immediately preceding the call paren should be exempted, not arbitrary Foo(...) calls. The simplest approach mirrors the line-276 exemption (if (ident === 'WebComponent') continue;); in practice the factory only appears in an extends position, so a plain identifier exemption is acceptable and matches register / define.

Implementation notes (for the implementing agent)

  • Where to edit: packages/server/src/component-elision.js:
    • hasModuleScopeSideEffect, the call-scanning loop, add a WebComponent exemption next to line 276 (if (ident === 'register' || ident === 'define') continue;).
    • Verify the per-class loop at lines 419 to 455 and hasNonStateFactoryProperty (line 581) then behave as intended for {}, { n: prop({ state: true }) }, and { n: Number } factory args.
  • Landmines:
    • The analyzer is a conservative denylist: a false "display-only" verdict BREAKS the page (the component never boots), a false "interactive" verdict only misses an optimization. The fix must not let a genuinely interactive factory component slip through, which is why the per-class checks (events, slots, shadow, lifecycle, non-state props, reactive imports) must still gate. The factory exemption only removes the BLANKET ship; the specific signals still ship.
    • static properties is now banned at runtime by feat: enforce declare-free factory DX (drop static properties + declare) #599 (no-static-properties), so hasNonStateReactiveProperty (the static properties reader, line ~524) is legacy; the live path is hasNonStateFactoryProperty.
    • The elision invariant (server AGENTS.md invariant 7) is verified differentially by test/elision/differential-elision.test.js plus the differential elision e2e: render on vs off must yield identical SSR HTML (modulo the JS set) and identical post-hydration DOM. The fix must keep that green.
  • Test blind spot to close: packages/server/test/elision/analyze.test.js DISPLAY_ONLY fixture uses the feat: enforce declare-free factory DX (drop static properties + declare) #599-banned static properties plus callless extends WebComponent. Migrate the elision fixtures to the factory DX so the factory form is actually exercised, and add cases asserting: WebComponent({}) elides, WebComponent({ n: prop({ state: true }) }) elides, WebComponent({ n: Number }) ships (non-state prop), WebComponent({}) plus @click ships.
  • Tests plus docs: unit (packages/server/test/elision/), plus a counterfactual that fails when the exemption is reverted. No public API change, so no AGENTS.md API edit; the elision section of server AGENTS.md invariant 7 already covers the behaviour.

Acceptance criteria

  • A display-only factory-form component (extends WebComponent({}) or state-only props) is elided; an inert route importing only it ships zero application JS
  • A factory component with a non-state reactive prop, @event, <slot>, static shadow, or a lifecycle hook still ships
  • analyzeComponentSource returns interactive: false for WebComponent({}) and WebComponent({ n: prop({ state: true }) }), and true for WebComponent({ n: Number })
  • Elision test fixtures migrated off the feat: enforce declare-free factory DX (drop static properties + declare) #599-banned static properties to the factory DX
  • A counterfactual proves the new test fires (revert the exemption, factory display-only component ships again)
  • test/elision/differential-elision.test.js and the differential-elision e2e stay green

Deep-research verification (2026-06-19)

Verified the exemption against the full analyzer and all callers. Result: SOUND, with one scoping refinement.

  • Sufficiency confirmed. Every interactivity surface has an INDEPENDENT check in analyzeComponentSource (events EVENT_BINDING_RE / EVENT_PROP_RE, SLOT_RE, importsReactivePrimitive, importsClientRouter, COMPONENT_CLIENT_GLOBAL_RE, importsSideEffectNonCorePackage, the per-class CLIENT_LIFECYCLE_HOOKS / CLIENT_METHOD_CALLS / declaresStaticTrue('shadow'|'refresh') / hasNonStateReactiveProperty / hasNonStateFactoryProperty, plus cross-module observation). NOTHING is caught ONLY by hasModuleScopeSideEffect. The factory call is a declaration-site construct, not client work, so exempting it cannot let an interactive factory component slip through.
  • Refinement: scope the exemption to the extends-clause form. hasModuleScopeSideEffect is SHARED: analyzeElision calls it at L777 to classify ALL files (route modules + helpers) into clientGlobalOrBareFiles, not just components. A blanket if (ident === 'WebComponent') continue; would also de-flag a non-component module that calls WebComponent(...) at top level. That pattern is an anti-pattern (a non-component file creating a base class never ships meaningfully) and would still ship via the other checks, so the blanket form is acceptable; but prefer scoping the exemption to a WebComponent call in an extends position (the only legitimate site) so the side-effect detector is unchanged for every other module. Add a unit case proving a top-level non-extends WebComponent(...) call in a NON-component file is still treated as a side effect.
  • const Base = WebComponent({...}); class X extends Base is unaffected. extractWebComponentClassBodies matches only extends WebComponent, so this returns bodies.length === 0 and ships via the "no parseable WebComponent class body" path (L415-416) both before and after the change. No new mis-elision.
  • Out of scope (do NOT expand this issue): a side-effecting factory-arg default (extends WebComponent({ x: sideEffect() })) is not detected by any check, but that is a PRE-EXISTING gap (the depth-0 frame scan already skips calls inside the ({...}) object literal) orthogonal to this fix. File separately if it matters; it is structurally unlikely.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type
No fields configured for issues without a type.

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions