Skip to content

dogfood: reactive-props-no-class-field false positive on object-literal keys inside a method #934

Description

@vivek7405

Problem

webjs check's reactive-props-no-class-field rule fires a FALSE POSITIVE on a
reactive prop that is correctly declared via the WebComponent({ ... }) factory
and set in the constructor, when a method body later contains a multi-line
object literal whose keys match a factory prop name (for example game: /
scoreboard:). The prop is never declared as a class field, yet the rule reports
"Reactive prop game uses a class-field declaration" and webjs check fails.

Found by dogfooding a tic-tac-toe app: the agent had a legitimate
game: prop(Object) factory prop, plus a method that built
{ game: ..., scoreboard: ... }, and check blocked it. The workaround was to
move the object-literal logic to a module-level function, which is a real
authoring tax for correct code.

Reproduction (minimal, confirmed)

import { WebComponent, prop, html } from '@webjsdev/core';

class Board extends WebComponent({
  game: prop(Object),
  scoreboard: prop(Object),
}) {
  constructor() {
    super();
    this.game = null;         // correct: default set in the constructor
    this.scoreboard = null;
  }

  makeMove(op) {
    const next = {            // object literal INSIDE a method
      game: { ...this.game, board: op },
      scoreboard: { ...this.scoreboard, x: 1 },
    };
    return next;
  }

  render() { return html`<div>${this.game}</div>`; }
}
Board.register('x-board');

webjs check reports reactive-props-no-class-field on BOTH game and
scoreboard, though neither is a class field.

Root cause

findFieldInitializers() in packages/server/src/check.js walks the class body
tracking brace depth, and only treats a line as a candidate class field when
depth === 0. The depth accounting is thrown off (the template-literal
${ ... } interpolation handling increments depth on ${ but the closing }
is consumed while still in string mode, so depth is not balanced; combined with
the object-literal braces this lets a line inside a method body be seen as
depth === 0). The per-line regex (typeOnlyRe) then matches game: <expr> as
a type-annotated class field.

Implementation notes (for the implementing agent)

  • Where to edit: packages/server/src/check.js, findFieldInitializers() (near
    the reactive-props-no-class-field rule around L589 to L611). The brace-depth
    walker is the bug site. The template-literal ${ branch increments depth
    but never decrements on the matching } because it stays in string mode.
  • The fix should make depth accounting correct across `html`` template
    interpolations and nested object literals, OR restrict class-field detection to
    the true class-body top level (for example, only consider lines before the
    first method / between member boundaries), so object-literal keys inside a
    method are never candidates.
  • Landmine: do NOT regress the true positives the rule exists to catch
    (count = 0, count: number = 0, count!: number, count?: number at the
    class-body top level). Keep those failing.
  • Tests: add a counterfactual in the check test suite that the minimal
    reproduction above PASSES (no violation), plus keep the existing true-positive
    cases failing. A component with a factory prop game and a method building a
    { game: ... } object literal must pass.

Acceptance criteria

  • The minimal reproduction above passes webjs check (no false positive)
  • Real class-field declarations of a factory prop still fail (true positives intact)
  • A counterfactual test proves the rule still fires on a genuine class field
  • Tests cover the object-literal-in-method case

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions