From 30e4e9f78561d1a91d4632d2fbb811bd29c5b6c5 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 03:09:09 +0200 Subject: [PATCH 1/6] Make dialog and toast specs independent of spec file execution order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite runs with `isolate: false`, so a module imported by one spec file stays in the registry with that file's mocks bound in, and vitest's file order is not stable between runs. Eight specs imported the module under test statically and therefore passed or failed depending on which file happened to load it first. Measured on an unmodified tree: three of six consecutive `yarn test` runs were red, failing in `Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result`, `for_CommandDialog/when_confirming_with_close_dialog_and_command_result` or `for_toastCommandResult/when_toasting_a_command_result`. A separate symptom of the same cause was an unhandled `commandInstance.execute is not a function` rejection, raised when a CommandDialog bound to another file's auto-firing Button mock reached a spec whose `useCommandInstance` returns no `execute`. That failed `yarn ci` with every test still reporting green. Each affected spec now re-evaluates the module under test inside `beforeEach` after `vi.resetModules()`, which is the idiom `when_validity_is_gated` and `when_step_has_field_errors` already used — those two never appeared in any failure. Ten consecutive runs are now clean with no unhandled errors. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DRCRgRvMz9N8P8NGwR34MM --- ...ng_with_close_dialog_and_command_result.ts | 34 ++++++++++++------- .../when_given_initial_valid_values.ts | 11 ++++-- .../for_CommandDialog/when_not_executing.ts | 11 ++++-- .../when_not_executing.ts | 13 +++++-- .../when_single_step.ts | 13 +++++-- ...confirming_with_close_dialog_and_result.ts | 11 ++++-- .../for_Dialog/when_rendered_with_is_busy.ts | 11 ++++-- .../when_toasting_a_command_result.ts | 20 ++++++++--- 8 files changed, 93 insertions(+), 31 deletions(-) diff --git a/Source/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts b/Source/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts index 43229a0..91c9b9a 100644 --- a/Source/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts +++ b/Source/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts @@ -5,7 +5,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; import { DialogResult, useDialogContext } from '@cratis/arc.react/dialogs'; -import { CommandDialog } from '../CommandDialog'; const { closeDialog, commandResult } = vi.hoisted(() => ({ closeDialog: vi.fn(), @@ -64,20 +63,29 @@ class TestCommand { name: string = ''; } -const TestDialog = () => { - const { closeDialog: closeWithResult } = useDialogContext(); - - return React.createElement(CommandDialog, { - command: TestCommand, - title: 'Update user', - onConfirm: async () => closeWithResult(DialogResult.Ok, commandResult), - onCancel: async () => closeWithResult(DialogResult.Cancelled), - }); -}; - describe('when confirming with close dialog and command result', () => { - beforeEach(() => { + beforeEach(async () => { closeDialog.mockReset(); + + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate CommandDialog + // under this file's own mocks so the confirm button is always the one that + // fires onClick — a static import here makes this spec pass or fail by luck. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + + const TestDialog = () => { + const { closeDialog: closeWithResult } = useDialogContext(); + + return React.createElement(CommandDialog, { + command: TestCommand, + title: 'Update user', + onConfirm: async () => closeWithResult(DialogResult.Ok, commandResult), + onCancel: async () => closeWithResult(DialogResult.Cancelled), + }); + }; + renderToStaticMarkup(React.createElement(TestDialog)); }); diff --git a/Source/CommandDialog/for_CommandDialog/when_given_initial_valid_values.ts b/Source/CommandDialog/for_CommandDialog/when_given_initial_valid_values.ts index 59b894f..539ddfc 100644 --- a/Source/CommandDialog/for_CommandDialog/when_given_initial_valid_values.ts +++ b/Source/CommandDialog/for_CommandDialog/when_given_initial_valid_values.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { CommandDialog } from '../CommandDialog'; vi.mock('primereact/dialog', () => { // PrimeReact 11's Dialog is compositional; each part is a pass-through that @@ -50,7 +49,15 @@ class TestCommand { describe('when CommandDialog is given initial valid values', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + const element = React.createElement(CommandDialog, { command: TestCommand as unknown as new () => object, initialValues: { name: 'John Doe' } as Partial, diff --git a/Source/CommandDialog/for_CommandDialog/when_not_executing.ts b/Source/CommandDialog/for_CommandDialog/when_not_executing.ts index 3850022..0b230e5 100644 --- a/Source/CommandDialog/for_CommandDialog/when_not_executing.ts +++ b/Source/CommandDialog/for_CommandDialog/when_not_executing.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { CommandDialog } from '../CommandDialog'; vi.mock('primereact/dialog', () => { // PrimeReact 11's Dialog is compositional; each part is a pass-through that @@ -51,7 +50,15 @@ class TestCommand { describe('when CommandDialog is in its initial state', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + const element = React.createElement(CommandDialog, { command: TestCommand as unknown as new () => object, visible: true, diff --git a/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts b/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts index 07d656c..a65cc2e 100644 --- a/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts +++ b/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts @@ -4,8 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { StepperCommandDialog } from '../StepperCommandDialog'; -import { StepperPanel } from '../StepperPanel'; // StepperCommandDialog now composes the Cratis Dialog wrapper (not primereact/dialog // directly) — render its custom footer (`buttons`) and body. @@ -66,7 +64,16 @@ class TestCommand { describe('when StepperCommandDialog is in its initial state', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { StepperCommandDialog } = await import('../StepperCommandDialog'); + const { StepperPanel } = await import('../StepperPanel'); + const element = React.createElement( StepperCommandDialog, { diff --git a/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts b/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts index c72789d..536378c 100644 --- a/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts +++ b/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts @@ -4,8 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { StepperCommandDialog } from '../StepperCommandDialog'; -import { StepperPanel } from '../StepperPanel'; vi.mock('../../Dialogs/Dialog', () => ({ Dialog: (props: { buttons?: React.ReactNode; children?: React.ReactNode }) => @@ -61,7 +59,16 @@ class TestCommand { describe('when StepperCommandDialog has a single step', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { StepperCommandDialog } = await import('../StepperCommandDialog'); + const { StepperPanel } = await import('../StepperPanel'); + const element = React.createElement( StepperCommandDialog, { diff --git a/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts b/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts index 6e6eba2..97ee419 100644 --- a/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts +++ b/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { Dialog } from '../Dialog'; const { closeDialog } = vi.hoisted(() => ({ closeDialog: vi.fn(), @@ -42,9 +41,17 @@ vi.mock('@cratis/arc.react/dialogs', () => ({ describe('when confirming with close dialog and result', () => { const resultPayload = { id: 'project-1', name: 'Project 1' }; - beforeEach(() => { + beforeEach(async () => { closeDialog.mockReset(); + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate Dialog under + // this file's own mocks so the confirm button is always the one that fires + // onClick — a static import here makes this spec pass or fail by luck. + vi.resetModules(); + const { Dialog } = await import('../Dialog'); + const element = React.createElement(Dialog, { title: 'Add project', visible: true, diff --git a/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts b/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts index 9d8405e..675c77c 100644 --- a/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts +++ b/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { Dialog } from '../Dialog'; vi.mock('primereact/dialog', () => { // PrimeReact 11's Dialog is compositional; each part is a pass-through that @@ -38,7 +37,15 @@ vi.mock('@cratis/arc.react/dialogs', () => ({ describe('when rendered with is busy', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { Dialog } = await import('../Dialog'); + const element = React.createElement(Dialog, { title: 'Save changes', visible: true, diff --git a/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts b/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts index 44f6634..8f839b1 100644 --- a/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts +++ b/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts @@ -4,9 +4,9 @@ import { vi } from 'vitest'; import type { ICommandResult } from '@cratis/arc/commands'; -// One file for all outcomes: the project runs specs with `isolate: false`, so a -// single module load + one mock avoids the cross-file mock caching that -// separate files (or vi.resetModules) would run into. +// One file for all outcomes: the project runs specs with `isolate: false`, so +// keeping every case behind one mock avoids multiplying the cross-file mock +// caching that separate files would each have to defend against. const { calls } = vi.hoisted(() => ({ calls: { success: [] as { title: string; description?: string }[], warn: [] as { title: string }[], error: [] as { title: string; description?: string }[] } })); vi.mock('primereact/toaster', () => ({ @@ -17,7 +17,19 @@ vi.mock('primereact/toaster', () => ({ }, })); -import { toastCommandResult } from '../toastCommandResult'; +// Loaded per test rather than statically imported: with `isolate: false` a module +// pulled in by an earlier spec file stays cached with that file's bindings, and +// `toastCommandResult` reaches `primereact/toaster` transitively through +// Common/CratisComponentsProvider — so a static import here binds the real toaster +// whenever another file happened to load that graph first, and the run order is not +// stable between runs. Re-evaluating under this file's own mock is what makes the +// spec deterministic. +let toastCommandResult: typeof import('../toastCommandResult').toastCommandResult; + +beforeEach(async () => { + vi.resetModules(); + ({ toastCommandResult } = await import('../toastCommandResult')); +}); const result = (over: Partial): ICommandResult => ({ isSuccess: false, isAuthorized: true, isValid: true, hasExceptions: false, validationResults: [], exceptionMessages: [], ...over } as unknown as ICommandResult); From 5175d0c7b4f39a7152c9bc68ba896c634b30af18 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 03:09:28 +0200 Subject: [PATCH 2/6] Identify CommandForm fields and columns by a marker, not only by displayName MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `CommandForm` child was classified as a field or a column by exactly one test: `component.displayName === 'CommandFormField'` (or `'CommandFormColumn'`). `displayName` is React's public, writable diagnostic name and a routine target for build tooling, so any transform that sets it unbinds every field — with no error, no warning and every gate green. The field then renders with no container: no label, no bound value, no change handler. This adds a marker that such a transform cannot reach, checked first, with the `displayName` comparison kept as a fallback: - `CommandFormFieldMarker` / `CommandFormColumnMarker` — `Symbol.for` registry keys, so `@cratis/arc.react` and `@cratis/components` resolve the same symbol without importing it from each other. A named import would be a hard module-link error against any version in this package's peer range that does not export it, and a plain `Symbol()` would give a duplicate install two keys that never compare equal. - `isCommandFormField` / `isCommandFormColumn` — marker first, `displayName` second — now used at all three read sites (`CommandDialog`, and both reads in `CommandStepper`). - `markAsCommandFormField` / `markAsCommandFormColumn` set the marker *and* the legacy `displayName`; `CommandDialog.Column` is stamped through the latter. The `displayName` path is retained indefinitely rather than deprecated. It is what lets these two independently versioned packages interoperate in both directions, and what keeps working every consumer who marks a field by hand. Removing it would reproduce the very failure this change prevents. Purely additive: no public API is removed and no existing consumer changes behaviour. This is the consumer half of the contract. The field marker only takes effect once `@cratis/arc.react` stamps it; until then every path here falls back to `displayName` exactly as before. Because both sides keep the fallback, the two packages may ship in either order without a skew hazard. Also documents that `displayName` is load-bearing on field and column components, including the Storybook `reactDocgen: 'react-docgen-typescript'` default that rewrites it and the `setDisplayName: false` setting that disables it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DRCRgRvMz9N8P8NGwR34MM --- Documentation/CommandForm/index.md | 58 ++++++++ Source/CommandDialog/CommandDialog.tsx | 5 +- Source/CommandDialog/CommandStepper.tsx | 5 +- ...ld_carries_only_the_legacy_display_name.ts | 92 ++++++++++++ .../when_field_display_name_is_overwritten.ts | 104 +++++++++++++ .../when_inspecting_the_column_wrapper.ts | 75 ++++++++++ ...epper_field_display_name_is_overwritten.ts | 137 ++++++++++++++++++ Source/CommandForm/commandFormMarkers.ts | 96 ++++++++++++ .../when_checking_marker_identity.ts | 28 ++++ .../when_display_name_is_overwritten.ts | 58 ++++++++ .../when_nothing_marks_the_component.ts | 32 ++++ ...only_the_legacy_display_name_is_present.ts | 45 ++++++ Source/CommandForm/index.ts | 1 + 13 files changed, 732 insertions(+), 4 deletions(-) create mode 100644 Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts create mode 100644 Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts create mode 100644 Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts create mode 100644 Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts create mode 100644 Source/CommandForm/commandFormMarkers.ts create mode 100644 Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts create mode 100644 Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts create mode 100644 Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts create mode 100644 Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts diff --git a/Documentation/CommandForm/index.md b/Documentation/CommandForm/index.md index b1de9d0..c3a8940 100644 --- a/Documentation/CommandForm/index.md +++ b/Documentation/CommandForm/index.md @@ -48,3 +48,61 @@ import { InputTextField, NumberField, CheckboxField } from '@cratis/components/C value={c => c.active} label="Active" /> ``` + +## How a child is recognised as a field + +`CommandForm`, `CommandDialog` and `CommandStepper` decide which of their children are +fields by inspecting the child's component type. A component is treated as a field when +it carries either of the following: + +- the `CommandFormFieldMarker` symbol set to `true` — what `asCommandFormField` and + `markAsCommandFormField` stamp; or +- the legacy `displayName` of `'CommandFormField'`, which is checked as a fallback and + is supported indefinitely. + +Columns work the same way, through `CommandFormColumnMarker` and `'CommandFormColumn'`. + +> [!IMPORTANT] +> **`displayName` is load-bearing on field and column components — never overwrite it.** +> A child whose `displayName` has been replaced and that carries no marker is not +> recognised as a field: it renders without its container, so it gets no label, no bound +> value and no change handler. This fails silently — there is no error and no warning. + +The marker exists so that this stops being fatal. Because it is a `Symbol.for` registry +key rather than a string property, a build-time transform that rewrites `displayName` +cannot reach it, and a field keeps working even after being renamed. + +### Build tooling that rewrites `displayName` + +The most common way to hit this is Storybook's +`reactDocgen: 'react-docgen-typescript'` setting, whose underlying plugin defaults +`setDisplayName` to `true` and appends `.displayName = ""` to +every module it processes. If you wrap fields of your own with `asCommandFormField`, +turn that off: + +```ts +// .storybook/main.ts +typescript: { + reactDocgen: 'react-docgen-typescript', + reactDocgenTypescriptOptions: { + setDisplayName: false + } +} +``` + +### Writing a custom field + +Prefer `asCommandFormField` from `@cratis/arc.react`, which marks the component for you. +To mark a component directly — when hand-rolling a field or a column — use the helpers: + +```tsx +import { markAsCommandFormField } from '@cratis/components/CommandForm'; + +const MyField = (props: { value?: (c: MyCommand) => unknown }) => { /* ... */ }; + +markAsCommandFormField(MyField); +``` + +Both helpers set the marker *and* the legacy `displayName`, so a component marked this +way is recognised by every version of `@cratis/arc.react` within this package's +supported range. diff --git a/Source/CommandDialog/CommandDialog.tsx b/Source/CommandDialog/CommandDialog.tsx index 3fe4d38..895a7b2 100644 --- a/Source/CommandDialog/CommandDialog.tsx +++ b/Source/CommandDialog/CommandDialog.tsx @@ -13,6 +13,7 @@ import { type CommandFormProps } from '@cratis/arc.react/commands'; import { applyBeforeExecute, type BeforeExecuteCallback } from './applyBeforeExecute'; +import { isCommandFormField, markAsCommandFormColumn } from '../CommandForm/commandFormMarkers'; /** * Props for {@link CommandDialog}. Combines the props of a `CommandForm` @@ -148,7 +149,7 @@ const CommandDialogWrapper = ({ if (!React.isValidElement(child)) return child; const component = child.type as React.ComponentType; - if (component.displayName === 'CommandFormField') { + if (isCommandFormField(component)) { type FieldElement = Parameters[0]['field']; return ; } @@ -349,7 +350,7 @@ const CommandDialogComponent = ( {children} ); -CommandDialogColumnWrapper.displayName = 'CommandFormColumn'; +markAsCommandFormColumn(CommandDialogColumnWrapper); CommandDialogComponent.Column = CommandDialogColumnWrapper; diff --git a/Source/CommandDialog/CommandStepper.tsx b/Source/CommandDialog/CommandStepper.tsx index 5cb880f..8550937 100644 --- a/Source/CommandDialog/CommandStepper.tsx +++ b/Source/CommandDialog/CommandStepper.tsx @@ -14,6 +14,7 @@ import { type CommandFormProps } from '@cratis/arc.react/commands'; import { applyBeforeExecute, type BeforeExecuteCallback } from './applyBeforeExecute'; +import { isCommandFormField } from '../CommandForm/commandFormMarkers'; import type { StepperPanelProps } from './StepperPanel'; import './CommandStepper.css'; @@ -145,7 +146,7 @@ const extractFieldNamesFromNode = (nodes: React.ReactNode): string[] => { React.Children.forEach(nodes, (child) => { if (!React.isValidElement(child)) return; const component = child.type as React.ComponentType; - if ((component as { displayName?: string }).displayName === 'CommandFormField') { + if (isCommandFormField(component)) { const fieldProps = child.props as { value?: (obj: unknown) => unknown }; const name = getPropertyName(fieldProps.value); if (name) names.push(name); @@ -164,7 +165,7 @@ const processChildren = (nodes: React.ReactNode): React.ReactNode => { if (!React.isValidElement(child)) return child; const component = child.type as React.ComponentType; - if ((component as { displayName?: string }).displayName === 'CommandFormField') { + if (isCommandFormField(component)) { type FieldElement = Parameters[0]['field']; return ; } diff --git a/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts b/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts new file mode 100644 index 0000000..81b3de8 --- /dev/null +++ b/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts @@ -0,0 +1,92 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { vi } from 'vitest'; +import { CommandFormFieldDisplayName } from '../../CommandForm/commandFormMarkers'; + +vi.mock('primereact/dialog', () => { + const part = (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children); + return { + Dialog: { + Root: part, Portal: part, Backdrop: part, Positioner: part, Popup: part, + Header: part, Title: part, Close: part, Content: part, Footer: part, + }, + }; +}); + +vi.mock('primereact/button', () => ({ + Button: (props: { disabled?: boolean; children?: React.ReactNode }) => + React.createElement('button', { disabled: props.disabled }, props.children), +})); + +vi.mock('@cratis/arc.react/dialogs', () => ({ + DialogButtons: { Ok: 1, OkCancel: 2, YesNo: 3, YesNoCancel: 4 }, + DialogResult: { None: 0, Yes: 1, No: 2, Ok: 3, Cancelled: 4 }, + useDialogContext: () => undefined, +})); + +vi.mock('@cratis/arc.react/commands', () => ({ + CommandForm: (props: { children?: React.ReactNode }) => + React.createElement('div', null, props.children), + useCommandFormContext: () => ({ + isValid: true, + setCommandValues: () => {}, + setCommandResult: () => {}, + }), + useCommandInstance: () => ({}), + // Tagged so the markup shows whether the dialog recognised the child as a + // field and wrapped it. An unrecognised child is returned untouched — no + // container, so no label, no bound value and no change handler. + CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => + React.createElement('div', { 'data-testid': 'field-wrapper' }, props.field), +})); + + +class TestCommand { + name: string = ''; +} + +// The compatibility case, and the one that keeps a new @cratis/components working +// against an older @cratis/arc.react: this field carries the legacy `displayName` +// and no marker at all, exactly as a hand-rolled field or a pre-marker release of +// Arc produces. Deleting the fallback would silently unbind every one of them. +const HandRolledField = (props: { value?: (c: TestCommand) => unknown }) => { + void props; + return React.createElement('input', { 'data-testid': 'the-field' }); +}; +HandRolledField.displayName = CommandFormFieldDisplayName; + +describe('when a field carries only the legacy displayName', () => { + let html: string; + + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate CommandDialog + // under this file's own mocks so the tagged CommandFormFieldWrapper is + // always the one in effect. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + + const element = React.createElement( + CommandDialog, + { + command: TestCommand as unknown as new () => object, + visible: true, + title: 'Test Dialog', + }, + React.createElement(HandRolledField, { value: (c: TestCommand) => c.name }) + ); + html = renderToStaticMarkup(element); + }); + + it('should_recognise_the_child_as_a_field_and_wrap_it', () => { + html.should.include('field-wrapper'); + }); + + it('should_render_the_field_itself', () => { + html.should.include('the-field'); + }); +}); diff --git a/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts b/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts new file mode 100644 index 0000000..5a54c7c --- /dev/null +++ b/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts @@ -0,0 +1,104 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { vi } from 'vitest'; +import { CommandFormFieldDisplayName, markAsCommandFormField } from '../../CommandForm/commandFormMarkers'; + +vi.mock('primereact/dialog', () => { + // PrimeReact 11's Dialog is compositional; each part is a pass-through that + // renders its children so the content reaches the markup. + const part = (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children); + return { + Dialog: { + Root: part, Portal: part, Backdrop: part, Positioner: part, Popup: part, + Header: part, Title: part, Close: part, Content: part, Footer: part, + }, + }; +}); + +vi.mock('primereact/button', () => ({ + Button: (props: { disabled?: boolean; children?: React.ReactNode }) => + React.createElement('button', { disabled: props.disabled }, props.children), +})); + +vi.mock('@cratis/arc.react/dialogs', () => ({ + DialogButtons: { Ok: 1, OkCancel: 2, YesNo: 3, YesNoCancel: 4 }, + DialogResult: { None: 0, Yes: 1, No: 2, Ok: 3, Cancelled: 4 }, + useDialogContext: () => undefined, +})); + +vi.mock('@cratis/arc.react/commands', () => ({ + CommandForm: (props: { children?: React.ReactNode }) => + React.createElement('div', null, props.children), + useCommandFormContext: () => ({ + isValid: true, + setCommandValues: () => {}, + setCommandResult: () => {}, + }), + useCommandInstance: () => ({}), + // Tagged so the markup shows whether the dialog recognised the child as a + // field and wrapped it. An unrecognised child is returned untouched — no + // container, so no label, no bound value and no change handler. + CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => + React.createElement('div', { 'data-testid': 'field-wrapper' }, props.field), +})); + + +const overwriteDisplayName = (component: object, name: string): void => { + (component as { displayName?: string }).displayName = name; +}; + +class TestCommand { + name: string = ''; +} + +// A field marked the way `asCommandFormField` marks one, whose `displayName` was +// then rewritten by a build transform — what Storybook's react-docgen-typescript +// integration does by default to every component it processes. +const RenamedField = markAsCommandFormField((props: { value?: (c: TestCommand) => unknown }) => { + void props; + return React.createElement('input', { 'data-testid': 'the-field' }); +}); +overwriteDisplayName(RenamedField, 'AppInputTextField'); + +describe('when a field displayName has been overwritten by a build transform', () => { + let html: string; + + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate CommandDialog + // under this file's own mocks so the tagged CommandFormFieldWrapper is + // always the one in effect. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + + const element = React.createElement( + CommandDialog, + { + command: TestCommand as unknown as new () => object, + visible: true, + title: 'Test Dialog', + }, + React.createElement(RenamedField, { value: (c: TestCommand) => c.name }) + ); + html = renderToStaticMarkup(element); + }); + + it('should_still_recognise_the_child_as_a_field_and_wrap_it', () => { + html.should.include('field-wrapper'); + }); + + it('should_still_render_the_field_itself', () => { + html.should.include('the-field'); + }); + + // Guards the two assertions above: were the overwrite to silently fail, they + // would pass through the legacy fallback and prove nothing about the marker. + it('should_have_actually_lost_the_legacy_display_name', () => { + (RenamedField as { displayName?: string }).displayName! + .should.not.equal(CommandFormFieldDisplayName); + }); +}); diff --git a/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts new file mode 100644 index 0000000..fbcbe55 --- /dev/null +++ b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts @@ -0,0 +1,75 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import React from 'react'; +import { vi } from 'vitest'; +import { + CommandFormColumnDisplayName, + CommandFormColumnMarker, + isCommandFormColumn, +} from '../../CommandForm/commandFormMarkers'; + +vi.mock('primereact/dialog', () => { + const part = (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children); + return { + Dialog: { + Root: part, Portal: part, Backdrop: part, Positioner: part, Popup: part, + Header: part, Title: part, Close: part, Content: part, Footer: part, + }, + }; +}); + +vi.mock('primereact/button', () => ({ + Button: (props: { disabled?: boolean; children?: React.ReactNode }) => + React.createElement('button', { disabled: props.disabled }, props.children), +})); + +vi.mock('@cratis/arc.react/dialogs', () => ({ + DialogButtons: { Ok: 1, OkCancel: 2, YesNo: 3, YesNoCancel: 4 }, + DialogResult: { None: 0, Yes: 1, No: 2, Ok: 3, Cancelled: 4 }, + useDialogContext: () => undefined, +})); + +vi.mock('@cratis/arc.react/commands', () => ({ + CommandForm: Object.assign( + (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children), + { Column: (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children) } + ), + useCommandFormContext: () => ({ + isValid: true, + setCommandValues: () => {}, + setCommandResult: () => {}, + }), + useCommandInstance: () => ({}), + CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => + React.createElement('div', null, props.field), +})); + +// This dialog's column wrapper is the one marker this package *writes* rather than +// reads: `CommandForm` in @cratis/arc.react is what classifies it. The assertions +// below are therefore the producer half of the cross-package contract — the field +// specs cover the consumer half. +describe('when inspecting the column wrapper', () => { + let Column: object; + + beforeEach(async () => { + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + Column = (CommandDialog as unknown as { Column: object }).Column; + }); + + it('should_carry_the_column_marker', () => { + (Column as Record)[CommandFormColumnMarker]!.should.equal(true); + }); + + it('should_be_recognised_as_a_column', () => { + isCommandFormColumn(Column).should.be.true; + }); + + // The legacy label has to stay: an @cratis/arc.react that predates the marker + // classifies columns by this string alone, and this package's peer range admits + // exactly those versions. Dropping it would silently unbind every column there. + it('should_still_carry_the_legacy_display_name_for_older_arc', () => { + (Column as { displayName?: string }).displayName!.should.equal(CommandFormColumnDisplayName); + }); +}); diff --git a/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts b/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts new file mode 100644 index 0000000..1fa83d9 --- /dev/null +++ b/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts @@ -0,0 +1,137 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { vi } from 'vitest'; +import { CommandFormFieldDisplayName, markAsCommandFormField } from '../../CommandForm/commandFormMarkers'; + +vi.mock('../../Dialogs/Dialog', () => ({ + Dialog: (props: { buttons?: React.ReactNode; children?: React.ReactNode }) => + React.createElement('div', { 'data-testid': 'dialog' }, props.buttons, props.children), +})); + +// PrimeReact 11's Stepper is compositional: each part renders its children, and +// the Number part forwards its inline `style` so specs can assert the per-step +// red/green indicator the wrapper applies directly to each step's number. +vi.mock('primereact/stepper', () => { + const part = (name: string) => { + const Component = (props: { children?: React.ReactNode; style?: React.CSSProperties }) => + React.createElement('div', { 'data-part': name, style: props.style }, props.children); + Component.displayName = name; + return Component; + }; + return { + Stepper: { + Root: part('root'), List: part('list'), Step: part('step'), + Header: part('header'), Number: part('number'), Title: part('title'), + Separator: part('separator'), Panels: part('panels'), Panel: part('panel'), + }, + }; +}); + +vi.mock('primereact/button', () => ({ + Button: (props: { children?: React.ReactNode; disabled?: boolean }) => + React.createElement('button', { disabled: props.disabled }, props.children), +})); + +vi.mock('@cratis/arc.react/dialogs', () => ({ + DialogButtons: { Ok: 1, OkCancel: 2, YesNo: 3, YesNoCancel: 4 }, + DialogResult: { None: 0, Yes: 1, No: 2, Ok: 3, Cancelled: 4 }, + useDialogContext: () => undefined, +})); + +// isValid: true — only getFieldError drives the per-step indicator, and it can +// only be consulted for a field whose name was successfully extracted. +vi.mock('@cratis/arc.react/commands', () => ({ + CommandForm: (props: { children?: React.ReactNode }) => + React.createElement('div', null, props.children), + useCommandFormContext: () => ({ + isValid: true, + setCommandValues: () => {}, + setCommandResult: () => {}, + getFieldError: (fieldName: string) => + fieldName === 'name' ? 'Name is required' : undefined, + }), + useCommandInstance: () => ({}), + // Tagged so the markup shows whether the stepper recognised the child as a + // field and wrapped it. An unrecognised child is returned untouched — no + // container, so no label, no bound value and no change handler. + CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => + React.createElement('div', { 'data-testid': 'field-wrapper' }, props.field), +})); + +const overwriteDisplayName = (component: object, name: string): void => { + (component as { displayName?: string }).displayName = name; +}; + +class TestCommand { + name: string = ''; + description: string = ''; +} + +// Marked as `asCommandFormField` marks a field, then renamed by a build transform. +const RenamedField = markAsCommandFormField((props: { value?: (c: TestCommand) => unknown }) => { + void props; + return React.createElement('div', { 'data-testid': 'the-field' }); +}); +overwriteDisplayName(RenamedField, 'AppInputTextField'); + +describe('when a stepper field displayName has been overwritten by a build transform', () => { + let html: string; + + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so the getFieldError stub driving the step indicator is + // always the one in effect. + vi.resetModules(); + const { StepperCommandDialog } = await import('../StepperCommandDialog'); + const { StepperPanel } = await import('../StepperPanel'); + + const element = React.createElement( + StepperCommandDialog, + { + command: TestCommand as unknown as new () => object, + visible: true, + title: 'Test Dialog', + }, + React.createElement( + StepperPanel, + { header: 'Step 1' }, + React.createElement(RenamedField, { value: (c: TestCommand) => c.name }) + ), + React.createElement(StepperPanel, { header: 'Step 2' }, 'No fields here') + ); + html = renderToStaticMarkup(element); + }); + + // Covers the read in processChildren. + it('should_still_recognise_the_child_as_a_field_and_wrap_it', () => { + html.should.include('field-wrapper'); + }); + + it('should_still_render_the_field_itself', () => { + html.should.include('the-field'); + }); + + // Covers the read in extractFieldNamesFromNode: the step indicator can only + // turn red if the field was recognised and its property name extracted. + it('should_still_extract_the_field_name_for_the_step_indicator', () => { + const step1Number = html.match(/]*>1<\/span>|
]*>1<\/div>/); + (step1Number?.[0] ?? '').should.include('red'); + }); + + it('should_not_mark_the_step_that_has_no_fields', () => { + const step2Number = html.match(/]*>2<\/span>|
]*>2<\/div>/); + (step2Number?.[0] ?? '').should.not.include('red'); + }); + + // Guards every assertion above: were the overwrite to silently fail, they + // would pass through the legacy fallback and prove nothing about the marker. + it('should_have_actually_lost_the_legacy_display_name', () => { + (RenamedField as { displayName?: string }).displayName! + .should.not.equal(CommandFormFieldDisplayName); + }); +}); diff --git a/Source/CommandForm/commandFormMarkers.ts b/Source/CommandForm/commandFormMarkers.ts new file mode 100644 index 0000000..a87ba6c --- /dev/null +++ b/Source/CommandForm/commandFormMarkers.ts @@ -0,0 +1,96 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +/** + * The registry key identifying a component as a `CommandForm` field. + * + * `Symbol.for` rather than `Symbol` is deliberate. The key is resolved through the + * global symbol registry, so `@cratis/arc.react` and `@cratis/components` arrive at + * the same symbol without either importing it from the other. That matters because + * the two packages are versioned independently — this one declares + * `@cratis/arc.react` as a range — so a named import would be a hard module-link + * error against any version that does not yet export it, and a duplicate install + * would otherwise produce two keys that never compare equal. + */ +export const CommandFormFieldMarker = Symbol.for('cratis.commandFormField'); + +/** + * The registry key identifying a component as a `CommandForm` column. + * See {@link CommandFormFieldMarker} for why this is a registry symbol. + */ +export const CommandFormColumnMarker = Symbol.for('cratis.commandFormColumn'); + +/** + * The `displayName` a `CommandForm` field has always carried. + * + * It is retained indefinitely rather than deprecated: it is the compatibility path + * for consumers that mark a field by hand, and it is what lets a new + * `@cratis/components` work against an older `@cratis/arc.react` that stamps nothing + * else. Removing it would silently unbind every such field — the exact failure the + * marker exists to prevent. + */ +export const CommandFormFieldDisplayName = 'CommandFormField'; + +/** The `displayName` a `CommandForm` column has always carried. See {@link CommandFormFieldDisplayName}. */ +export const CommandFormColumnDisplayName = 'CommandFormColumn'; + +/** The properties read when deciding what a `CommandForm` child is. */ +type CommandFormChild = { + displayName?: string; + [CommandFormFieldMarker]?: boolean; + [CommandFormColumnMarker]?: boolean; +}; + +/** + * Determines whether `component` is a `CommandForm` field. + * + * The marker is checked first and `displayName` second. `displayName` is public, + * writable, and a routine target for build tooling — Storybook's + * `reactDocgen: 'react-docgen-typescript'` setting rewrites it by default — so a + * component whose label has been rewritten by a third party is still recognised + * through the marker, while one carrying only the legacy label still works. + * + * @param component - The child's component type. Anything may be passed; host + * elements such as `'div'` and nullish values are simply not fields. + */ +export const isCommandFormField = (component: unknown): boolean => { + const candidate = component as CommandFormChild | undefined; + return candidate?.[CommandFormFieldMarker] === true + || candidate?.displayName === CommandFormFieldDisplayName; +}; + +/** + * Determines whether `component` is a `CommandForm` column. + * See {@link isCommandFormField} for the ordering and why it matters. + */ +export const isCommandFormColumn = (component: unknown): boolean => { + const candidate = component as CommandFormChild | undefined; + return candidate?.[CommandFormColumnMarker] === true + || candidate?.displayName === CommandFormColumnDisplayName; +}; + +/** + * Marks `component` as a `CommandForm` field, setting both the tamper-resistant + * marker and the legacy `displayName`, and returns it. + * + * Both are set on purpose: the marker is what survives a build transform, and the + * `displayName` is what an older `@cratis/arc.react` — which knows nothing of the + * marker — still needs in order to bind the field. + */ +export const markAsCommandFormField = (component: T): T => { + const target = component as T & CommandFormChild; + target[CommandFormFieldMarker] = true; + target.displayName = CommandFormFieldDisplayName; + return component; +}; + +/** + * Marks `component` as a `CommandForm` column, setting both the marker and the + * legacy `displayName`, and returns it. See {@link markAsCommandFormField}. + */ +export const markAsCommandFormColumn = (component: T): T => { + const target = component as T & CommandFormChild; + target[CommandFormColumnMarker] = true; + target.displayName = CommandFormColumnDisplayName; + return component; +}; diff --git a/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts b/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts new file mode 100644 index 0000000..df3fb27 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts @@ -0,0 +1,28 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { CommandFormColumnMarker, CommandFormFieldMarker } from '../commandFormMarkers'; + +// Why this spec exists: the markers are `Symbol.for` registry keys rather than +// plain `Symbol`s precisely so that two module instances — a duplicate install, a +// bundler that fails to dedupe, or `@cratis/arc.react` and `@cratis/components` +// each carrying their own copy — still agree on one key. A plain `Symbol()` would +// produce two keys that never compare equal, and every field marked by one +// instance would be invisible to the other: the same silent unbind the marker +// exists to prevent, reached by a different route. +// +// Resolving the key from the global registry here is exactly what a second +// instance of the module would do, so an equal result is the guarantee itself. +describe('when checking marker identity', () => { + it('should_resolve_the_field_marker_from_the_global_symbol_registry', () => { + (Symbol.for('cratis.commandFormField') === CommandFormFieldMarker).should.be.true; + }); + + it('should_resolve_the_column_marker_from_the_global_symbol_registry', () => { + (Symbol.for('cratis.commandFormColumn') === CommandFormColumnMarker).should.be.true; + }); + + it('should_keep_the_field_and_column_markers_distinct', () => { + (CommandFormFieldMarker === (CommandFormColumnMarker as symbol)).should.be.false; + }); +}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts b/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts new file mode 100644 index 0000000..768ff43 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts @@ -0,0 +1,58 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { + CommandFormColumnDisplayName, + CommandFormFieldDisplayName, + isCommandFormColumn, + isCommandFormField, + markAsCommandFormColumn, + markAsCommandFormField, +} from '../commandFormMarkers'; + +// Reproduces what a build-time transform does to a marked component. Storybook's +// `reactDocgen: 'react-docgen-typescript'` setting runs a plugin that defaults to +// appending `.displayName = ""` to every module it +// processes, silently replacing the label the framework stamped. +const overwriteDisplayName = (component: object, name: string): void => { + (component as { displayName?: string }).displayName = name; +}; + +describe('when a marked component has had its displayName overwritten', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = markAsCommandFormField(() => undefined); + column = markAsCommandFormColumn(() => undefined); + overwriteDisplayName(field, 'AppInputTextField'); + overwriteDisplayName(column, 'AppColumnWrapper'); + }); + + it('should_still_recognise_the_field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should_still_recognise_the_column', () => { + isCommandFormColumn(column).should.be.true; + }); + + it('should_not_mistake_a_field_for_a_column', () => { + isCommandFormColumn(field).should.be.false; + }); + + it('should_not_mistake_a_column_for_a_field', () => { + isCommandFormField(column).should.be.false; + }); + + // These two guard the specs above: if the overwrite silently failed to take, + // every assertion here would pass through the legacy `displayName` fallback + // and prove nothing about the marker. + it('should_have_actually_lost_the_legacy_field_display_name', () => { + (field as { displayName?: string }).displayName!.should.not.equal(CommandFormFieldDisplayName); + }); + + it('should_have_actually_lost_the_legacy_column_display_name', () => { + (column as { displayName?: string }).displayName!.should.not.equal(CommandFormColumnDisplayName); + }); +}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts new file mode 100644 index 0000000..edf8cb8 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts @@ -0,0 +1,32 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; + +describe('when nothing marks the component', () => { + it('should_not_recognise_an_unmarked_component', () => { + isCommandFormField(() => undefined).should.be.false; + }); + + it('should_not_recognise_an_unrelated_display_name', () => { + const component = () => undefined; + (component as { displayName?: string }).displayName = 'StepperPanel'; + isCommandFormField(component).should.be.false; + isCommandFormColumn(component).should.be.false; + }); + + // A child's `type` is a string for host elements such as `
`, and the + // predicates are called on every child a form is given, so neither of these + // may throw. + it('should_not_recognise_a_host_element', () => { + isCommandFormField('div').should.be.false; + isCommandFormColumn('div').should.be.false; + }); + + it('should_not_recognise_nullish_values', () => { + isCommandFormField(undefined).should.be.false; + isCommandFormField(null).should.be.false; + isCommandFormColumn(undefined).should.be.false; + isCommandFormColumn(null).should.be.false; + }); +}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts b/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts new file mode 100644 index 0000000..f44f70a --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts @@ -0,0 +1,45 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { + CommandFormColumnDisplayName, + CommandFormFieldDisplayName, + isCommandFormColumn, + isCommandFormField, +} from '../commandFormMarkers'; + +// The compatibility surface. A consumer marking a field by hand, and any +// `@cratis/arc.react` predating the marker, stamp nothing but the `displayName` — +// so this pins the fallback against a later cleanup that would delete it and +// silently unbind every such field. +const stampDisplayNameOnly = (name: string): object => { + const component = () => undefined; + (component as { displayName?: string }).displayName = name; + return component; +}; + +describe('when a component carries only the legacy displayName', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = stampDisplayNameOnly(CommandFormFieldDisplayName); + column = stampDisplayNameOnly(CommandFormColumnDisplayName); + }); + + it('should_recognise_the_field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should_recognise_the_column', () => { + isCommandFormColumn(column).should.be.true; + }); + + it('should_not_mistake_a_field_for_a_column', () => { + isCommandFormColumn(field).should.be.false; + }); + + it('should_not_mistake_a_column_for_a_field', () => { + isCommandFormField(column).should.be.false; + }); +}); diff --git a/Source/CommandForm/index.ts b/Source/CommandForm/index.ts index 76a65e1..c37851f 100644 --- a/Source/CommandForm/index.ts +++ b/Source/CommandForm/index.ts @@ -1,4 +1,5 @@ // Copyright (c) Cratis. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +export * from './commandFormMarkers'; export * from './fields'; From 2e4bc5cd8c1b9ca8fdc197eedee0e12ab803b40d Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 07:58:06 +0200 Subject: [PATCH 3/6] Broaden the CommandForm marker specs to both failure directions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass proved the marker path and left gaps around it. Adds: - a component carrying *only* the marker and no displayName at all — the mirror of the legacy-only case, and the one that shows the marker is sufficient by itself rather than merely corroborating the label; - what the marking helpers actually do — both identifiers set, no cross-marking, and the component returned is the one that was marked, since both call styles are used in this package; - strictness of the check: a marker of `false`, or of a truthy non-boolean, is not a marker; - a renamed field nested inside `CommandDialog.Column`, which reaches the field through `processChildren`' recursion rather than as a direct child — the arrangement the column API exists for; - `Symbol.keyFor` assertions on marker identity. That distinguishes `Symbol.for('x')` from `Symbol('x')`, which nothing else about the value does, and names the key the other package has to use. Those key strings are the whole cross-package contract: changing one breaks it while every exported identifier stays the same. Both directions are now mutation-proven. Reverting the predicates to legacy-string-only reds 11 tests across 7 files; removing the legacy fallback instead reds 7 across 5 — including the pre-existing `when_step_has_field_errors`, which stamps the string on a fake to make it a field and is exactly the canary for that breaking change. Also documents that the helpers replace any existing `displayName`, which is forced rather than incidental — an older Arc binds by that exact string — so a component needing its own diagnostic label cannot also be marked. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DRCRgRvMz9N8P8NGwR34MM --- ...n_a_renamed_field_is_nested_in_a_column.ts | 113 ++++++++++++++++++ Source/CommandForm/commandFormMarkers.ts | 10 +- .../when_checking_marker_identity.ts | 21 +++- .../when_marking_a_component.ts | 55 +++++++++ .../when_nothing_marks_the_component.ts | 15 ++- .../when_only_the_marker_is_present.ts | 52 ++++++++ 6 files changed, 259 insertions(+), 7 deletions(-) create mode 100644 Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts create mode 100644 Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts create mode 100644 Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts diff --git a/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts b/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts new file mode 100644 index 0000000..5e2eee7 --- /dev/null +++ b/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts @@ -0,0 +1,113 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { vi } from 'vitest'; +import { CommandFormFieldDisplayName, markAsCommandFormField } from '../../CommandForm/commandFormMarkers'; + +vi.mock('primereact/dialog', () => { + const part = (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children); + return { + Dialog: { + Root: part, Portal: part, Backdrop: part, Positioner: part, Popup: part, + Header: part, Title: part, Close: part, Content: part, Footer: part, + }, + }; +}); + +vi.mock('primereact/button', () => ({ + Button: (props: { disabled?: boolean; children?: React.ReactNode }) => + React.createElement('button', { disabled: props.disabled }, props.children), +})); + +vi.mock('@cratis/arc.react/dialogs', () => ({ + DialogButtons: { Ok: 1, OkCancel: 2, YesNo: 3, YesNoCancel: 4 }, + DialogResult: { None: 0, Yes: 1, No: 2, Ok: 3, Cancelled: 4 }, + useDialogContext: () => undefined, +})); + +vi.mock('@cratis/arc.react/commands', () => ({ + CommandForm: Object.assign( + (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children), + { + Column: (props: { children?: React.ReactNode }) => + React.createElement('div', { 'data-testid': 'column' }, props.children), + } + ), + useCommandFormContext: () => ({ + isValid: true, + setCommandValues: () => {}, + setCommandResult: () => {}, + }), + useCommandInstance: () => ({}), + CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => + React.createElement('div', { 'data-testid': 'field-wrapper' }, props.field), +})); + +const overwriteDisplayName = (component: object, name: string): void => { + (component as { displayName?: string }).displayName = name; +}; + +class TestCommand { + name: string = ''; +} + +const RenamedField = markAsCommandFormField((props: { value?: (c: TestCommand) => unknown }) => { + void props; + return React.createElement('input', { 'data-testid': 'the-field' }); +}); +overwriteDisplayName(RenamedField, 'AppInputTextField'); + +// `processChildren` only tests the child itself for fieldness; anything else with +// children it recurses into. A field inside `CommandDialog.Column` is therefore +// reached one level down, and a marker that worked only for direct children would +// still leave every column-laid-out form broken. This is the arrangement the +// column API exists for, so it gets its own spec rather than being assumed from +// the flat case. +describe('when a renamed field is nested in a column', () => { + let html: string; + + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate CommandDialog + // under this file's own mocks. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + + const element = React.createElement( + CommandDialog, + { + command: TestCommand as unknown as new () => object, + visible: true, + title: 'Test Dialog', + }, + React.createElement( + (CommandDialog as unknown as { Column: React.ComponentType<{ children?: React.ReactNode }> }).Column, + null, + React.createElement(RenamedField, { value: (c: TestCommand) => c.name }) + ) + ); + html = renderToStaticMarkup(element); + }); + + it('should_still_recognise_the_nested_field_and_wrap_it', () => { + html.should.include('field-wrapper'); + }); + + it('should_still_render_the_column_around_it', () => { + html.should.include('column'); + }); + + it('should_still_render_the_field_itself', () => { + html.should.include('the-field'); + }); + + // Guards the assertions above: were the overwrite to silently fail, they would + // pass through the legacy fallback and prove nothing about the marker. + it('should_have_actually_lost_the_legacy_display_name', () => { + (RenamedField as { displayName?: string }).displayName! + .should.not.equal(CommandFormFieldDisplayName); + }); +}); diff --git a/Source/CommandForm/commandFormMarkers.ts b/Source/CommandForm/commandFormMarkers.ts index a87ba6c..2e3d3ea 100644 --- a/Source/CommandForm/commandFormMarkers.ts +++ b/Source/CommandForm/commandFormMarkers.ts @@ -76,6 +76,13 @@ export const isCommandFormColumn = (component: unknown): boolean => { * Both are set on purpose: the marker is what survives a build transform, and the * `displayName` is what an older `@cratis/arc.react` — which knows nothing of the * marker — still needs in order to bind the field. + * + * Prefer `asCommandFormField` from `@cratis/arc.react` where it applies; it marks + * the wrapped component for you. Reach for this when hand-rolling a field. + * + * ⚠️ Any existing `displayName` is replaced. That is not incidental — an older Arc + * binds the field by that exact string — so a component needing its own diagnostic + * label cannot also be marked this way. */ export const markAsCommandFormField = (component: T): T => { const target = component as T & CommandFormChild; @@ -86,7 +93,8 @@ export const markAsCommandFormField = (component: T): T => { /** * Marks `component` as a `CommandForm` column, setting both the marker and the - * legacy `displayName`, and returns it. See {@link markAsCommandFormField}. + * legacy `displayName`, and returns it. Any existing `displayName` is replaced. + * See {@link markAsCommandFormField}. */ export const markAsCommandFormColumn = (component: T): T => { const target = component as T & CommandFormChild; diff --git a/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts b/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts index df3fb27..918ece9 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts @@ -11,18 +11,29 @@ import { CommandFormColumnMarker, CommandFormFieldMarker } from '../commandFormM // instance would be invisible to the other: the same silent unbind the marker // exists to prevent, reached by a different route. // -// Resolving the key from the global registry here is exactly what a second -// instance of the module would do, so an equal result is the guarantee itself. +// `Symbol.keyFor` returns undefined for any symbol outside the global registry, so +// it tells `Symbol.for('x')` apart from `Symbol('x')` — which nothing else about +// the value does — and it names the key the other package has to use. Those key +// strings are the whole cross-package contract: changing one is a breaking change +// even though no exported identifier changes. describe('when checking marker identity', () => { - it('should_resolve_the_field_marker_from_the_global_symbol_registry', () => { + it('should_register_the_field_marker_globally_under_its_documented_key', () => { + Symbol.keyFor(CommandFormFieldMarker)!.should.equal('cratis.commandFormField'); + }); + + it('should_register_the_column_marker_globally_under_its_documented_key', () => { + Symbol.keyFor(CommandFormColumnMarker)!.should.equal('cratis.commandFormColumn'); + }); + + it('should_resolve_the_field_marker_a_second_module_instance_would_compute', () => { (Symbol.for('cratis.commandFormField') === CommandFormFieldMarker).should.be.true; }); - it('should_resolve_the_column_marker_from_the_global_symbol_registry', () => { + it('should_resolve_the_column_marker_a_second_module_instance_would_compute', () => { (Symbol.for('cratis.commandFormColumn') === CommandFormColumnMarker).should.be.true; }); it('should_keep_the_field_and_column_markers_distinct', () => { - (CommandFormFieldMarker === (CommandFormColumnMarker as symbol)).should.be.false; + (CommandFormFieldMarker as symbol).should.not.equal(CommandFormColumnMarker as symbol); }); }); diff --git a/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts new file mode 100644 index 0000000..af787e8 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts @@ -0,0 +1,55 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { + CommandFormColumnDisplayName, + CommandFormColumnMarker, + CommandFormFieldDisplayName, + CommandFormFieldMarker, + markAsCommandFormColumn, + markAsCommandFormField, +} from '../commandFormMarkers'; + +// The helpers set *both* identifiers, and both halves matter. The marker is what +// survives a build transform; the legacy `displayName` is what an older +// `@cratis/arc.react` — which knows nothing of the marker — needs in order to bind +// the component at all. This package's peer range admits exactly those versions, +// so a helper that set only the marker would silently unbind every component it +// touched on a perfectly supported Arc. +describe('when marking a component', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = markAsCommandFormField(() => undefined); + column = markAsCommandFormColumn(() => undefined); + }); + + it('should_set_the_field_marker', () => { + (field as Record)[CommandFormFieldMarker]!.should.equal(true); + }); + + it('should_set_the_column_marker', () => { + (column as Record)[CommandFormColumnMarker]!.should.equal(true); + }); + + it('should_also_set_the_legacy_field_display_name_for_older_arc', () => { + (field as { displayName?: string }).displayName!.should.equal(CommandFormFieldDisplayName); + }); + + it('should_also_set_the_legacy_column_display_name_for_older_arc', () => { + (column as { displayName?: string }).displayName!.should.equal(CommandFormColumnDisplayName); + }); + + it('should_not_cross_mark_a_field_with_the_column_marker', () => { + ((field as Record)[CommandFormColumnMarker] === undefined).should.be.true; + }); + + // The helpers mark in place and hand the component back, so both + // `markAsCommandFormField(C)` as a statement and `const C = markAs...(fn)` as an + // expression mark the same object — the two call styles used across this package. + it('should_return_the_very_component_it_marked', () => { + const component = () => undefined; + markAsCommandFormField(component).should.equal(component); + }); +}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts index edf8cb8..945bda9 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts @@ -1,7 +1,7 @@ // Copyright (c) Cratis. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -import { isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; +import { CommandFormFieldMarker, isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; describe('when nothing marks the component', () => { it('should_not_recognise_an_unmarked_component', () => { @@ -23,6 +23,19 @@ describe('when nothing marks the component', () => { isCommandFormColumn('div').should.be.false; }); + // Pins the `=== true` comparison rather than a truthiness check. A component + // that deliberately carries `marker = false` is opting out, and must not be + // recognised through some other value that merely happens to be present. + it('should_not_recognise_a_marker_that_is_not_true', () => { + const disabled = () => undefined; + (disabled as unknown as Record)[CommandFormFieldMarker] = false; + isCommandFormField(disabled).should.be.false; + + const wrongType = () => undefined; + (wrongType as unknown as Record)[CommandFormFieldMarker] = 'yes'; + isCommandFormField(wrongType).should.be.false; + }); + it('should_not_recognise_nullish_values', () => { isCommandFormField(undefined).should.be.false; isCommandFormField(null).should.be.false; diff --git a/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts new file mode 100644 index 0000000..81fe358 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts @@ -0,0 +1,52 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { + CommandFormColumnMarker, + CommandFormFieldMarker, + isCommandFormColumn, + isCommandFormField, +} from '../commandFormMarkers'; + +// The mirror of when_only_the_legacy_display_name_is_present: a component carrying +// the marker and no `displayName` whatsoever. This is the case a build transform +// cannot produce by renaming, and the one that proves the marker is sufficient on +// its own rather than merely corroborating the legacy label. +const stampMarkerOnly = (marker: symbol): object => { + const component = () => undefined; + (component as unknown as Record)[marker] = true; + return component; +}; + +describe('when a component carries only the marker', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = stampMarkerOnly(CommandFormFieldMarker); + column = stampMarkerOnly(CommandFormColumnMarker); + }); + + it('should_recognise_the_field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should_recognise_the_column', () => { + isCommandFormColumn(column).should.be.true; + }); + + it('should_not_mistake_a_field_for_a_column', () => { + isCommandFormColumn(field).should.be.false; + }); + + it('should_not_mistake_a_column_for_a_field', () => { + isCommandFormField(column).should.be.false; + }); + + // Guards the four assertions above: if a `displayName` had leaked onto these + // components they would pass through the legacy fallback instead. + it('should_have_no_display_name_at_all', () => { + ((field as { displayName?: string }).displayName === undefined).should.be.true; + ((column as { displayName?: string }).displayName === undefined).should.be.true; + }); +}); From a69b74ae6008cf40acee9a3a45bcf0987849a163 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 07:58:21 +0200 Subject: [PATCH 4/6] Add no-raw-command-form-marker rule to the components ESLint plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The marker makes the right thing possible; this makes the wrong thing visible. Consumers hand-roll `CommandForm` fields, and the failure mode being fixed is silent — a renamed component simply stops being a field, with no error, no warning and every gate green — so a lint rule is the only place it surfaces at authoring time. Flags identifying a field or column by a hand-written `displayName` string in either direction: stamping it (assignment, computed access, or object-literal form as in `Object.assign`) and comparing against it (`===`/`!==`, either operand order, including the `(x as { displayName?: string })` cast form this package itself used). Points at `markAsCommandFormField`/`markAsCommandFormColumn` and `isCommandFormField`/`isCommandFormColumn`, naming the right helper for the string that was written. Referring to the exported `CommandFormFieldDisplayName` / `CommandFormColumnDisplayName` constants is not flagged, so the declarations themselves and any deliberate legacy-path code stay clean. Going through the helpers is strictly more permissive than the literal, never less — they still set and honour the legacy `displayName` — so the rule never trades compatibility for safety. Verified end to end through the ESLint Linter, not only RuleTester: all three shapes this repo carried before the marker existed are reported, each naming the correct helper. 21 rule tests added. This repo's own eslint config does not load the plugin, so this changes no gate here; it is published surface for consumers and is enabled in `configs.recommended` alongside the existing four rules. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DRCRgRvMz9N8P8NGwR34MM --- ESLint/README.md | 37 +++++++++++- ESLint/index.js | 5 +- ESLint/lib/noRawCommandFormMarker.js | 87 ++++++++++++++++++++++++++++ ESLint/package.json | 2 +- ESLint/test/rules.test.js | 64 ++++++++++++++++++++ 5 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 ESLint/lib/noRawCommandFormMarker.js diff --git a/ESLint/README.md b/ESLint/README.md index fe36d64..da2ce22 100644 --- a/ESLint/README.md +++ b/ESLint/README.md @@ -9,6 +9,7 @@ Cratis base config, [`@cratis/eslint-config`](https://www.npmjs.com/package/@cra | `no-primereact-dialog` | Disallows importing `Dialog` from `primereact/dialog`. Use `CommandDialog` from `@cratis/components/CommandDialog`, or `Dialog` from `@cratis/components/Dialogs` — the wrappers add Arc command binding, overlay/focus fixes, and theming. | | `onbeforeexecute-must-return` | Requires an `onBeforeExecute` callback to return the command values. `onBeforeExecute` is a transformer — a body that can complete without returning executes the command with `undefined` (silent data loss). | | `no-hooks-in-view-model` | Disallows React hooks (including generated Arc proxies' `.use()`) inside a view model class. View models must be plain, hook-free classes that receive injected abstractions. | +| `no-raw-command-form-marker` | Disallows identifying a CommandForm field or column by a hand-written `displayName` string, in either direction. Use `markAsCommandFormField`/`markAsCommandFormColumn` and `isCommandFormField`/`isCommandFormColumn` from `@cratis/components/CommandForm` — they go through a marker a build transform cannot rewrite. | The two import rules cover `import` and re-`export … from` forms. @@ -44,7 +45,7 @@ export default [ }], ``` -`onbeforeexecute-must-return` and `no-hooks-in-view-model` take no options. +`onbeforeexecute-must-return`, `no-hooks-in-view-model` and `no-raw-command-form-marker` take no options. ## Rules @@ -96,3 +97,37 @@ A class is treated as a view model when it is registered via `withViewModel(...) with `@injectable`, or named `*ViewModel`. Both bare hooks (`useState`, `useIdentity`, …) and proxy member hooks (`.use()`, `.useSuspense()`, `.useChangeStream()`) are flagged. Hooks inside a nested non–view-model class are not. + +### `no-raw-command-form-marker` + +`CommandForm`, `CommandDialog` and `CommandStepper` decide which children are fields by +inspecting the child's component type. Historically that test was a single string +comparison against `displayName` — and `displayName` is React's public, writable +*diagnostic* name, a routine target for build tooling. Storybook's +`reactDocgen: 'react-docgen-typescript'` setting rewrites it by default. + +When it is rewritten, the child stops being recognised as a field: it renders with no +container, so no label, no bound value and no change handler. There is no error and no +warning, and every gate stays green. + +The helpers go through a `Symbol.for` marker that a rename cannot reach, while still +setting and honouring the legacy `displayName` — so they are strictly more permissive +than the literal, never less. + +```ts +// ❌ a build transform that rewrites displayName silently unbinds this field +MyField.displayName = 'CommandFormField'; +if (component.displayName === 'CommandFormField') { wrap(component); } + +// ✅ marker first, legacy displayName still set and still honoured +import { markAsCommandFormField, isCommandFormField } from '@cratis/components/CommandForm'; + +markAsCommandFormField(MyField); +if (isCommandFormField(component)) { wrap(component); } +``` + +Flagged in both directions: assignment (`C.displayName = '…'`, including computed and +object-literal forms) and comparison (`===`, `!==`, either operand order). Referring to the +exported `CommandFormFieldDisplayName` / `CommandFormColumnDisplayName` constants is not +flagged, so the declarations themselves and any deliberate legacy-path code stay clean. +Prefer `asCommandFormField` from `@cratis/arc.react` where it applies — it marks for you. diff --git a/ESLint/index.js b/ESLint/index.js index 4547c53..0115852 100644 --- a/ESLint/index.js +++ b/ESLint/index.js @@ -3,6 +3,7 @@ import { noPrimereactDialog } from './lib/noPrimereactDialog.js'; import { noRootBarrelImport } from './lib/noRootBarrelImport.js'; import { onbeforeexecuteMustReturn } from './lib/onbeforeexecuteMustReturn.js'; import { noHooksInViewModel } from './lib/noHooksInViewModel.js'; +import { noRawCommandFormMarker } from './lib/noRawCommandFormMarker.js'; const { version } = createRequire(import.meta.url)('./package.json'); @@ -17,6 +18,7 @@ const plugin = { 'no-root-barrel-import': noRootBarrelImport, 'onbeforeexecute-must-return': onbeforeexecuteMustReturn, 'no-hooks-in-view-model': noHooksInViewModel, + 'no-raw-command-form-marker': noRawCommandFormMarker, }, configs: {}, }; @@ -37,6 +39,7 @@ Object.assign(plugin.configs, { '@cratis/components/no-root-barrel-import': 'error', '@cratis/components/onbeforeexecute-must-return': 'error', '@cratis/components/no-hooks-in-view-model': 'error', + '@cratis/components/no-raw-command-form-marker': 'error', }, }, ], @@ -44,4 +47,4 @@ Object.assign(plugin.configs, { export default plugin; export const { configs, rules, meta } = plugin; -export { noPrimereactDialog, noRootBarrelImport, onbeforeexecuteMustReturn, noHooksInViewModel }; +export { noPrimereactDialog, noRootBarrelImport, onbeforeexecuteMustReturn, noHooksInViewModel, noRawCommandFormMarker }; diff --git a/ESLint/lib/noRawCommandFormMarker.js b/ESLint/lib/noRawCommandFormMarker.js new file mode 100644 index 0000000..bc381aa --- /dev/null +++ b/ESLint/lib/noRawCommandFormMarker.js @@ -0,0 +1,87 @@ +// The identifiers a CommandForm child is recognised by, and the helpers that own each one. +const MARKERS = { + CommandFormField: { mark: 'markAsCommandFormField', predicate: 'isCommandFormField' }, + CommandFormColumn: { mark: 'markAsCommandFormColumn', predicate: 'isCommandFormColumn' }, +}; + +const EQUALITY = new Set(['===', '!==', '==', '!=']); + +// True for `x.displayName` and `x['displayName']`. +const isDisplayNameMember = node => + node?.type === 'MemberExpression' && + (node.computed + ? node.property?.type === 'Literal' && node.property.value === 'displayName' + : node.property?.type === 'Identifier' && node.property.name === 'displayName'); + +const markerFor = node => + node?.type === 'Literal' && typeof node.value === 'string' + ? MARKERS[node.value] + : undefined; + +// Disallow hand-writing the `displayName` strings that identify a CommandForm field or +// column, in either direction — stamping one onto a component, or comparing against one to +// decide what a child is. +// +// `displayName` is React's public, writable diagnostic name and a routine target for build +// tooling: Storybook's `reactDocgen: 'react-docgen-typescript'` setting rewrites it by +// default. A component identified only by that string stops being recognised the moment +// anything renames it, and it fails silently — the field simply renders with no container, +// so no label, no bound value and no change handler, with no error and no warning. +// +// The helpers in '@cratis/components/CommandForm' set and read a `Symbol.for` marker that a +// rename cannot reach, and keep the legacy `displayName` alongside it for compatibility, so +// going through them is both safer and strictly more permissive than the literal. +export const noRawCommandFormMarker = { + meta: { + type: 'problem', + docs: { + description: 'Disallow identifying CommandForm fields and columns by a raw displayName string; use the marker helpers.', + recommended: true, + url: 'https://github.com/Cratis/Components/blob/main/ESLint/README.md', + }, + schema: [], + messages: { + useMarkHelper: + "Do not stamp displayName = '{{name}}' by hand — a build transform that rewrites displayName silently unbinds this component. Use {{helper}}() from '@cratis/components/CommandForm'.", + usePredicate: + "Do not identify a CommandForm child by comparing displayName to '{{name}}' — a component whose displayName was rewritten is missed. Use {{helper}}() from '@cratis/components/CommandForm'.", + }, + }, + create(context) { + const report = (node, messageId, helper, name) => + context.report({ node, messageId, data: { helper, name } }); + + return { + // C.displayName = 'CommandFormField' + AssignmentExpression(node) { + if (node.operator !== '=' || !isDisplayNameMember(node.left)) return; + const marker = markerFor(node.right); + if (marker) report(node, 'useMarkHelper', marker.mark, node.right.value); + }, + + // { displayName: 'CommandFormField' } — e.g. via Object.assign + Property(node) { + const key = node.computed ? undefined : node.key; + const named = + (key?.type === 'Identifier' && key.name === 'displayName') || + (key?.type === 'Literal' && key.value === 'displayName'); + if (!named) return; + const marker = markerFor(node.value); + if (marker) report(node, 'useMarkHelper', marker.mark, node.value.value); + }, + + // child.displayName === 'CommandFormField' (either operand order) + BinaryExpression(node) { + if (!EQUALITY.has(node.operator)) return; + const [member, literal] = isDisplayNameMember(node.left) + ? [node.left, node.right] + : [node.right, node.left]; + if (!isDisplayNameMember(member)) return; + const marker = markerFor(literal); + if (marker) report(node, 'usePredicate', marker.predicate, literal.value); + }, + }; + }, +}; + +export default noRawCommandFormMarker; diff --git a/ESLint/package.json b/ESLint/package.json index f94bdbb..4a87a7c 100644 --- a/ESLint/package.json +++ b/ESLint/package.json @@ -1,7 +1,7 @@ { "name": "@cratis/eslint-plugin-components", "version": "0.0.0", - "description": "Cratis Components ESLint rules: import from subpaths not the root barrel, use the Cratis dialog wrappers instead of primereact/dialog, require onBeforeExecute callbacks to return their values, and keep React hooks out of view models. Compose on top of @cratis/eslint-config.", + "description": "Cratis Components ESLint rules: import from subpaths not the root barrel, use the Cratis dialog wrappers instead of primereact/dialog, require onBeforeExecute callbacks to return their values, keep React hooks out of view models, and identify CommandForm fields by a tamper-resistant marker rather than a raw displayName string. Compose on top of @cratis/eslint-config.", "author": "Cratis", "license": "MIT", "type": "module", diff --git a/ESLint/test/rules.test.js b/ESLint/test/rules.test.js index 4c550d2..6e2603c 100644 --- a/ESLint/test/rules.test.js +++ b/ESLint/test/rules.test.js @@ -5,6 +5,7 @@ import { noPrimereactDialog } from '../lib/noPrimereactDialog.js'; import { noRootBarrelImport } from '../lib/noRootBarrelImport.js'; import { onbeforeexecuteMustReturn } from '../lib/onbeforeexecuteMustReturn.js'; import { noHooksInViewModel } from '../lib/noHooksInViewModel.js'; +import { noRawCommandFormMarker } from '../lib/noRawCommandFormMarker.js'; RuleTester.afterAll = afterAll; RuleTester.describe = describe; @@ -142,3 +143,66 @@ tsRuleTester.run('no-hooks-in-view-model', noHooksInViewModel, { }, ], }); + +tsRuleTester.run('no-raw-command-form-marker', noRawCommandFormMarker, { + valid: [ + // The sanctioned way to mark and to test, in both directions. + "markAsCommandFormField(MyField);", + "markAsCommandFormColumn(MyColumn);", + "if (isCommandFormField(component)) { wrap(component); }", + "if (isCommandFormColumn(component)) { layout(component); }", + // Referring to the exported constant rather than repeating the literal. + "MyField.displayName = CommandFormFieldDisplayName;", + "if (component.displayName === CommandFormFieldDisplayName) { wrap(component); }", + // Declaring the constants themselves — the one place the literal belongs. + "export const CommandFormFieldDisplayName = 'CommandFormField';", + "export const CommandFormColumnDisplayName = 'CommandFormColumn';", + // displayName used as the diagnostic label it is meant to be. + "MyDialog.displayName = 'MyDialog';", + "StepperPanel.displayName = 'StepperPanel';", + "const label = `DialogWrapper(${Component.displayName})`;", + // A different property that happens to hold the same string. + "const meta = { kind: 'CommandFormField' };", + // Comparing a non-displayName property. + "if (component.name === 'CommandFormField') { legacy(component); }", + ], + invalid: [ + { + code: "MyField.displayName = 'CommandFormField';", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormField', name: 'CommandFormField' } }], + }, + { + code: "MyColumn.displayName = 'CommandFormColumn';", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormColumn', name: 'CommandFormColumn' } }], + }, + { + // Computed member access is the same stamp. + code: "MyField['displayName'] = 'CommandFormField';", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormField', name: 'CommandFormField' } }], + }, + { + // Set through an object literal, e.g. Object.assign. + code: "Object.assign(MyField, { displayName: 'CommandFormField' });", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormField', name: 'CommandFormField' } }], + }, + { + code: "if (component.displayName === 'CommandFormField') { wrap(component); }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormField', name: 'CommandFormField' } }], + }, + { + // Reversed operand order. + code: "if ('CommandFormColumn' === component.displayName) { layout(component); }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormColumn', name: 'CommandFormColumn' } }], + }, + { + // Negated comparison misses a renamed field just as badly. + code: "if (component.displayName !== 'CommandFormField') { return child; }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormField', name: 'CommandFormField' } }], + }, + { + // The cast form this package used before the marker existed. + code: "if ((component as { displayName?: string }).displayName === 'CommandFormField') { wrap(component); }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormField', name: 'CommandFormField' } }], + }, + ], +}); From 107ec26284acf5f7d767552cd541f18f21882b98 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 08:24:12 +0200 Subject: [PATCH 5/6] Conform new marker code and specs to the AI corpus conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two rules under .ai/rules were missed when this work was written. American English (general.md, typescript.md "Language — American English Only"): "recognise"/"recognised" become "recognize"/"recognized" across the marker module, its specs, the ESLint rule and both docs pages. `Cancelled` is left alone — that is the spelling of Arc's DialogResult member, an API name rather than prose. Spaces in it() descriptions (specs.typescript.md "Naming Conventions", where it('should_return_invalid_result') is the explicit counter-example): every it() in the new specs becomes a readable sentence. The pre-existing underscore descriptions elsewhere are left untouched — they predate this work, and the repo already runs 157 space-style descriptions against 57 underscore ones, so the convention followed here is also the majority one. No behavior change; identifiers, assertions and control flow are untouched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DRCRgRvMz9N8P8NGwR34MM --- Documentation/CommandForm/index.md | 6 +++--- ESLint/README.md | 2 +- ESLint/lib/noRawCommandFormMarker.js | 4 ++-- ...when_a_renamed_field_is_nested_in_a_column.ts | 8 ++++---- ...field_carries_only_the_legacy_display_name.ts | 8 ++++---- .../when_field_display_name_is_overwritten.ts | 10 +++++----- .../when_inspecting_the_column_wrapper.ts | 6 +++--- ..._stepper_field_display_name_is_overwritten.ts | 16 ++++++++-------- Source/CommandForm/commandFormMarkers.ts | 2 +- .../when_checking_marker_identity.ts | 10 +++++----- .../when_display_name_is_overwritten.ts | 12 ++++++------ .../when_marking_a_component.ts | 12 ++++++------ .../when_nothing_marks_the_component.ts | 12 ++++++------ ...en_only_the_legacy_display_name_is_present.ts | 8 ++++---- .../when_only_the_marker_is_present.ts | 10 +++++----- 15 files changed, 63 insertions(+), 63 deletions(-) diff --git a/Documentation/CommandForm/index.md b/Documentation/CommandForm/index.md index c3a8940..7b917b9 100644 --- a/Documentation/CommandForm/index.md +++ b/Documentation/CommandForm/index.md @@ -49,7 +49,7 @@ import { InputTextField, NumberField, CheckboxField } from '@cratis/components/C ``` -## How a child is recognised as a field +## How a child is recognized as a field `CommandForm`, `CommandDialog` and `CommandStepper` decide which of their children are fields by inspecting the child's component type. A component is treated as a field when @@ -65,7 +65,7 @@ Columns work the same way, through `CommandFormColumnMarker` and `'CommandFormCo > [!IMPORTANT] > **`displayName` is load-bearing on field and column components — never overwrite it.** > A child whose `displayName` has been replaced and that carries no marker is not -> recognised as a field: it renders without its container, so it gets no label, no bound +> recognized as a field: it renders without its container, so it gets no label, no bound > value and no change handler. This fails silently — there is no error and no warning. The marker exists so that this stops being fatal. Because it is a `Symbol.for` registry @@ -104,5 +104,5 @@ markAsCommandFormField(MyField); ``` Both helpers set the marker *and* the legacy `displayName`, so a component marked this -way is recognised by every version of `@cratis/arc.react` within this package's +way is recognized by every version of `@cratis/arc.react` within this package's supported range. diff --git a/ESLint/README.md b/ESLint/README.md index da2ce22..e05cb1a 100644 --- a/ESLint/README.md +++ b/ESLint/README.md @@ -106,7 +106,7 @@ comparison against `displayName` — and `displayName` is React's public, writab *diagnostic* name, a routine target for build tooling. Storybook's `reactDocgen: 'react-docgen-typescript'` setting rewrites it by default. -When it is rewritten, the child stops being recognised as a field: it renders with no +When it is rewritten, the child stops being recognized as a field: it renders with no container, so no label, no bound value and no change handler. There is no error and no warning, and every gate stays green. diff --git a/ESLint/lib/noRawCommandFormMarker.js b/ESLint/lib/noRawCommandFormMarker.js index bc381aa..abf6417 100644 --- a/ESLint/lib/noRawCommandFormMarker.js +++ b/ESLint/lib/noRawCommandFormMarker.js @@ -1,4 +1,4 @@ -// The identifiers a CommandForm child is recognised by, and the helpers that own each one. +// The identifiers a CommandForm child is recognized by, and the helpers that own each one. const MARKERS = { CommandFormField: { mark: 'markAsCommandFormField', predicate: 'isCommandFormField' }, CommandFormColumn: { mark: 'markAsCommandFormColumn', predicate: 'isCommandFormColumn' }, @@ -24,7 +24,7 @@ const markerFor = node => // // `displayName` is React's public, writable diagnostic name and a routine target for build // tooling: Storybook's `reactDocgen: 'react-docgen-typescript'` setting rewrites it by -// default. A component identified only by that string stops being recognised the moment +// default. A component identified only by that string stops being recognized the moment // anything renames it, and it fails silently — the field simply renders with no container, // so no label, no bound value and no change handler, with no error and no warning. // diff --git a/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts b/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts index 5e2eee7..88b4b27 100644 --- a/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts +++ b/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts @@ -92,21 +92,21 @@ describe('when a renamed field is nested in a column', () => { html = renderToStaticMarkup(element); }); - it('should_still_recognise_the_nested_field_and_wrap_it', () => { + it('should still recognize the nested field and wrap it', () => { html.should.include('field-wrapper'); }); - it('should_still_render_the_column_around_it', () => { + it('should still render the column around it', () => { html.should.include('column'); }); - it('should_still_render_the_field_itself', () => { + it('should still render the field itself', () => { html.should.include('the-field'); }); // Guards the assertions above: were the overwrite to silently fail, they would // pass through the legacy fallback and prove nothing about the marker. - it('should_have_actually_lost_the_legacy_display_name', () => { + it('should have actually lost the legacy display name', () => { (RenamedField as { displayName?: string }).displayName! .should.not.equal(CommandFormFieldDisplayName); }); diff --git a/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts b/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts index 81b3de8..b050b30 100644 --- a/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts +++ b/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts @@ -36,8 +36,8 @@ vi.mock('@cratis/arc.react/commands', () => ({ setCommandResult: () => {}, }), useCommandInstance: () => ({}), - // Tagged so the markup shows whether the dialog recognised the child as a - // field and wrapped it. An unrecognised child is returned untouched — no + // Tagged so the markup shows whether the dialog recognized the child as a + // field and wrapped it. An unrecognized child is returned untouched — no // container, so no label, no bound value and no change handler. CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => React.createElement('div', { 'data-testid': 'field-wrapper' }, props.field), @@ -82,11 +82,11 @@ describe('when a field carries only the legacy displayName', () => { html = renderToStaticMarkup(element); }); - it('should_recognise_the_child_as_a_field_and_wrap_it', () => { + it('should recognize the child as a field and wrap it', () => { html.should.include('field-wrapper'); }); - it('should_render_the_field_itself', () => { + it('should render the field itself', () => { html.should.include('the-field'); }); }); diff --git a/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts b/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts index 5a54c7c..212d2cf 100644 --- a/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts +++ b/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts @@ -38,8 +38,8 @@ vi.mock('@cratis/arc.react/commands', () => ({ setCommandResult: () => {}, }), useCommandInstance: () => ({}), - // Tagged so the markup shows whether the dialog recognised the child as a - // field and wrapped it. An unrecognised child is returned untouched — no + // Tagged so the markup shows whether the dialog recognized the child as a + // field and wrapped it. An unrecognized child is returned untouched — no // container, so no label, no bound value and no change handler. CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => React.createElement('div', { 'data-testid': 'field-wrapper' }, props.field), @@ -87,17 +87,17 @@ describe('when a field displayName has been overwritten by a build transform', ( html = renderToStaticMarkup(element); }); - it('should_still_recognise_the_child_as_a_field_and_wrap_it', () => { + it('should still recognize the child as a field and wrap it', () => { html.should.include('field-wrapper'); }); - it('should_still_render_the_field_itself', () => { + it('should still render the field itself', () => { html.should.include('the-field'); }); // Guards the two assertions above: were the overwrite to silently fail, they // would pass through the legacy fallback and prove nothing about the marker. - it('should_have_actually_lost_the_legacy_display_name', () => { + it('should have actually lost the legacy display name', () => { (RenamedField as { displayName?: string }).displayName! .should.not.equal(CommandFormFieldDisplayName); }); diff --git a/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts index fbcbe55..405959e 100644 --- a/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts +++ b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts @@ -58,18 +58,18 @@ describe('when inspecting the column wrapper', () => { Column = (CommandDialog as unknown as { Column: object }).Column; }); - it('should_carry_the_column_marker', () => { + it('should carry the column marker', () => { (Column as Record)[CommandFormColumnMarker]!.should.equal(true); }); - it('should_be_recognised_as_a_column', () => { + it('should be recognized as a column', () => { isCommandFormColumn(Column).should.be.true; }); // The legacy label has to stay: an @cratis/arc.react that predates the marker // classifies columns by this string alone, and this package's peer range admits // exactly those versions. Dropping it would silently unbind every column there. - it('should_still_carry_the_legacy_display_name_for_older_arc', () => { + it('should still carry the legacy display name for older arc', () => { (Column as { displayName?: string }).displayName!.should.equal(CommandFormColumnDisplayName); }); }); diff --git a/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts b/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts index 1fa83d9..00531df 100644 --- a/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts +++ b/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts @@ -54,8 +54,8 @@ vi.mock('@cratis/arc.react/commands', () => ({ fieldName === 'name' ? 'Name is required' : undefined, }), useCommandInstance: () => ({}), - // Tagged so the markup shows whether the stepper recognised the child as a - // field and wrapped it. An unrecognised child is returned untouched — no + // Tagged so the markup shows whether the stepper recognized the child as a + // field and wrapped it. An unrecognized child is returned untouched — no // container, so no label, no bound value and no change handler. CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => React.createElement('div', { 'data-testid': 'field-wrapper' }, props.field), @@ -108,29 +108,29 @@ describe('when a stepper field displayName has been overwritten by a build trans }); // Covers the read in processChildren. - it('should_still_recognise_the_child_as_a_field_and_wrap_it', () => { + it('should still recognize the child as a field and wrap it', () => { html.should.include('field-wrapper'); }); - it('should_still_render_the_field_itself', () => { + it('should still render the field itself', () => { html.should.include('the-field'); }); // Covers the read in extractFieldNamesFromNode: the step indicator can only - // turn red if the field was recognised and its property name extracted. - it('should_still_extract_the_field_name_for_the_step_indicator', () => { + // turn red if the field was recognized and its property name extracted. + it('should still extract the field name for the step indicator', () => { const step1Number = html.match(/]*>1<\/span>|
]*>1<\/div>/); (step1Number?.[0] ?? '').should.include('red'); }); - it('should_not_mark_the_step_that_has_no_fields', () => { + it('should not mark the step that has no fields', () => { const step2Number = html.match(/]*>2<\/span>|
]*>2<\/div>/); (step2Number?.[0] ?? '').should.not.include('red'); }); // Guards every assertion above: were the overwrite to silently fail, they // would pass through the legacy fallback and prove nothing about the marker. - it('should_have_actually_lost_the_legacy_display_name', () => { + it('should have actually lost the legacy display name', () => { (RenamedField as { displayName?: string }).displayName! .should.not.equal(CommandFormFieldDisplayName); }); diff --git a/Source/CommandForm/commandFormMarkers.ts b/Source/CommandForm/commandFormMarkers.ts index 2e3d3ea..c49114f 100644 --- a/Source/CommandForm/commandFormMarkers.ts +++ b/Source/CommandForm/commandFormMarkers.ts @@ -47,7 +47,7 @@ type CommandFormChild = { * The marker is checked first and `displayName` second. `displayName` is public, * writable, and a routine target for build tooling — Storybook's * `reactDocgen: 'react-docgen-typescript'` setting rewrites it by default — so a - * component whose label has been rewritten by a third party is still recognised + * component whose label has been rewritten by a third party is still recognized * through the marker, while one carrying only the legacy label still works. * * @param component - The child's component type. Anything may be passed; host diff --git a/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts b/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts index 918ece9..83eefeb 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts @@ -17,23 +17,23 @@ import { CommandFormColumnMarker, CommandFormFieldMarker } from '../commandFormM // strings are the whole cross-package contract: changing one is a breaking change // even though no exported identifier changes. describe('when checking marker identity', () => { - it('should_register_the_field_marker_globally_under_its_documented_key', () => { + it('should register the field marker globally under its documented key', () => { Symbol.keyFor(CommandFormFieldMarker)!.should.equal('cratis.commandFormField'); }); - it('should_register_the_column_marker_globally_under_its_documented_key', () => { + it('should register the column marker globally under its documented key', () => { Symbol.keyFor(CommandFormColumnMarker)!.should.equal('cratis.commandFormColumn'); }); - it('should_resolve_the_field_marker_a_second_module_instance_would_compute', () => { + it('should resolve the field marker a second module instance would compute', () => { (Symbol.for('cratis.commandFormField') === CommandFormFieldMarker).should.be.true; }); - it('should_resolve_the_column_marker_a_second_module_instance_would_compute', () => { + it('should resolve the column marker a second module instance would compute', () => { (Symbol.for('cratis.commandFormColumn') === CommandFormColumnMarker).should.be.true; }); - it('should_keep_the_field_and_column_markers_distinct', () => { + it('should keep the field and column markers distinct', () => { (CommandFormFieldMarker as symbol).should.not.equal(CommandFormColumnMarker as symbol); }); }); diff --git a/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts b/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts index 768ff43..fee7c02 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts @@ -29,30 +29,30 @@ describe('when a marked component has had its displayName overwritten', () => { overwriteDisplayName(column, 'AppColumnWrapper'); }); - it('should_still_recognise_the_field', () => { + it('should still recognize the field', () => { isCommandFormField(field).should.be.true; }); - it('should_still_recognise_the_column', () => { + it('should still recognize the column', () => { isCommandFormColumn(column).should.be.true; }); - it('should_not_mistake_a_field_for_a_column', () => { + it('should not mistake a field for a column', () => { isCommandFormColumn(field).should.be.false; }); - it('should_not_mistake_a_column_for_a_field', () => { + it('should not mistake a column for a field', () => { isCommandFormField(column).should.be.false; }); // These two guard the specs above: if the overwrite silently failed to take, // every assertion here would pass through the legacy `displayName` fallback // and prove nothing about the marker. - it('should_have_actually_lost_the_legacy_field_display_name', () => { + it('should have actually lost the legacy field display name', () => { (field as { displayName?: string }).displayName!.should.not.equal(CommandFormFieldDisplayName); }); - it('should_have_actually_lost_the_legacy_column_display_name', () => { + it('should have actually lost the legacy column display name', () => { (column as { displayName?: string }).displayName!.should.not.equal(CommandFormColumnDisplayName); }); }); diff --git a/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts index af787e8..b29a19b 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts @@ -25,30 +25,30 @@ describe('when marking a component', () => { column = markAsCommandFormColumn(() => undefined); }); - it('should_set_the_field_marker', () => { + it('should set the field marker', () => { (field as Record)[CommandFormFieldMarker]!.should.equal(true); }); - it('should_set_the_column_marker', () => { + it('should set the column marker', () => { (column as Record)[CommandFormColumnMarker]!.should.equal(true); }); - it('should_also_set_the_legacy_field_display_name_for_older_arc', () => { + it('should also set the legacy field display name for older arc', () => { (field as { displayName?: string }).displayName!.should.equal(CommandFormFieldDisplayName); }); - it('should_also_set_the_legacy_column_display_name_for_older_arc', () => { + it('should also set the legacy column display name for older arc', () => { (column as { displayName?: string }).displayName!.should.equal(CommandFormColumnDisplayName); }); - it('should_not_cross_mark_a_field_with_the_column_marker', () => { + it('should not cross mark a field with the column marker', () => { ((field as Record)[CommandFormColumnMarker] === undefined).should.be.true; }); // The helpers mark in place and hand the component back, so both // `markAsCommandFormField(C)` as a statement and `const C = markAs...(fn)` as an // expression mark the same object — the two call styles used across this package. - it('should_return_the_very_component_it_marked', () => { + it('should return the very component it marked', () => { const component = () => undefined; markAsCommandFormField(component).should.equal(component); }); diff --git a/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts index 945bda9..6c8f4d9 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts @@ -4,11 +4,11 @@ import { CommandFormFieldMarker, isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; describe('when nothing marks the component', () => { - it('should_not_recognise_an_unmarked_component', () => { + it('should not recognize an unmarked component', () => { isCommandFormField(() => undefined).should.be.false; }); - it('should_not_recognise_an_unrelated_display_name', () => { + it('should not recognize an unrelated display name', () => { const component = () => undefined; (component as { displayName?: string }).displayName = 'StepperPanel'; isCommandFormField(component).should.be.false; @@ -18,15 +18,15 @@ describe('when nothing marks the component', () => { // A child's `type` is a string for host elements such as `
`, and the // predicates are called on every child a form is given, so neither of these // may throw. - it('should_not_recognise_a_host_element', () => { + it('should not recognize a host element', () => { isCommandFormField('div').should.be.false; isCommandFormColumn('div').should.be.false; }); // Pins the `=== true` comparison rather than a truthiness check. A component // that deliberately carries `marker = false` is opting out, and must not be - // recognised through some other value that merely happens to be present. - it('should_not_recognise_a_marker_that_is_not_true', () => { + // recognized through some other value that merely happens to be present. + it('should not recognize a marker that is not true', () => { const disabled = () => undefined; (disabled as unknown as Record)[CommandFormFieldMarker] = false; isCommandFormField(disabled).should.be.false; @@ -36,7 +36,7 @@ describe('when nothing marks the component', () => { isCommandFormField(wrongType).should.be.false; }); - it('should_not_recognise_nullish_values', () => { + it('should not recognize nullish values', () => { isCommandFormField(undefined).should.be.false; isCommandFormField(null).should.be.false; isCommandFormColumn(undefined).should.be.false; diff --git a/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts b/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts index f44f70a..aab8661 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts @@ -27,19 +27,19 @@ describe('when a component carries only the legacy displayName', () => { column = stampDisplayNameOnly(CommandFormColumnDisplayName); }); - it('should_recognise_the_field', () => { + it('should recognize the field', () => { isCommandFormField(field).should.be.true; }); - it('should_recognise_the_column', () => { + it('should recognize the column', () => { isCommandFormColumn(column).should.be.true; }); - it('should_not_mistake_a_field_for_a_column', () => { + it('should not mistake a field for a column', () => { isCommandFormColumn(field).should.be.false; }); - it('should_not_mistake_a_column_for_a_field', () => { + it('should not mistake a column for a field', () => { isCommandFormField(column).should.be.false; }); }); diff --git a/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts index 81fe358..966ede9 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts @@ -27,25 +27,25 @@ describe('when a component carries only the marker', () => { column = stampMarkerOnly(CommandFormColumnMarker); }); - it('should_recognise_the_field', () => { + it('should recognize the field', () => { isCommandFormField(field).should.be.true; }); - it('should_recognise_the_column', () => { + it('should recognize the column', () => { isCommandFormColumn(column).should.be.true; }); - it('should_not_mistake_a_field_for_a_column', () => { + it('should not mistake a field for a column', () => { isCommandFormColumn(field).should.be.false; }); - it('should_not_mistake_a_column_for_a_field', () => { + it('should not mistake a column for a field', () => { isCommandFormField(column).should.be.false; }); // Guards the four assertions above: if a `displayName` had leaked onto these // components they would pass through the legacy fallback instead. - it('should_have_no_display_name_at_all', () => { + it('should have no display name at all', () => { ((field as { displayName?: string }).displayName === undefined).should.be.true; ((column as { displayName?: string }).displayName === undefined).should.be.true; }); From b3c4840214b59bc32bccc33e96716971e25a9cd9 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 10:34:10 +0200 Subject: [PATCH 6/6] Align the CommandForm marker with the shape arc uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @cratis/arc.react marks fields and columns with `isCommandFormField` and `isCommandFormColumn` boolean properties. This package had reached for `Symbol.for` registry keys instead, and the two markers cannot see each other. Nothing threw, because both sides kept the legacy `displayName` fallback — but that is what hid the defect. The marker did nothing across the package boundary, so a field whose `displayName` a build transform had rewritten still bound in a bare `CommandForm` and silently unbound inside a `CommandDialog` or `CommandStepper`: the exact failure the marker was added to prevent, surviving the fix, with every spec in both packages passing. Arc's shape wins because arc owns the contract — it defines `asCommandFormField` and `CommandForm` — and because the argument for the Symbol does not hold up. A plain property needs no cross-package import either, since either side can test `isCommandFormField === true` locally, so it gives up none of the version decoupling; and no build transform renames arbitrary static properties, only `displayName`, which is the whole hazard. `CommandFormMarked` is duplicated here rather than imported: the peer range on @cratis/arc.react spans versions that do not export it, so a named import would be a hard module-link error rather than a graceful degrade. Adds the spec neither package had — a component marked the way arc marks one, with its displayName then overwritten, is recognized here; and one marked here carries the exact property names arc reads. Renaming either marker now reds that spec, where before it changed nothing observable in either repo. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DRCRgRvMz9N8P8NGwR34MM --- Documentation/CommandForm/index.md | 11 +- ESLint/README.md | 7 +- ESLint/lib/noRawCommandFormMarker.js | 8 +- .../when_inspecting_the_column_wrapper.ts | 8 +- Source/CommandForm/commandFormMarkers.ts | 138 +++++++++--------- .../when_checking_marker_identity.ts | 39 ----- ...n_exchanging_marked_components_with_arc.ts | 93 ++++++++++++ .../when_marking_a_component.ts | 8 +- .../when_nothing_marks_the_component.ts | 6 +- .../when_only_the_marker_is_present.ts | 15 +- 10 files changed, 186 insertions(+), 147 deletions(-) delete mode 100644 Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts create mode 100644 Source/CommandForm/for_commandFormMarkers/when_exchanging_marked_components_with_arc.ts diff --git a/Documentation/CommandForm/index.md b/Documentation/CommandForm/index.md index 7b917b9..52af1ab 100644 --- a/Documentation/CommandForm/index.md +++ b/Documentation/CommandForm/index.md @@ -55,12 +55,12 @@ import { InputTextField, NumberField, CheckboxField } from '@cratis/components/C fields by inspecting the child's component type. A component is treated as a field when it carries either of the following: -- the `CommandFormFieldMarker` symbol set to `true` — what `asCommandFormField` and +- an `isCommandFormField` property set to `true` — what `asCommandFormField` and `markAsCommandFormField` stamp; or - the legacy `displayName` of `'CommandFormField'`, which is checked as a fallback and is supported indefinitely. -Columns work the same way, through `CommandFormColumnMarker` and `'CommandFormColumn'`. +Columns work the same way, through `isCommandFormColumn` and `'CommandFormColumn'`. > [!IMPORTANT] > **`displayName` is load-bearing on field and column components — never overwrite it.** @@ -68,9 +68,10 @@ Columns work the same way, through `CommandFormColumnMarker` and `'CommandFormCo > recognized as a field: it renders without its container, so it gets no label, no bound > value and no change handler. This fails silently — there is no error and no warning. -The marker exists so that this stops being fatal. Because it is a `Symbol.for` registry -key rather than a string property, a build-time transform that rewrites `displayName` -cannot reach it, and a field keeps working even after being renamed. +The marker exists so that this stops being fatal. A build-time transform rewrites +`displayName` and nothing else, so it leaves the marker alone and a field keeps working +even after being renamed. `@cratis/arc.react` marks and reads the same two properties, so a +component marked by either package is recognized by both. ### Build tooling that rewrites `displayName` diff --git a/ESLint/README.md b/ESLint/README.md index e05cb1a..53987cc 100644 --- a/ESLint/README.md +++ b/ESLint/README.md @@ -110,9 +110,10 @@ When it is rewritten, the child stops being recognized as a field: it renders wi container, so no label, no bound value and no change handler. There is no error and no warning, and every gate stays green. -The helpers go through a `Symbol.for` marker that a rename cannot reach, while still -setting and honouring the legacy `displayName` — so they are strictly more permissive -than the literal, never less. +The helpers go through an `isCommandFormField` / `isCommandFormColumn` marker that a rename +does not touch, while still setting and honoring the legacy `displayName` — so they are strictly +more permissive than the literal, never less. `@cratis/arc.react` marks and reads the same two +properties, and that shared shape is what carries the contract across the two packages. ```ts // ❌ a build transform that rewrites displayName silently unbinds this field diff --git a/ESLint/lib/noRawCommandFormMarker.js b/ESLint/lib/noRawCommandFormMarker.js index abf6417..89db0ea 100644 --- a/ESLint/lib/noRawCommandFormMarker.js +++ b/ESLint/lib/noRawCommandFormMarker.js @@ -28,9 +28,11 @@ const markerFor = node => // anything renames it, and it fails silently — the field simply renders with no container, // so no label, no bound value and no change handler, with no error and no warning. // -// The helpers in '@cratis/components/CommandForm' set and read a `Symbol.for` marker that a -// rename cannot reach, and keep the legacy `displayName` alongside it for compatibility, so -// going through them is both safer and strictly more permissive than the literal. +// The helpers in '@cratis/components/CommandForm' set and read an `isCommandFormField` / +// `isCommandFormColumn` marker that a rename does not touch, and keep the legacy `displayName` +// alongside it for compatibility, so going through them is both safer and strictly more +// permissive than the literal. `@cratis/arc.react` marks and reads the same two properties; +// that shared shape is what carries the contract between the packages. export const noRawCommandFormMarker = { meta: { type: 'problem', diff --git a/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts index 405959e..5713e45 100644 --- a/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts +++ b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts @@ -3,11 +3,7 @@ import React from 'react'; import { vi } from 'vitest'; -import { - CommandFormColumnDisplayName, - CommandFormColumnMarker, - isCommandFormColumn, -} from '../../CommandForm/commandFormMarkers'; +import { CommandFormColumnDisplayName, isCommandFormColumn } from '../../CommandForm/commandFormMarkers'; vi.mock('primereact/dialog', () => { const part = (props: { children?: React.ReactNode }) => React.createElement('div', null, props.children); @@ -59,7 +55,7 @@ describe('when inspecting the column wrapper', () => { }); it('should carry the column marker', () => { - (Column as Record)[CommandFormColumnMarker]!.should.equal(true); + (Column as { isCommandFormColumn?: boolean }).isCommandFormColumn!.should.equal(true); }); it('should be recognized as a column', () => { diff --git a/Source/CommandForm/commandFormMarkers.ts b/Source/CommandForm/commandFormMarkers.ts index c49114f..5bfa796 100644 --- a/Source/CommandForm/commandFormMarkers.ts +++ b/Source/CommandForm/commandFormMarkers.ts @@ -2,103 +2,95 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. /** - * The registry key identifying a component as a `CommandForm` field. + * The `displayName` a command form field carries. * - * `Symbol.for` rather than `Symbol` is deliberate. The key is resolved through the - * global symbol registry, so `@cratis/arc.react` and `@cratis/components` arrive at - * the same symbol without either importing it from the other. That matters because - * the two packages are versioned independently — this one declares - * `@cratis/arc.react` as a range — so a named import would be a hard module-link - * error against any version that does not yet export it, and a duplicate install - * would otherwise produce two keys that never compare equal. + * Exported so a consumer recognizing a field has a constant to compare against rather than a + * duplicated string literal. */ -export const CommandFormFieldMarker = Symbol.for('cratis.commandFormField'); +export const CommandFormFieldDisplayName = 'CommandFormField'; -/** - * The registry key identifying a component as a `CommandForm` column. - * See {@link CommandFormFieldMarker} for why this is a registry symbol. - */ -export const CommandFormColumnMarker = Symbol.for('cratis.commandFormColumn'); +/** The `displayName` a command form column carries. */ +export const CommandFormColumnDisplayName = 'CommandFormColumn'; /** - * The `displayName` a `CommandForm` field has always carried. + * The shape a component carries to say what it is to a `CommandForm`. * - * It is retained indefinitely rather than deprecated: it is the compatibility path - * for consumers that mark a field by hand, and it is what lets a new - * `@cratis/components` work against an older `@cratis/arc.react` that stamps nothing - * else. Removing it would silently unbind every such field — the exact failure the - * marker exists to prevent. + * This is the cross-package contract, and it is defined identically here and in + * `@cratis/arc.react`. It is duplicated rather than imported on purpose: this package declares + * `@cratis/arc.react` as a version range, so a named import would be a hard module-link error + * against any version in that range predating the marker. Both packages therefore describe the + * same shape independently, and neither has to know the other's version. + * + * ⚠️ Changing a property name here is a breaking change to that contract even though nothing in + * this package stops compiling — the other package simply stops seeing the marker, and every + * field whose `displayName` a build transform has rewritten silently unbinds. */ -export const CommandFormFieldDisplayName = 'CommandFormField'; +export type CommandFormMarked = { + /** Set on a field component. */ + isCommandFormField?: boolean; -/** The `displayName` a `CommandForm` column has always carried. See {@link CommandFormFieldDisplayName}. */ -export const CommandFormColumnDisplayName = 'CommandFormColumn'; + /** Set on a column component. */ + isCommandFormColumn?: boolean; -/** The properties read when deciding what a `CommandForm` child is. */ -type CommandFormChild = { + /** The React display name, kept as the compatibility fallback. */ displayName?: string; - [CommandFormFieldMarker]?: boolean; - [CommandFormColumnMarker]?: boolean; }; /** - * Determines whether `component` is a `CommandForm` field. + * Marks a component as a command form field, and returns it. + * + * Sets both the marker and the `displayName`. The `displayName` is not redundant and is not on a + * deprecation path: it is what lets a version of this package interoperate with a version of + * `@cratis/arc.react` that only knows the string, in both directions. Removing it would silently + * unbind every field across that version boundary — the very failure the marker exists to prevent. * - * The marker is checked first and `displayName` second. `displayName` is public, - * writable, and a routine target for build tooling — Storybook's - * `reactDocgen: 'react-docgen-typescript'` setting rewrites it by default — so a - * component whose label has been rewritten by a third party is still recognized - * through the marker, while one carrying only the legacy label still works. + * Prefer `asCommandFormField` from `@cratis/arc.react` where it applies; it marks the wrapped + * component for you. Reach for this when hand-rolling a field. * - * @param component - The child's component type. Anything may be passed; host - * elements such as `'div'` and nullish values are simply not fields. + * ⚠️ Any existing `displayName` is replaced. That is forced rather than incidental — a version of + * `@cratis/arc.react` predating the marker binds the field by that exact string — so a component + * needing its own diagnostic label cannot also be marked this way. */ -export const isCommandFormField = (component: unknown): boolean => { - const candidate = component as CommandFormChild | undefined; - return candidate?.[CommandFormFieldMarker] === true - || candidate?.displayName === CommandFormFieldDisplayName; -}; +export function markAsCommandFormField(component: T): T & CommandFormMarked { + const marked = component as T & CommandFormMarked; + marked.isCommandFormField = true; + marked.displayName = CommandFormFieldDisplayName; + return marked; +} /** - * Determines whether `component` is a `CommandForm` column. - * See {@link isCommandFormField} for the ordering and why it matters. + * Marks a component as a command form column, and returns it. Any existing `displayName` is + * replaced. See {@link markAsCommandFormField}. */ -export const isCommandFormColumn = (component: unknown): boolean => { - const candidate = component as CommandFormChild | undefined; - return candidate?.[CommandFormColumnMarker] === true - || candidate?.displayName === CommandFormColumnDisplayName; -}; +export function markAsCommandFormColumn(component: T): T & CommandFormMarked { + const marked = component as T & CommandFormMarked; + marked.isCommandFormColumn = true; + marked.displayName = CommandFormColumnDisplayName; + return marked; +} /** - * Marks `component` as a `CommandForm` field, setting both the tamper-resistant - * marker and the legacy `displayName`, and returns it. - * - * Both are set on purpose: the marker is what survives a build transform, and the - * `displayName` is what an older `@cratis/arc.react` — which knows nothing of the - * marker — still needs in order to bind the field. + * Whether a component is a command form field. * - * Prefer `asCommandFormField` from `@cratis/arc.react` where it applies; it marks - * the wrapped component for you. Reach for this when hand-rolling a field. + * The marker is checked first and the `displayName` second. A build transform that rewrites + * `displayName` — which `react-docgen-typescript` does by default, and Storybook selects it + * through a documented option — leaves the marker alone, so a field survives where it used to + * unbind silently. The fallback keeps a hand-marked component, and a component from a version of + * `@cratis/arc.react` that predates the marker, working exactly as before. * - * ⚠️ Any existing `displayName` is replaced. That is not incidental — an older Arc - * binds the field by that exact string — so a component needing its own diagnostic - * label cannot also be marked this way. + * @param component - The child's component type. Anything may be passed: this runs over every + * child a form is given, and a host element's type is a string rather than a component. */ -export const markAsCommandFormField = (component: T): T => { - const target = component as T & CommandFormChild; - target[CommandFormFieldMarker] = true; - target.displayName = CommandFormFieldDisplayName; - return component; -}; +export function isCommandFormField(component: unknown): boolean { + const candidate = component as CommandFormMarked | undefined; + return candidate?.isCommandFormField === true || candidate?.displayName === CommandFormFieldDisplayName; +} /** - * Marks `component` as a `CommandForm` column, setting both the marker and the - * legacy `displayName`, and returns it. Any existing `displayName` is replaced. - * See {@link markAsCommandFormField}. + * Whether a component is a command form column. + * See {@link isCommandFormField} for the ordering and why it matters. */ -export const markAsCommandFormColumn = (component: T): T => { - const target = component as T & CommandFormChild; - target[CommandFormColumnMarker] = true; - target.displayName = CommandFormColumnDisplayName; - return component; -}; +export function isCommandFormColumn(component: unknown): boolean { + const candidate = component as CommandFormMarked | undefined; + return candidate?.isCommandFormColumn === true || candidate?.displayName === CommandFormColumnDisplayName; +} diff --git a/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts b/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts deleted file mode 100644 index 83eefeb..0000000 --- a/Source/CommandForm/for_commandFormMarkers/when_checking_marker_identity.ts +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) Cratis. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -import { CommandFormColumnMarker, CommandFormFieldMarker } from '../commandFormMarkers'; - -// Why this spec exists: the markers are `Symbol.for` registry keys rather than -// plain `Symbol`s precisely so that two module instances — a duplicate install, a -// bundler that fails to dedupe, or `@cratis/arc.react` and `@cratis/components` -// each carrying their own copy — still agree on one key. A plain `Symbol()` would -// produce two keys that never compare equal, and every field marked by one -// instance would be invisible to the other: the same silent unbind the marker -// exists to prevent, reached by a different route. -// -// `Symbol.keyFor` returns undefined for any symbol outside the global registry, so -// it tells `Symbol.for('x')` apart from `Symbol('x')` — which nothing else about -// the value does — and it names the key the other package has to use. Those key -// strings are the whole cross-package contract: changing one is a breaking change -// even though no exported identifier changes. -describe('when checking marker identity', () => { - it('should register the field marker globally under its documented key', () => { - Symbol.keyFor(CommandFormFieldMarker)!.should.equal('cratis.commandFormField'); - }); - - it('should register the column marker globally under its documented key', () => { - Symbol.keyFor(CommandFormColumnMarker)!.should.equal('cratis.commandFormColumn'); - }); - - it('should resolve the field marker a second module instance would compute', () => { - (Symbol.for('cratis.commandFormField') === CommandFormFieldMarker).should.be.true; - }); - - it('should resolve the column marker a second module instance would compute', () => { - (Symbol.for('cratis.commandFormColumn') === CommandFormColumnMarker).should.be.true; - }); - - it('should keep the field and column markers distinct', () => { - (CommandFormFieldMarker as symbol).should.not.equal(CommandFormColumnMarker as symbol); - }); -}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_exchanging_marked_components_with_arc.ts b/Source/CommandForm/for_commandFormMarkers/when_exchanging_marked_components_with_arc.ts new file mode 100644 index 0000000..c043646 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_exchanging_marked_components_with_arc.ts @@ -0,0 +1,93 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { + CommandFormColumnDisplayName, + CommandFormFieldDisplayName, + isCommandFormColumn, + isCommandFormField, + markAsCommandFormColumn, + markAsCommandFormField, +} from '../commandFormMarkers'; + +// The cross-package contract, and the only spec that can catch the two packages drifting apart. +// +// `@cratis/arc.react` writes the field marker this package reads, and reads the column marker this +// package writes. Neither imports the other's helper — the peer range spans versions that do not +// export one — so the contract is carried entirely by the property names below being identical in +// both packages. Nothing about that is enforced by the compiler: if one side changed shape (to a +// Symbol, say, or a differently spelled flag), both packages would keep compiling, every one of +// their own specs would keep passing, and the marker would simply stop crossing the boundary. +// A field whose displayName a build transform had rewritten would then bind in a bare CommandForm +// and silently unbind inside a CommandDialog — the exact failure the marker was added to prevent, +// surviving the fix. +// +// The literals here are therefore deliberate rather than lazy: they restate what +// @cratis/arc.react's commandFormMarkers module writes, so this spec reds if either side moves. +const asArcMarksAField = (): object => { + const component = () => undefined; + Object.assign(component, { isCommandFormField: true, displayName: CommandFormFieldDisplayName }); + return component; +}; + +const asArcMarksAColumn = (): object => { + const component = () => undefined; + Object.assign(component, { isCommandFormColumn: true, displayName: CommandFormColumnDisplayName }); + return component; +}; + +const overwriteDisplayName = (component: object, name: string): void => { + (component as { displayName?: string }).displayName = name; +}; + +describe('when exchanging marked components with arc', () => { + describe('and arc marked the component', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = asArcMarksAField(); + column = asArcMarksAColumn(); + // What react-docgen-typescript does by default to every component it processes, + // which is what removes the legacy fallback and leaves only the marker. + overwriteDisplayName(field, 'RenamedByABuildTransform'); + overwriteDisplayName(column, 'RenamedByABuildTransform'); + }); + + it('should recognize the field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should recognize the column', () => { + isCommandFormColumn(column).should.be.true; + }); + }); + + describe('and this package marked the component', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = markAsCommandFormField(() => undefined); + column = markAsCommandFormColumn(() => undefined); + }); + + // Asserted as raw property access rather than through the helpers on purpose: this is the + // half arc performs, and it has to hold without any code from this package running. + it('should expose the field marker under the property name arc reads', () => { + (field as Record).isCommandFormField!.should.equal(true); + }); + + it('should expose the column marker under the property name arc reads', () => { + (column as Record).isCommandFormColumn!.should.equal(true); + }); + + it('should keep the legacy field display name for a version of arc predating the marker', () => { + (field as Record).displayName!.should.equal('CommandFormField'); + }); + + it('should keep the legacy column display name for a version of arc predating the marker', () => { + (column as Record).displayName!.should.equal('CommandFormColumn'); + }); + }); +}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts index b29a19b..e14e78f 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts @@ -3,9 +3,7 @@ import { CommandFormColumnDisplayName, - CommandFormColumnMarker, CommandFormFieldDisplayName, - CommandFormFieldMarker, markAsCommandFormColumn, markAsCommandFormField, } from '../commandFormMarkers'; @@ -26,11 +24,11 @@ describe('when marking a component', () => { }); it('should set the field marker', () => { - (field as Record)[CommandFormFieldMarker]!.should.equal(true); + (field as { isCommandFormField?: boolean }).isCommandFormField!.should.equal(true); }); it('should set the column marker', () => { - (column as Record)[CommandFormColumnMarker]!.should.equal(true); + (column as { isCommandFormColumn?: boolean }).isCommandFormColumn!.should.equal(true); }); it('should also set the legacy field display name for older arc', () => { @@ -42,7 +40,7 @@ describe('when marking a component', () => { }); it('should not cross mark a field with the column marker', () => { - ((field as Record)[CommandFormColumnMarker] === undefined).should.be.true; + ((field as { isCommandFormColumn?: boolean }).isCommandFormColumn === undefined).should.be.true; }); // The helpers mark in place and hand the component back, so both diff --git a/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts index 6c8f4d9..4f8a043 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.ts @@ -1,7 +1,7 @@ // Copyright (c) Cratis. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -import { CommandFormFieldMarker, isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; +import { isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; describe('when nothing marks the component', () => { it('should not recognize an unmarked component', () => { @@ -28,11 +28,11 @@ describe('when nothing marks the component', () => { // recognized through some other value that merely happens to be present. it('should not recognize a marker that is not true', () => { const disabled = () => undefined; - (disabled as unknown as Record)[CommandFormFieldMarker] = false; + (disabled as unknown as Record).isCommandFormField = false; isCommandFormField(disabled).should.be.false; const wrongType = () => undefined; - (wrongType as unknown as Record)[CommandFormFieldMarker] = 'yes'; + (wrongType as unknown as Record).isCommandFormField = 'yes'; isCommandFormField(wrongType).should.be.false; }); diff --git a/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts index 966ede9..5997e41 100644 --- a/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts +++ b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts @@ -1,20 +1,15 @@ // Copyright (c) Cratis. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -import { - CommandFormColumnMarker, - CommandFormFieldMarker, - isCommandFormColumn, - isCommandFormField, -} from '../commandFormMarkers'; +import { isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; // The mirror of when_only_the_legacy_display_name_is_present: a component carrying // the marker and no `displayName` whatsoever. This is the case a build transform // cannot produce by renaming, and the one that proves the marker is sufficient on // its own rather than merely corroborating the legacy label. -const stampMarkerOnly = (marker: symbol): object => { +const stampMarkerOnly = (marker: 'isCommandFormField' | 'isCommandFormColumn'): object => { const component = () => undefined; - (component as unknown as Record)[marker] = true; + (component as unknown as Record)[marker] = true; return component; }; @@ -23,8 +18,8 @@ describe('when a component carries only the marker', () => { let column: object; beforeEach(() => { - field = stampMarkerOnly(CommandFormFieldMarker); - column = stampMarkerOnly(CommandFormColumnMarker); + field = stampMarkerOnly('isCommandFormField'); + column = stampMarkerOnly('isCommandFormColumn'); }); it('should recognize the field', () => {