refactor(react): one config-bag predicate, and a pin against a seventh spelling

.changeset/6761-one-config-bag-predicate.md

@@ -0,0 +1,22 @@
+---
+---
+
+Internal refactor only; no published behaviour changes. "Is this value a real config bag
+(an object, not an array, not null)?" was asked in `packages/react` in six places in four
+spellings — `SchemaRenderer`'s module-private `isConfigBag`, its `winningVisibilityKey`
+inline, `utils/propsBagDiagnostic.ts`'s private twin, and TWO in
+`utils/unevaluatedExpression.ts` (`scanBag`'s negated early return and the `hoisted`
+dedupe read). All six now read one exported predicate, `utils/configBag.ts`
+(objectui#6761).
+
+They agreed, which was never the cost: a copy that drifts produces no error, only a
+different answer on one channel — the failure mode `hasDeclaredPredicate` was created to
+end for "is a gate DECLARED?" (objectui#3850). So the convergence ships with a pin,
+`utils/configBag.pin.test.ts`, that fails when a seventh spelling appears anywhere in
+`packages/react/src`. The two sites that ask the same SHAPE question about a data ROW
+(`SchemaRenderer`'s `boundRecord`, `usePredicateRecordContext`) are deliberately NOT
+merged and are named in the pin's allowlist with that reason: their "no row → bind
+NOTHING" rule is a different ruling, and it must not move if "config bag" ever narrows.
+
+Each of the six sites was ablated individually to bare truthiness and measured, rather
+than assumed equivalent because the lines look alike.

packages/react/src/SchemaRenderer.tsx

@@ -29,6 +29,7 @@ import { useRecordContext } from './context/RecordContext.js';
 import { usePredicateScope } from './hooks/useExpression.js';
 import { usePageVariables } from './hooks/usePageVariables.js';
 import { resolveKeyedI18nLabel } from './utils/i18n.js';
+import { isConfigBag } from './utils/configBag.js';
 import { reportUnevaluatedExpressions } from './utils/unevaluatedExpression.js';
 import { reportDroppedPropsBag } from './utils/propsBagDiagnostic.js';
 import {
@@ -139,27 +140,6 @@ function resolveAriaProps(schema: Record<string, any>): Record<string, string |
  */
 const HOIST_PROTECTED_KEYS = new Set(['type', 'id']);
 
-/**
- * A real config bag: an object, and not an array pretending to be one
- * (objectui#6752).
- *
- * One predicate for BOTH bags. It existed only as the `properties` branch's
- * inline guard in the evaluation memo, spelled again inline in
- * {@link propsWithoutCanonicalKeys}; `props` had no equivalent at either site,
- * which is the whole of objectui#6752. `typeof null === 'object'` is covered by
- * the truthiness test.
- *
- * ⚠️ This is the FOURTH spelling of this one question in this package —
- * `propsBagDiagnostic.ts` and `unevaluatedExpression.ts` each carry their own,
- * and the visibility block below carries a fifth inline. Only the two in THIS
- * file are unified here: the other two live in objectui#6708's and
- * objectui#4795's modules, and merging them is a refactor across cards rather
- * than this one's to make. Filed rather than smuggled in.
- */
-function isConfigBag(value: unknown): value is Record<string, unknown> {
-  return !!value && typeof value === 'object' && !Array.isArray(value);
-}
-
 /**
  * The legacy `props` bag, minus every key the canonical `properties` bag also
  * declares (objectui#5123).
@@ -356,8 +336,7 @@ const visibilityGateKind = (key: VisibilityChainKey): PredicateGateKind =>
  */
 function winningVisibilityKey(node: Record<string, unknown>): VisibilityChainKey | undefined {
   const propertiesBag = node.properties;
-  const hasPropertiesBag =
-    propertiesBag != null && typeof propertiesBag === 'object' && !Array.isArray(propertiesBag);
+  const hasPropertiesBag = isConfigBag(propertiesBag);
   const effective = (key: string): unknown =>
     hasPropertiesBag && Object.prototype.hasOwnProperty.call(propertiesBag, key)
       ? (propertiesBag as Record<string, unknown>)[key]

packages/react/src/utils/configBag.pin.test.ts

@@ -0,0 +1,128 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#6761 — "is this a real config bag?" is asked in ONE place.
+ *
+ * ## Why a pin, and why this one is the point of the card
+ *
+ * The six occurrences this replaced all AGREED. Convergence alone therefore
+ * fixes today and not tomorrow: nothing stops a seventh from being written
+ * next week, and nothing would report it if it drifted — each spelling is a
+ * boolean expression, so a disagreement produces no error, only a different
+ * answer on one channel. `hasDeclaredPredicate` (`@object-ui/core`) is the
+ * repo's precedent for both halves of that lesson: three spellings with three
+ * different scopes, ended by one definition (objectui#3850).
+ *
+ * ## What it measures
+ *
+ * Every production source in `packages/react/src` (tests excluded — a test
+ * asserting on shapes is not a runtime answer that can drift on a channel),
+ * scanned for the `typeof x === 'object'` + `Array.isArray(x)` conjunction on
+ * the SAME operand, in either order and either polarity. That is the family
+ * every spelling of this question has been written in here — four of them
+ * across six occurrences, measured on `b98352a15`. A genuinely novel
+ * construction (`Object.prototype.toString.call`, a `constructor` test) would
+ * evade it; the job is to make the CHEAP path fail — copying a line that
+ * already exists — not to be a theorem about object-ness.
+ *
+ * ## The allowlist is a decision, not an escape hatch
+ *
+ * Two entries survive, and they are the SHAPE question asked about a data ROW
+ * rather than a config bag: "did a row bind?", whose answer carries its own
+ * pinned meaning (binding NOTHING rather than an empty row is what keeps a
+ * host-supplied `record` from being shadowed — `usePredicateRecordContext`).
+ * If "config bag" ever narrows, those two must not follow. Adding an entry
+ * here is how you say a new site asks a different question; importing
+ * {@link isConfigBag} is how you say it asks this one.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync, readdirSync } from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const here = path.dirname(fileURLToPath(import.meta.url));
+const SRC_ROOT = path.resolve(here, '..');
+
+/** Every production `.ts`/`.tsx` under `packages/react/src`. */
+function collectSources(dir: string, out: string[] = []): string[] {
+  for (const entry of readdirSync(dir, { withFileTypes: true })) {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) {
+      collectSources(full, out);
+      continue;
+    }
+    if (!/\.tsx?$/.test(entry.name)) continue;
+    if (/\.test\.tsx?$/.test(entry.name)) continue;
+    out.push(full);
+  }
+  return out;
+}
+
+/**
+ * The conjunction, on one operand, in either operand order. `[\s\S]*?` inside
+ * a bounded window rather than `\s*` so a multi-line `&&` chain with a comment
+ * between the halves still matches.
+ */
+const SPELLINGS: RegExp[] = [
+  /typeof\s+([A-Za-z_$][\w$.]*)\s*[!=]==\s*['"]object['"]\s*(?:&&|\|\|)\s*!?\s*Array\.isArray\(\s*\1\s*\)/g,
+  /!?\s*Array\.isArray\(\s*([A-Za-z_$][\w$.]*)\s*\)\s*(?:&&|\|\|)\s*typeof\s+\1\s*[!=]==\s*['"]object['"]/g,
+];
+
+/** `<path relative to packages/react/src>: <matched text, whitespace collapsed>` */
+function findSpellings(): string[] {
+  const found: string[] = [];
+  for (const file of collectSources(SRC_ROOT).sort()) {
+    const source = readFileSync(file, 'utf8');
+    for (const pattern of SPELLINGS) {
+      pattern.lastIndex = 0;
+      let match: RegExpExecArray | null;
+      while ((match = pattern.exec(source)) !== null) {
+        const rel = path.relative(SRC_ROOT, file).split(path.sep).join('/');
+        found.push(`${rel}: ${match[0].replace(/\s+/g, ' ')}`);
+      }
+    }
+  }
+  return found.sort();
+}
+
+/**
+ * The definition, plus the two ROW sites. Every entry names the question it
+ * answers; a new line here without one is the drift this file exists to stop.
+ */
+const ALLOWED = [
+  // THE definition. Everything asking "is this a config bag?" reads this.
+  "utils/configBag.ts: typeof value === 'object' && !Array.isArray(value)",
+  // A data ROW, not a config bag: whether the node's bound record is a row to
+  // put in the evaluator scope at all (binding `{}` would SHADOW a
+  // host-supplied `record`).
+  "SchemaRenderer.tsx: typeof boundRecord === 'object' && !Array.isArray(boundRecord)",
+  // The same row question, one layer down — `usePredicateRecordContext`'s
+  // "no row → bind NOTHING" rule.
+  "hooks/useExpression.ts: typeof record !== 'object' || Array.isArray(record)",
+].sort();
+
+describe('objectui#6761 — one config-bag predicate', () => {
+  it('has no spelling outside the definition and the two row sites', () => {
+    expect(findSpellings()).toEqual(ALLOWED);
+  });
+
+  it('is read by every module that asks the question', () => {
+    for (const rel of [
+      'SchemaRenderer.tsx',
+      'utils/propsBagDiagnostic.ts',
+      'utils/unevaluatedExpression.ts',
+    ]) {
+      const source = readFileSync(path.resolve(SRC_ROOT, rel), 'utf8');
+      expect(source, `${rel} no longer imports the shared predicate`).toMatch(
+        /import \{ isConfigBag \} from '\.(?:\/utils)?\/configBag\.js';/
+      );
+    }
+  });
+});

packages/react/src/utils/configBag.ts

@@ -0,0 +1,71 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * Is this value a real config bag — an object, and not an array pretending to
+ * be one? (objectui#6761)
+ *
+ * This is the ONE definition of that question in `@object-ui/react`, and it
+ * lives here rather than in whichever module happened to need it first,
+ * because every module that asks it is a CONSUMER of the same authored shape:
+ * the `properties` / `props` bags a node may carry. `typeof null === 'object'`
+ * is covered by the truthiness test.
+ *
+ * ## Why one definition, when all the copies agreed
+ *
+ * They agreed; that was never the cost. The cost is that disagreement between
+ * them produces NO error — each copy is a boolean expression, and a copy that
+ * drifts answers a different question on ONE channel while every other channel
+ * keeps the old answer. That is the same failure mode `hasDeclaredPredicate`
+ * (`@object-ui/core`) was created to end for "is a gate DECLARED?", which had
+ * three spellings with three different SCOPES before objectui#3850 —
+ * `disabled: null` greying a control out on one path and not another
+ * (objectui#3862).
+ *
+ * ## The spellings this replaces, measured on `b98352a15`
+ *
+ * Six occurrences, four spellings, all in `packages/react/src`:
+ *
+ *   1. `SchemaRenderer`'s module-private `isConfigBag` — itself already the
+ *      merge of the `properties` evaluation guard and `propsWithoutCanonicalKeys`
+ *      (objectui#6752, which unified those two because it had to touch both,
+ *      and filed the rest as objectui#6761 rather than smuggling it in);
+ *   2. `SchemaRenderer`'s `winningVisibilityKey`, inline, spelled with
+ *      `!= null` where the others spell truthiness;
+ *   3. `utils/propsBagDiagnostic.ts`'s module-private twin (objectui#6708);
+ *   4. `utils/unevaluatedExpression.ts`'s `scanBag` early return, spelled
+ *      NEGATED (objectui#4795);
+ *   5. `utils/unevaluatedExpression.ts`'s `hoisted`, in the same file as (4)
+ *      and spelled positively — this one is not in objectui#6761's inventory
+ *      of five, which counted the file once; six occurrences is the count
+ *      measured on this branch's base, and it was six on the card's base
+ *      (`b76ca6764`) too.
+ *
+ * `!= null` (2) and truthiness (1, 3, 5) cannot disagree HERE, and the
+ * difference is worth naming rather than smoothing over: they part company
+ * only on a falsy value, and every falsy value except `document.all` fails
+ * `typeof === 'object'` in the very next conjunct. The texts differ; the
+ * answers cannot.
+ *
+ * ## What this is NOT
+ *
+ * Not a general "is this a plain object?" utility, and deliberately not shared
+ * with the two sites that ask the same SHAPE question about a data ROW —
+ * `SchemaRenderer`'s `boundRecord` scope entry and
+ * `usePredicateRecordContext` (`hooks/useExpression.ts`). Those answer "did a
+ * row bind?", and their answer has its own pinned meaning: binding NOTHING
+ * rather than an empty row is what keeps a host-supplied `record` from being
+ * shadowed. Should "config bag" ever narrow (rejecting a class instance, say),
+ * the row sites must NOT follow — merging them would make that a single edit
+ * with two rulings behind it. The pin in `configBag.pin.test.ts` names both
+ * sites with this reason, so the next reader finds a decision rather than an
+ * oversight.
+ */
+export function isConfigBag(value: unknown): value is Record<string, unknown> {
+  return !!value && typeof value === 'object' && !Array.isArray(value);
+}

packages/react/src/utils/propsBagDiagnostic.ts

@@ -6,6 +6,8 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import { isConfigBag } from './configBag.js';
+
 /**
  * Dev-build diagnostic: a `props` CONFIG BAG on a node whose renderer reads its
  * config from `schema` — so the keys inside it are evaluated, spread as React
@@ -185,11 +187,6 @@ export function __resetDroppedPropsBagWarnings(): void {
   _warnedPropsBags.clear();
 }
 
-/** A real config bag: an object, and not an array pretending to be one. */
-function isConfigBag(value: unknown): value is Record<string, unknown> {
-  return !!value && typeof value === 'object' && !Array.isArray(value);
-}
-
 /**
  * Which keys of the OUTGOING props bag are dropped by a `schema`-reading
  * renderer, or `null` when there is nothing to say.

packages/react/src/utils/unevaluatedExpression.ts

@@ -6,6 +6,8 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import { isConfigBag } from './configBag.js';
+
 /**
  * Dev-build diagnostic: an UNEVALUATED template expression reached the DOM.
  *
@@ -102,8 +104,8 @@ function scanBag(
   into: UnevaluatedExpressionFinding[],
   skip?: (key: string, value: string) => boolean
 ): void {
-  if (!bag || typeof bag !== 'object' || Array.isArray(bag)) return;
-  for (const [key, value] of Object.entries(bag as Record<string, unknown>)) {
+  if (!isConfigBag(bag)) return;
+  for (const [key, value] of Object.entries(bag)) {
     if (typeof value !== 'string') continue;
     if (skip?.(key, value)) continue;
     const expressions = findExpressionSources(value);
@@ -141,10 +143,7 @@ export function collectUnevaluatedExpressions(
 
   // The hoist copies every `properties.*` value onto the node's top level, so
   // the same residue is visible twice. Report the authored spelling only.
-  const hoisted =
-    properties && typeof properties === 'object' && !Array.isArray(properties)
-      ? (properties as Record<string, unknown>)
-      : undefined;
+  const hoisted = isConfigBag(properties) ? properties : undefined;
   scanBag(spread, 'schema', findings, (key, value) => hoisted?.[key] === value);
 
   scanBag(props, 'props', findings);