Skip to content

dogfood: flag array-typed reactive props declared with Object, declare Array #631

@vivek7405

Description

@vivek7405

Problem

An array-typed reactive property declared via the WebComponent({ ... }) factory should pass the Array runtime converter, not Object. Found while dogfooding examples/blog:

// examples/blog/modules/comments/components/comments-thread.ts:13
initial:  prop<CommentFormatted[]>(Object),   // value is CommentFormatted[]

It does work. In packages/core/src/component.js the default converter treats Object and Array identically (decl.type === Object || decl.type === Array runs JSON.stringify on reflect and JSON.parse on read), so there is no runtime behavior difference. But declaring Object for an array-typed value misstates the shape, reads as a mistake to the next agent or human, and diverges from the documented built-in set (String, Number, Boolean, Object, Array). For an AI-first framework whose users develop with AI agents, the idiomatic form should be both used in our own code and enforced by the tooling.

Design / approach

Three parts.

  1. Fix the existing occurrence(s). A repo-wide grep currently finds exactly ONE: examples/blog/.../comments-thread.ts:13. Switch it to prop<CommentFormatted[]>(Array). Audit also for the bare foo: Object form whose value is an array, and for prop<Array<X>>(Object).

  2. Add a webjs check correctness rule, proposed name array-prop-uses-array-type. It flags a factory-declared reactive prop whose TS generic is an array type (prop<T[]>(Object) or prop<Array<T>>(Object)) but whose runtime constructor is Object, with a fix message to use Array. This is statically detectable from the factory argument text, since the generic and the constructor sit in the same prop<...>(Object) call. Conservative: fire only when BOTH the array-typed generic AND the Object constructor are present in one prop(...) declaration. Do not guess at a bare foo: Object with no generic to prove array-ness, which would risk false positives. Report-only, consistent with the rest of webjs check (no autofix).

  3. Document the guardrail so agents write it right the first time. The check is the backstop, the docs are the primary prevention.

Implementation notes (for the implementing agent)

  • Rule home is packages/server/src/check.js. Register the name in the RULES array (around L55 to L130, alongside reactive-props-no-class-field and no-static-properties) with a name plus description, then add an implementation block in checkConventions() (the prop-related blocks live around L396 to L441). Follow the existing reactive-props-no-class-field block: it gates on /class\s+\w+\s+extends\s+WebComponent/, iterates extractWebComponentClassBodies(scan) (from packages/server/src/js-scan.js), and reads factoryProps. Confirm whether factoryProps carries the raw per-prop declaration TEXT (needed to see prop<...[]>(Object)). If it only carries names, parse the factory-argument object text directly (the same extractor already isolates the factory call) via a small helper rather than re-lexing.
  • Use the redaction mask (scan is the redacted source the other rules read) so a prop<X[]>(Object) shown inside an html example string or a docs <pre> is never flagged. This is the same false-positive guard the sibling rules rely on.
  • Landmine, Object and Array are runtime-identical. The converter in packages/core/src/component.js (L654 to L664 toAttribute, L951 to L958 fromAttribute) collapses Object and Array, so this rule is about clarity and correctness of intent, not a crash. Word the rule description accordingly. It fits webjs check (wrong to ship, since it misstates the prop contract) but stays honest that it is not a runtime bug. The user explicitly asked for tool enforcement that ships to end users, so check is the intended home rather than CONVENTIONS.md prose.
  • Tests live in packages/server/test/check/. Add a positive fixture that fires, a counterfactual prop<X[]>(Array) that does NOT fire, and an html-embedded example that does NOT fire. If a rule-list test asserts the full rule set, add the new name there too.
  • Docs surfaces (run the webjs-doc-sync skill): the reactive-props sections of root AGENTS.md and agent-docs/components.md (state the array-prop rule next to the prop() options), CONVENTIONS.md, the docs site components page under docs/app/docs/, and the scaffold per-agent rule templates under packages/cli/templates/ so a freshly scaffolded app carries the guidance for every agent (Claude, Cursor, Copilot, and the rest). If webjs check --rules output is documented anywhere, add the new rule there.
  • Invariants: framework packages/ stays plain .js plus JSDoc (no .ts). No banned pause-punctuation in new prose (AGENTS.md invariant 11). Commit per logical unit (fix the example, add the rule, sync docs as coherent units).

Acceptance criteria

  • examples/blog/.../comments-thread.ts uses prop<CommentFormatted[]>(Array); repo grep finds no remaining prop<...[]>(Object) or prop<Array<...>>(Object)
  • webjs check ships an array-prop-uses-array-type rule that flags prop<T[]>(Object) and passes on prop<T[]>(Array)
  • A counterfactual proves the rule fires, and that it does NOT fire on the Array form or an html-embedded example
  • Rule covered by tests in packages/server/test/check/
  • Docs synced via webjs-doc-sync: AGENTS.md, agent-docs/components.md, CONVENTIONS.md, docs site, scaffold per-agent templates

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type
No fields configured for issues without a type.

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions