From d83b84503a01779aa9b9e997fa996eda61c89f91 Mon Sep 17 00:00:00 2001 From: Einar Date: Thu, 13 Aug 2026 15:46:58 +0200 Subject: [PATCH 1/7] Let a range filter render buckets that were counted elsewhere RangeHistogramFilter could only count values the browser already held, so a range picker over a data set larger than the loaded page misrepresented what each range actually selects - and there was no way to feed it a server-side aggregation. numericRange now takes an optional histogram of pre-counted buckets, rendered as given and scaled against their own tallest bar. The counting itself moves out of the component into buildHistogram so both paths are covered by specs. values stays supported and unchanged for client-side counting. Co-Authored-By: Claude Opus 5 (1M context) --- Source/Filter/FilterPanel.tsx | 1 + Source/Filter/RangeHistogramFilter.tsx | 59 +++++++------------ .../when_buckets_were_counted_elsewhere.ts | 39 ++++++++++++ .../when_counting_values_in_the_browser.ts | 42 +++++++++++++ Source/Filter/index.ts | 4 +- Source/Filter/types.ts | 20 ++++++- Source/Filter/utils.ts | 48 ++++++++++++++- 7 files changed, 170 insertions(+), 43 deletions(-) create mode 100644 Source/Filter/for_buildHistogram/when_buckets_were_counted_elsewhere.ts create mode 100644 Source/Filter/for_buildHistogram/when_counting_values_in_the_browser.ts diff --git a/Source/Filter/FilterPanel.tsx b/Source/Filter/FilterPanel.tsx index a4a5d344..8887017b 100644 --- a/Source/Filter/FilterPanel.tsx +++ b/Source/Filter/FilterPanel.tsx @@ -317,6 +317,7 @@ export function FilterPanel({ ) : isNumeric && filter.numericRange ? ( string; } -interface HistogramBucket { - start: number; - end: number; - count: number; - maxCount: number; -} - const defaultFormatValue = (value: number) => { if (Number.isInteger(value)) return value.toString(); return value.toFixed(1); @@ -32,6 +36,7 @@ const defaultFormatValue = (value: number) => { export function RangeHistogramFilter({ values, + histogram: providedHistogram, min, max, buckets = 20, @@ -44,7 +49,7 @@ export function RangeHistogramFilter({ const [dragStart, setDragStart] = useState<{ x: number; range: [number, number] } | null>(null); const numericValues = useMemo(() => { - return values + return (values ?? []) .map((v) => { if (typeof v === 'number') return v; if (v instanceof Date) return v.getTime(); @@ -54,34 +59,10 @@ export function RangeHistogramFilter({ .filter((v): v is number => v !== null); }, [values]); - const histogram = useMemo((): HistogramBucket[] => { - const range = max - min; - if (range <= 0 || numericValues.length === 0) { - return []; - } - - const bucketSize = range / buckets; - const bucketCounts: number[] = Array(buckets).fill(0); - - numericValues.forEach((value) => { - const bucketIndex = Math.min( - Math.floor((value - min) / bucketSize), - buckets - 1 - ); - if (bucketIndex >= 0 && bucketIndex < buckets) { - bucketCounts[bucketIndex]++; - } - }); - - const maxCount = Math.max(...bucketCounts, 1); - - return bucketCounts.map((count, i) => ({ - start: min + i * bucketSize, - end: min + (i + 1) * bucketSize, - count, - maxCount, - })); - }, [numericValues, min, max, buckets]); + const histogram = useMemo( + () => buildHistogram(numericValues, min, max, buckets, providedHistogram), + [providedHistogram, numericValues, min, max, buckets] + ); const currentRange = selectedRange ?? [min, max]; @@ -155,7 +136,7 @@ export function RangeHistogramFilter({ }; }, [isDragging, dragStart, min, max, onChange]); - const handleBarClick = (bucket: HistogramBucket) => { + const handleBarClick = (bucket: RenderedHistogramBucket) => { onChange([bucket.start, bucket.end]); }; diff --git a/Source/Filter/for_buildHistogram/when_buckets_were_counted_elsewhere.ts b/Source/Filter/for_buildHistogram/when_buckets_were_counted_elsewhere.ts new file mode 100644 index 00000000..c1a02752 --- /dev/null +++ b/Source/Filter/for_buildHistogram/when_buckets_were_counted_elsewhere.ts @@ -0,0 +1,39 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { buildHistogram } from '../utils'; +import type { HistogramBucket } from '../types'; + +/** + * Pre-counted buckets come from a source that can see more data than the browser holds - a server + * aggregating over a whole table, for instance. Recounting them against the loaded values would + * understate the totals and make the picker misrepresent what a range actually selects. + */ +describe('when buckets were counted elsewhere', () => { + const provided: HistogramBucket[] = [ + { start: 0, end: 5, count: 3000 }, + { start: 5, end: 10, count: 1000 }, + ]; + const result = buildHistogram([1, 2, 3], 0, 10, 20, provided); + + it('should render exactly the buckets it was given', () => { + result.should.have.lengthOf(2); + }); + + it('should keep the counts it was given rather than recounting the values', () => { + result[0].count.should.equal(3000); + result[1].count.should.equal(1000); + }); + + it('should ignore the requested bucket count', () => { + result.should.not.have.lengthOf(20); + }); + + it('should scale the bars against the tallest provided bucket', () => { + result.every((bucket) => bucket.maxCount === 3000).should.be.true; + }); + + it('should produce no buckets when given an empty set', () => { + buildHistogram([1, 2, 3], 0, 10, 20, []).should.have.lengthOf(0); + }); +}); diff --git a/Source/Filter/for_buildHistogram/when_counting_values_in_the_browser.ts b/Source/Filter/for_buildHistogram/when_counting_values_in_the_browser.ts new file mode 100644 index 00000000..6720b436 --- /dev/null +++ b/Source/Filter/for_buildHistogram/when_counting_values_in_the_browser.ts @@ -0,0 +1,42 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { buildHistogram } from '../utils'; + +describe('when counting values in the browser', () => { + const result = buildHistogram([0, 1, 5, 6, 7, 9], 0, 10, 2); + + it('should produce the requested number of buckets', () => { + result.should.have.lengthOf(2); + }); + + it('should span the range evenly', () => { + result[0].start.should.equal(0); + result[0].end.should.equal(5); + result[1].start.should.equal(5); + result[1].end.should.equal(10); + }); + + it('should count each value into its bucket', () => { + result[0].count.should.equal(2); + result[1].count.should.equal(4); + }); + + it('should report the tallest count so bars can size themselves', () => { + result.every((bucket) => bucket.maxCount === 4).should.be.true; + }); + + it('should place a value on the upper bound in the last bucket', () => { + buildHistogram([10], 0, 10, 2)[1].count.should.equal(1); + }); +}); + +describe('when there is nothing to count', () => { + it('should produce no buckets for an empty set of values', () => { + buildHistogram([], 0, 10, 5).should.have.lengthOf(0); + }); + + it('should produce no buckets when the range has no width', () => { + buildHistogram([5, 5], 5, 5, 5).should.have.lengthOf(0); + }); +}); diff --git a/Source/Filter/index.ts b/Source/Filter/index.ts index 8073dc50..86e48b44 100644 --- a/Source/Filter/index.ts +++ b/Source/Filter/index.ts @@ -9,12 +9,14 @@ export { RangeHistogramFilter } from './RangeHistogramFilter'; export type { RangeHistogramFilterProps } from './RangeHistogramFilter'; export { useFilterState } from './useFilterState'; export type { UseFilterStateResult } from './useFilterState'; -export { buildFilterValues, buildRangeValues } from './utils'; +export { buildFilterValues, buildRangeValues, buildHistogram } from './utils'; +export type { RenderedHistogramBucket } from './utils'; export type { FilterValue, FilterOption, FilterEditorProps, FilterDefinition, + HistogramBucket, FilterValues, RangeValues, CustomFilterValues, diff --git a/Source/Filter/types.ts b/Source/Filter/types.ts index 5ae43968..ba5f68fe 100644 --- a/Source/Filter/types.ts +++ b/Source/Filter/types.ts @@ -15,6 +15,16 @@ export interface FilterEditorProps { onChange: (value: unknown) => void; } +/** One pre-counted bar of a range filter's histogram. */ +export interface HistogramBucket { + /** Inclusive start of the bucket, on the same scale as the filter's range. */ + start: number; + /** Exclusive end of the bucket, on the same scale as the filter's range. */ + end: number; + /** Number of items that fall inside the bucket. */ + count: number; +} + export interface FilterDefinition { key: string; label: string; @@ -27,8 +37,14 @@ export interface FilterDefinition { multi?: boolean; /** Pre-computed options for string/date filters. */ options?: FilterOption[]; - /** Numeric range data for 'number' type filters. */ - numericRange?: { min: number; max: number; values: FilterValue[] }; + /** + * Numeric range data for 'number' and 'date' type filters. + * + * Supply `values` to have the histogram counted in the browser, or `histogram` when the counts + * were produced elsewhere - by a server aggregating over more rows than are worth transferring, + * for instance. `histogram` wins when both are present. + */ + numericRange?: { min: number; max: number; values?: FilterValue[]; histogram?: HistogramBucket[] }; /** Number of histogram buckets. Defaults to 20. */ buckets?: number; /** Show an inline search box that filters the displayed options for this group. */ diff --git a/Source/Filter/utils.ts b/Source/Filter/utils.ts index d1014682..f050e860 100644 --- a/Source/Filter/utils.ts +++ b/Source/Filter/utils.ts @@ -1,7 +1,12 @@ // Copyright (c) Cratis. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -import type { FilterDefinition, FilterValues, RangeValues } from './types'; +import type { FilterDefinition, FilterValues, HistogramBucket, RangeValues } from './types'; + +/** A histogram bucket with the tallest count in its set, so a bar can size itself. */ +export interface RenderedHistogramBucket extends HistogramBucket { + maxCount: number; +} /** Initialise the string/option selection map for all string/date filters. */ export function buildFilterValues(filters: FilterDefinition[] | undefined): FilterValues { @@ -24,3 +29,44 @@ export function buildRangeValues(filters: FilterDefinition[] | undefined): Range }); return state; } + +/** + * Build the bars a range filter renders. + * + * Pre-counted buckets are rendered as given - they come from a source that saw more data than the + * browser holds, so recounting them against the loaded values would understate the real totals. + * Otherwise the raw values are counted into `bucketCount` evenly sized buckets across the range. + */ +export function buildHistogram( + values: number[], + min: number, + max: number, + bucketCount: number, + provided?: HistogramBucket[] +): RenderedHistogramBucket[] { + if (provided !== undefined) { + if (provided.length === 0) return []; + const providedMax = Math.max(...provided.map((bucket) => bucket.count), 1); + return provided.map((bucket) => ({ ...bucket, maxCount: providedMax })); + } + + const range = max - min; + if (range <= 0 || values.length === 0) return []; + + const bucketSize = range / bucketCount; + const counts: number[] = Array(bucketCount).fill(0); + + values.forEach((value) => { + const index = Math.min(Math.floor((value - min) / bucketSize), bucketCount - 1); + if (index >= 0 && index < bucketCount) counts[index]++; + }); + + const maxCount = Math.max(...counts, 1); + + return counts.map((count, index) => ({ + start: min + index * bucketSize, + end: min + (index + 1) * bucketSize, + count, + maxCount, + })); +} From a1a475af734292302789ac2a2d629a3f68169d3e Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 03:09:28 +0200 Subject: [PATCH 2/7] 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 | 8 + 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, 682 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 4af5ed1d..fdc125f3 100644 --- a/Documentation/CommandForm/index.md +++ b/Documentation/CommandForm/index.md @@ -71,3 +71,11 @@ Two field props refine this per field - both work on every field type in this pa - `initialValue` - override how the field's value is derived from the query result, either a property accessor matched by name or a function composing a value from the whole result. See Arc's [Populating a Form from a Query](https://github.com/Cratis/Arc/blob/main/Documentation/frontend/react/command-form/data-loading.md) for the full behavior, including how the populated data becomes the form's change-tracking baseline. + +## How a child is recognized as a field + +`CommandForm`, `CommandDialog`, and `CommandStepper` inspect each child's component type. A field carries `isCommandFormField: true`; a column carries `isCommandFormColumn: true`. The legacy `displayName` values remain permanent fallbacks so independently versioned Components and Arc packages interoperate in either upgrade order. + +Use `asCommandFormField` from `@cratis/arc.react/commands` for custom fields; it stamps the marker automatically. For a hand-rolled field or column, use `markAsCommandFormField` or `markAsCommandFormColumn` from `@cratis/components/CommandForm`. + +The marker is a plain shared property rather than a package-private symbol, so a component marked by Arc is recognized by Components and vice versa. Build transforms may rewrite `displayName` without silently breaking binding. diff --git a/Source/CommandDialog/CommandDialog.tsx b/Source/CommandDialog/CommandDialog.tsx index fa1d6db2..2489b06b 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` @@ -154,7 +155,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 ; } @@ -390,7 +391,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 8856b7f2..be5134da 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 { getStepPanels } from './stepChildren'; @@ -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 00000000..81b3de8b --- /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 00000000..5a54c7c3 --- /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 00000000..fbcbe554 --- /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 00000000..1fa83d9c --- /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 00000000..a87ba6c9 --- /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 00000000..df3fb276 --- /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 00000000..768ff437 --- /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 00000000..edf8cb8a --- /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 00000000..f44f70a5 --- /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 45eebaea..e79348a0 100644 --- a/Source/CommandForm/index.ts +++ b/Source/CommandForm/index.ts @@ -1,6 +1,7 @@ // 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'; export * from './FieldTypeProvider'; export * from './fieldTypeProviderRegistry'; From 2a7c21146ed4ce32f1e2a060cc521eef83b9a803 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 07:58:06 +0200 Subject: [PATCH 3/7] 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 00000000..5e2eee77 --- /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 a87ba6c9..2e3d3eae 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 df3fb276..918ece99 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 00000000..af787e84 --- /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 edf8cb8a..945bda98 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 00000000..81fe358e --- /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 52970231b34e299b7225f1a4422d0b7c26f9c75d Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 07:58:21 +0200 Subject: [PATCH 4/7] 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 fe36d64e..da2ce22e 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 4547c530..01158525 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 00000000..bc381aa1 --- /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 f94bdbbd..4a87a7ca 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 4c550d20..6e2603c5 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 ea78f18946601b32b14118889e9be9fcd5ed3503 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 08:24:12 +0200 Subject: [PATCH 5/7] 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 --- 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 +++++----- 14 files changed, 60 insertions(+), 60 deletions(-) diff --git a/ESLint/README.md b/ESLint/README.md index da2ce22e..e05cb1ac 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 bc381aa1..abf64179 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 5e2eee77..88b4b278 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 81b3de8b..b050b30a 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 5a54c7c3..212d2cf3 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 fbcbe554..405959e9 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 1fa83d9c..00531df0 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 2e3d3eae..c49114f2 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 918ece99..83eefeb6 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 768ff437..fee7c02e 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 af787e84..b29a19b1 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 945bda98..6c8f4d9b 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 f44f70a5..aab86618 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 81fe358e..966ede9b 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 84c44432a914d21a1e56b3a18847090cf3b8bc8f Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 4 Aug 2026 10:34:10 +0200 Subject: [PATCH 6/7] 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 --- 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 +- 9 files changed, 180 insertions(+), 142 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/ESLint/README.md b/ESLint/README.md index e05cb1ac..53987ccd 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 abf64179..89db0ea7 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 405959e9..5713e454 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 c49114f2..5bfa7965 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 83eefeb6..00000000 --- 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 00000000..c043646b --- /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 b29a19b1..e14e78fd 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 6c8f4d9b..4f8a043b 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 966ede9b..5997e410 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', () => { From 13b576e86c6fb6558ff9c2409ddbc3c8437d6000 Mon Sep 17 00:00:00 2001 From: woksin Date: Sun, 23 Aug 2026 15:54:54 +0200 Subject: [PATCH 7/7] Clean the Components Storybook gate --- Source/.storybook/main.ts | 17 +++++++++-------- Source/vite.config.ts | 7 +------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/Source/.storybook/main.ts b/Source/.storybook/main.ts index dee55628..f9b993e2 100644 --- a/Source/.storybook/main.ts +++ b/Source/.storybook/main.ts @@ -5,12 +5,7 @@ import type { StorybookConfig } from '@storybook/react-vite'; import type { UserConfig as ViteConfig } from 'vite'; const config: StorybookConfig = { - stories: [ - '../**/*.stories.@(ts|tsx)', - '!../node_modules/**', - '!../dist/**', - '!../storybook-static/**' - ], + stories: ['../!(dist|node_modules|storybook-static)/**/*.stories.@(ts|tsx)'], addons: [], framework: { name: '@storybook/react-vite', @@ -21,8 +16,14 @@ const config: StorybookConfig = { // Ensure Vite dev server does not open the browser when Storybook starts async viteFinal(existingConfig: ViteConfig) { const cfg: ViteConfig = { ...existingConfig }; - cfg.server = { ...(cfg.server || {}), open: false } as unknown; - cfg.build = { ...(cfg.build || {}), cssMinify: false }; + cfg.server = { ...(cfg.server || {}), open: false }; + cfg.build = { + ...(cfg.build || {}), + cssMinify: false, + // The preview index includes Storybook's manager runtime and every story entry; + // it is documentation output rather than a consumer package chunk. + chunkSizeWarningLimit: 1800 + }; return cfg; } }; diff --git a/Source/vite.config.ts b/Source/vite.config.ts index ee5b131a..c4d32afc 100644 --- a/Source/vite.config.ts +++ b/Source/vite.config.ts @@ -11,11 +11,6 @@ export default defineConfig({ optimizeDeps: { exclude: ['tslib'], }, - esbuild: { - supported: { - 'top-level-await': true, - }, - }, build: { outDir: './wwwroot', assetsDir: '', @@ -72,7 +67,7 @@ export default defineConfig({ '**/for_*/**/when_*.ts', '**/for_*/**/when_*.tsx', ], - setupFiles: `${__dirname}/vitest.setup.ts`, + setupFiles: `${import.meta.dirname}/vitest.setup.ts`, }, plugins: [ react(),