diff --git a/Documentation/CommandForm/index.md b/Documentation/CommandForm/index.md index b1de9d0..52af1ab 100644 --- a/Documentation/CommandForm/index.md +++ b/Documentation/CommandForm/index.md @@ -48,3 +48,62 @@ import { InputTextField, NumberField, CheckboxField } from '@cratis/components/C value={c => c.active} label="Active" /> ``` + +## How a child is recognized as a field + +`CommandForm`, `CommandDialog` and `CommandStepper` decide which of their children are +fields by inspecting the child's component type. A component is treated as a field when +it carries either of the following: + +- an `isCommandFormField` property set to `true` — what `asCommandFormField` and + `markAsCommandFormField` stamp; or +- the legacy `displayName` of `'CommandFormField'`, which is checked as a fallback and + is supported indefinitely. + +Columns work the same way, through `isCommandFormColumn` and `'CommandFormColumn'`. + +> [!IMPORTANT] +> **`displayName` is load-bearing on field and column components — never overwrite it.** +> A child whose `displayName` has been replaced and that carries no marker is not +> recognized as a field: it renders without its container, so it gets no label, no bound +> value and no change handler. This fails silently — there is no error and no warning. + +The marker exists so that this stops being fatal. A build-time transform rewrites +`displayName` and nothing else, so it leaves the marker alone and a field keeps working +even after being renamed. `@cratis/arc.react` marks and reads the same two properties, so a +component marked by either package is recognized by both. + +### Build tooling that rewrites `displayName` + +The most common way to hit this is Storybook's +`reactDocgen: 'react-docgen-typescript'` setting, whose underlying plugin defaults +`setDisplayName` to `true` and appends `.displayName = ""` to +every module it processes. If you wrap fields of your own with `asCommandFormField`, +turn that off: + +```ts +// .storybook/main.ts +typescript: { + reactDocgen: 'react-docgen-typescript', + reactDocgenTypescriptOptions: { + setDisplayName: false + } +} +``` + +### Writing a custom field + +Prefer `asCommandFormField` from `@cratis/arc.react`, which marks the component for you. +To mark a component directly — when hand-rolling a field or a column — use the helpers: + +```tsx +import { markAsCommandFormField } from '@cratis/components/CommandForm'; + +const MyField = (props: { value?: (c: MyCommand) => unknown }) => { /* ... */ }; + +markAsCommandFormField(MyField); +``` + +Both helpers set the marker *and* the legacy `displayName`, so a component marked this +way is recognized by every version of `@cratis/arc.react` within this package's +supported range. diff --git a/ESLint/README.md b/ESLint/README.md index fe36d64..53987cc 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,38 @@ 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 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. + +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 +MyField.displayName = 'CommandFormField'; +if (component.displayName === 'CommandFormField') { wrap(component); } + +// ✅ marker first, legacy displayName still set and still honoured +import { markAsCommandFormField, isCommandFormField } from '@cratis/components/CommandForm'; + +markAsCommandFormField(MyField); +if (isCommandFormField(component)) { wrap(component); } +``` + +Flagged in both directions: assignment (`C.displayName = '…'`, including computed and +object-literal forms) and comparison (`===`, `!==`, either operand order). Referring to the +exported `CommandFormFieldDisplayName` / `CommandFormColumnDisplayName` constants is not +flagged, so the declarations themselves and any deliberate legacy-path code stay clean. +Prefer `asCommandFormField` from `@cratis/arc.react` where it applies — it marks for you. diff --git a/ESLint/index.js b/ESLint/index.js index 4547c53..0115852 100644 --- a/ESLint/index.js +++ b/ESLint/index.js @@ -3,6 +3,7 @@ import { noPrimereactDialog } from './lib/noPrimereactDialog.js'; import { noRootBarrelImport } from './lib/noRootBarrelImport.js'; import { onbeforeexecuteMustReturn } from './lib/onbeforeexecuteMustReturn.js'; import { noHooksInViewModel } from './lib/noHooksInViewModel.js'; +import { noRawCommandFormMarker } from './lib/noRawCommandFormMarker.js'; const { version } = createRequire(import.meta.url)('./package.json'); @@ -17,6 +18,7 @@ const plugin = { 'no-root-barrel-import': noRootBarrelImport, 'onbeforeexecute-must-return': onbeforeexecuteMustReturn, 'no-hooks-in-view-model': noHooksInViewModel, + 'no-raw-command-form-marker': noRawCommandFormMarker, }, configs: {}, }; @@ -37,6 +39,7 @@ Object.assign(plugin.configs, { '@cratis/components/no-root-barrel-import': 'error', '@cratis/components/onbeforeexecute-must-return': 'error', '@cratis/components/no-hooks-in-view-model': 'error', + '@cratis/components/no-raw-command-form-marker': 'error', }, }, ], @@ -44,4 +47,4 @@ Object.assign(plugin.configs, { export default plugin; export const { configs, rules, meta } = plugin; -export { noPrimereactDialog, noRootBarrelImport, onbeforeexecuteMustReturn, noHooksInViewModel }; +export { noPrimereactDialog, noRootBarrelImport, onbeforeexecuteMustReturn, noHooksInViewModel, noRawCommandFormMarker }; diff --git a/ESLint/lib/noRawCommandFormMarker.js b/ESLint/lib/noRawCommandFormMarker.js new file mode 100644 index 0000000..89db0ea --- /dev/null +++ b/ESLint/lib/noRawCommandFormMarker.js @@ -0,0 +1,89 @@ +// 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' }, +}; + +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 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. +// +// 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', + docs: { + description: 'Disallow identifying CommandForm fields and columns by a raw displayName string; use the marker helpers.', + recommended: true, + url: 'https://github.com/Cratis/Components/blob/main/ESLint/README.md', + }, + schema: [], + messages: { + useMarkHelper: + "Do not stamp displayName = '{{name}}' by hand — a build transform that rewrites displayName silently unbinds this component. Use {{helper}}() from '@cratis/components/CommandForm'.", + usePredicate: + "Do not identify a CommandForm child by comparing displayName to '{{name}}' — a component whose displayName was rewritten is missed. Use {{helper}}() from '@cratis/components/CommandForm'.", + }, + }, + create(context) { + const report = (node, messageId, helper, name) => + context.report({ node, messageId, data: { helper, name } }); + + return { + // C.displayName = 'CommandFormField' + AssignmentExpression(node) { + if (node.operator !== '=' || !isDisplayNameMember(node.left)) return; + const marker = markerFor(node.right); + if (marker) report(node, 'useMarkHelper', marker.mark, node.right.value); + }, + + // { displayName: 'CommandFormField' } — e.g. via Object.assign + Property(node) { + const key = node.computed ? undefined : node.key; + const named = + (key?.type === 'Identifier' && key.name === 'displayName') || + (key?.type === 'Literal' && key.value === 'displayName'); + if (!named) return; + const marker = markerFor(node.value); + if (marker) report(node, 'useMarkHelper', marker.mark, node.value.value); + }, + + // child.displayName === 'CommandFormField' (either operand order) + BinaryExpression(node) { + if (!EQUALITY.has(node.operator)) return; + const [member, literal] = isDisplayNameMember(node.left) + ? [node.left, node.right] + : [node.right, node.left]; + if (!isDisplayNameMember(member)) return; + const marker = markerFor(literal); + if (marker) report(node, 'usePredicate', marker.predicate, literal.value); + }, + }; + }, +}; + +export default noRawCommandFormMarker; diff --git a/ESLint/package.json b/ESLint/package.json index f94bdbb..4a87a7c 100644 --- a/ESLint/package.json +++ b/ESLint/package.json @@ -1,7 +1,7 @@ { "name": "@cratis/eslint-plugin-components", "version": "0.0.0", - "description": "Cratis Components ESLint rules: import from subpaths not the root barrel, use the Cratis dialog wrappers instead of primereact/dialog, require onBeforeExecute callbacks to return their values, and keep React hooks out of view models. Compose on top of @cratis/eslint-config.", + "description": "Cratis Components ESLint rules: import from subpaths not the root barrel, use the Cratis dialog wrappers instead of primereact/dialog, require onBeforeExecute callbacks to return their values, keep React hooks out of view models, and identify CommandForm fields by a tamper-resistant marker rather than a raw displayName string. Compose on top of @cratis/eslint-config.", "author": "Cratis", "license": "MIT", "type": "module", diff --git a/ESLint/test/rules.test.js b/ESLint/test/rules.test.js index 4c550d2..6e2603c 100644 --- a/ESLint/test/rules.test.js +++ b/ESLint/test/rules.test.js @@ -5,6 +5,7 @@ import { noPrimereactDialog } from '../lib/noPrimereactDialog.js'; import { noRootBarrelImport } from '../lib/noRootBarrelImport.js'; import { onbeforeexecuteMustReturn } from '../lib/onbeforeexecuteMustReturn.js'; import { noHooksInViewModel } from '../lib/noHooksInViewModel.js'; +import { noRawCommandFormMarker } from '../lib/noRawCommandFormMarker.js'; RuleTester.afterAll = afterAll; RuleTester.describe = describe; @@ -142,3 +143,66 @@ tsRuleTester.run('no-hooks-in-view-model', noHooksInViewModel, { }, ], }); + +tsRuleTester.run('no-raw-command-form-marker', noRawCommandFormMarker, { + valid: [ + // The sanctioned way to mark and to test, in both directions. + "markAsCommandFormField(MyField);", + "markAsCommandFormColumn(MyColumn);", + "if (isCommandFormField(component)) { wrap(component); }", + "if (isCommandFormColumn(component)) { layout(component); }", + // Referring to the exported constant rather than repeating the literal. + "MyField.displayName = CommandFormFieldDisplayName;", + "if (component.displayName === CommandFormFieldDisplayName) { wrap(component); }", + // Declaring the constants themselves — the one place the literal belongs. + "export const CommandFormFieldDisplayName = 'CommandFormField';", + "export const CommandFormColumnDisplayName = 'CommandFormColumn';", + // displayName used as the diagnostic label it is meant to be. + "MyDialog.displayName = 'MyDialog';", + "StepperPanel.displayName = 'StepperPanel';", + "const label = `DialogWrapper(${Component.displayName})`;", + // A different property that happens to hold the same string. + "const meta = { kind: 'CommandFormField' };", + // Comparing a non-displayName property. + "if (component.name === 'CommandFormField') { legacy(component); }", + ], + invalid: [ + { + code: "MyField.displayName = 'CommandFormField';", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormField', name: 'CommandFormField' } }], + }, + { + code: "MyColumn.displayName = 'CommandFormColumn';", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormColumn', name: 'CommandFormColumn' } }], + }, + { + // Computed member access is the same stamp. + code: "MyField['displayName'] = 'CommandFormField';", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormField', name: 'CommandFormField' } }], + }, + { + // Set through an object literal, e.g. Object.assign. + code: "Object.assign(MyField, { displayName: 'CommandFormField' });", + errors: [{ messageId: 'useMarkHelper', data: { helper: 'markAsCommandFormField', name: 'CommandFormField' } }], + }, + { + code: "if (component.displayName === 'CommandFormField') { wrap(component); }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormField', name: 'CommandFormField' } }], + }, + { + // Reversed operand order. + code: "if ('CommandFormColumn' === component.displayName) { layout(component); }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormColumn', name: 'CommandFormColumn' } }], + }, + { + // Negated comparison misses a renamed field just as badly. + code: "if (component.displayName !== 'CommandFormField') { return child; }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormField', name: 'CommandFormField' } }], + }, + { + // The cast form this package used before the marker existed. + code: "if ((component as { displayName?: string }).displayName === 'CommandFormField') { wrap(component); }", + errors: [{ messageId: 'usePredicate', data: { helper: 'isCommandFormField', name: 'CommandFormField' } }], + }, + ], +}); diff --git a/Source/CommandDialog/CommandDialog.tsx b/Source/CommandDialog/CommandDialog.tsx index 3fe4d38..895a7b2 100644 --- a/Source/CommandDialog/CommandDialog.tsx +++ b/Source/CommandDialog/CommandDialog.tsx @@ -13,6 +13,7 @@ import { type CommandFormProps } from '@cratis/arc.react/commands'; import { applyBeforeExecute, type BeforeExecuteCallback } from './applyBeforeExecute'; +import { isCommandFormField, markAsCommandFormColumn } from '../CommandForm/commandFormMarkers'; /** * Props for {@link CommandDialog}. Combines the props of a `CommandForm` @@ -148,7 +149,7 @@ const CommandDialogWrapper = ({ if (!React.isValidElement(child)) return child; const component = child.type as React.ComponentType; - if (component.displayName === 'CommandFormField') { + if (isCommandFormField(component)) { type FieldElement = Parameters[0]['field']; return ; } @@ -349,7 +350,7 @@ const CommandDialogComponent = ( {children} ); -CommandDialogColumnWrapper.displayName = 'CommandFormColumn'; +markAsCommandFormColumn(CommandDialogColumnWrapper); CommandDialogComponent.Column = CommandDialogColumnWrapper; diff --git a/Source/CommandDialog/CommandStepper.tsx b/Source/CommandDialog/CommandStepper.tsx index 5cb880f..8550937 100644 --- a/Source/CommandDialog/CommandStepper.tsx +++ b/Source/CommandDialog/CommandStepper.tsx @@ -14,6 +14,7 @@ import { type CommandFormProps } from '@cratis/arc.react/commands'; import { applyBeforeExecute, type BeforeExecuteCallback } from './applyBeforeExecute'; +import { isCommandFormField } from '../CommandForm/commandFormMarkers'; import type { StepperPanelProps } from './StepperPanel'; import './CommandStepper.css'; @@ -145,7 +146,7 @@ const extractFieldNamesFromNode = (nodes: React.ReactNode): string[] => { React.Children.forEach(nodes, (child) => { if (!React.isValidElement(child)) return; const component = child.type as React.ComponentType; - if ((component as { displayName?: string }).displayName === 'CommandFormField') { + if (isCommandFormField(component)) { const fieldProps = child.props as { value?: (obj: unknown) => unknown }; const name = getPropertyName(fieldProps.value); if (name) names.push(name); @@ -164,7 +165,7 @@ const processChildren = (nodes: React.ReactNode): React.ReactNode => { if (!React.isValidElement(child)) return child; const component = child.type as React.ComponentType; - if ((component as { displayName?: string }).displayName === 'CommandFormField') { + if (isCommandFormField(component)) { type FieldElement = Parameters[0]['field']; return ; } diff --git a/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts b/Source/CommandDialog/for_CommandDialog/when_a_renamed_field_is_nested_in_a_column.ts new file mode 100644 index 0000000..88b4b27 --- /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 recognize 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/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts b/Source/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts index 43229a0..91c9b9a 100644 --- a/Source/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts +++ b/Source/CommandDialog/for_CommandDialog/when_confirming_with_close_dialog_and_command_result.ts @@ -5,7 +5,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; import { DialogResult, useDialogContext } from '@cratis/arc.react/dialogs'; -import { CommandDialog } from '../CommandDialog'; const { closeDialog, commandResult } = vi.hoisted(() => ({ closeDialog: vi.fn(), @@ -64,20 +63,29 @@ class TestCommand { name: string = ''; } -const TestDialog = () => { - const { closeDialog: closeWithResult } = useDialogContext(); - - return React.createElement(CommandDialog, { - command: TestCommand, - title: 'Update user', - onConfirm: async () => closeWithResult(DialogResult.Ok, commandResult), - onCancel: async () => closeWithResult(DialogResult.Cancelled), - }); -}; - describe('when confirming with close dialog and command result', () => { - beforeEach(() => { + beforeEach(async () => { closeDialog.mockReset(); + + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate CommandDialog + // under this file's own mocks so the confirm button is always the one that + // fires onClick — a static import here makes this spec pass or fail by luck. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + + const TestDialog = () => { + const { closeDialog: closeWithResult } = useDialogContext(); + + return React.createElement(CommandDialog, { + command: TestCommand, + title: 'Update user', + onConfirm: async () => closeWithResult(DialogResult.Ok, commandResult), + onCancel: async () => closeWithResult(DialogResult.Cancelled), + }); + }; + renderToStaticMarkup(React.createElement(TestDialog)); }); diff --git a/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts b/Source/CommandDialog/for_CommandDialog/when_field_carries_only_the_legacy_display_name.ts new file mode 100644 index 0000000..b050b30 --- /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 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), +})); + + +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 recognize the child as a field and wrap it', () => { + html.should.include('field-wrapper'); + }); + + it('should render the field itself', () => { + html.should.include('the-field'); + }); +}); diff --git a/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts b/Source/CommandDialog/for_CommandDialog/when_field_display_name_is_overwritten.ts new file mode 100644 index 0000000..212d2cf --- /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 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), +})); + + +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 recognize 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_given_initial_valid_values.ts b/Source/CommandDialog/for_CommandDialog/when_given_initial_valid_values.ts index 59b894f..539ddfc 100644 --- a/Source/CommandDialog/for_CommandDialog/when_given_initial_valid_values.ts +++ b/Source/CommandDialog/for_CommandDialog/when_given_initial_valid_values.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { CommandDialog } from '../CommandDialog'; vi.mock('primereact/dialog', () => { // PrimeReact 11's Dialog is compositional; each part is a pass-through that @@ -50,7 +49,15 @@ class TestCommand { describe('when CommandDialog is given initial valid values', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + const element = React.createElement(CommandDialog, { command: TestCommand as unknown as new () => object, initialValues: { name: 'John Doe' } as Partial, diff --git a/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts new file mode 100644 index 0000000..5713e45 --- /dev/null +++ b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts @@ -0,0 +1,71 @@ +// 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, 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 { isCommandFormColumn?: boolean }).isCommandFormColumn!.should.equal(true); + }); + + 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', () => { + (Column as { displayName?: string }).displayName!.should.equal(CommandFormColumnDisplayName); + }); +}); diff --git a/Source/CommandDialog/for_CommandDialog/when_not_executing.ts b/Source/CommandDialog/for_CommandDialog/when_not_executing.ts index 3850022..0b230e5 100644 --- a/Source/CommandDialog/for_CommandDialog/when_not_executing.ts +++ b/Source/CommandDialog/for_CommandDialog/when_not_executing.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { CommandDialog } from '../CommandDialog'; vi.mock('primereact/dialog', () => { // PrimeReact 11's Dialog is compositional; each part is a pass-through that @@ -51,7 +50,15 @@ class TestCommand { describe('when CommandDialog is in its initial state', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { CommandDialog } = await import('../CommandDialog'); + const element = React.createElement(CommandDialog, { command: TestCommand as unknown as new () => object, visible: true, diff --git a/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts b/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts index 07d656c..a65cc2e 100644 --- a/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts +++ b/Source/CommandDialog/for_StepperCommandDialog/when_not_executing.ts @@ -4,8 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { StepperCommandDialog } from '../StepperCommandDialog'; -import { StepperPanel } from '../StepperPanel'; // StepperCommandDialog now composes the Cratis Dialog wrapper (not primereact/dialog // directly) — render its custom footer (`buttons`) and body. @@ -66,7 +64,16 @@ class TestCommand { describe('when StepperCommandDialog is in its initial state', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { StepperCommandDialog } = await import('../StepperCommandDialog'); + const { StepperPanel } = await import('../StepperPanel'); + const element = React.createElement( StepperCommandDialog, { diff --git a/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts b/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts index c72789d..536378c 100644 --- a/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts +++ b/Source/CommandDialog/for_StepperCommandDialog/when_single_step.ts @@ -4,8 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { StepperCommandDialog } from '../StepperCommandDialog'; -import { StepperPanel } from '../StepperPanel'; vi.mock('../../Dialogs/Dialog', () => ({ Dialog: (props: { buttons?: React.ReactNode; children?: React.ReactNode }) => @@ -61,7 +59,16 @@ class TestCommand { describe('when StepperCommandDialog has a single step', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { StepperCommandDialog } = await import('../StepperCommandDialog'); + const { StepperPanel } = await import('../StepperPanel'); + const element = React.createElement( StepperCommandDialog, { diff --git a/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts b/Source/CommandDialog/for_StepperCommandDialog/when_stepper_field_display_name_is_overwritten.ts new file mode 100644 index 0000000..00531df --- /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 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), +})); + +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 recognize 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 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', () => { + const step2Number = html.match(/]*>2<\/span>|
]*>2<\/div>/); + (step2Number?.[0] ?? '').should.not.include('red'); + }); + + // Guards every assertion above: were the overwrite to silently fail, they + // would pass through the legacy fallback and prove nothing about the marker. + it('should have actually lost the legacy display name', () => { + (RenamedField as { displayName?: string }).displayName! + .should.not.equal(CommandFormFieldDisplayName); + }); +}); diff --git a/Source/CommandForm/commandFormMarkers.ts b/Source/CommandForm/commandFormMarkers.ts new file mode 100644 index 0000000..5bfa796 --- /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 `displayName` a command form field carries. + * + * Exported so a consumer recognizing a field has a constant to compare against rather than a + * duplicated string literal. + */ +export const CommandFormFieldDisplayName = 'CommandFormField'; + +/** The `displayName` a command form column carries. */ +export const CommandFormColumnDisplayName = 'CommandFormColumn'; + +/** + * The shape a component carries to say what it is to a `CommandForm`. + * + * 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 type CommandFormMarked = { + /** Set on a field component. */ + isCommandFormField?: boolean; + + /** Set on a column component. */ + isCommandFormColumn?: boolean; + + /** The React display name, kept as the compatibility fallback. */ + displayName?: string; +}; + +/** + * 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. + * + * 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 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 function markAsCommandFormField(component: T): T & CommandFormMarked { + const marked = component as T & CommandFormMarked; + marked.isCommandFormField = true; + marked.displayName = CommandFormFieldDisplayName; + return marked; +} + +/** + * Marks a component as a command form column, and returns it. Any existing `displayName` is + * replaced. See {@link markAsCommandFormField}. + */ +export function markAsCommandFormColumn(component: T): T & CommandFormMarked { + const marked = component as T & CommandFormMarked; + marked.isCommandFormColumn = true; + marked.displayName = CommandFormColumnDisplayName; + return marked; +} + +/** + * Whether a component is a command form 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. + * + * @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 function isCommandFormField(component: unknown): boolean { + const candidate = component as CommandFormMarked | undefined; + return candidate?.isCommandFormField === true || candidate?.displayName === CommandFormFieldDisplayName; +} + +/** + * Whether a component is a command form column. + * See {@link isCommandFormField} for the ordering and why it matters. + */ +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_display_name_is_overwritten.ts b/Source/CommandForm/for_commandFormMarkers/when_display_name_is_overwritten.ts new file mode 100644 index 0000000..fee7c02 --- /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 recognize the field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should still recognize 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_exchanging_marked_components_with_arc.ts b/Source/CommandForm/for_commandFormMarkers/when_exchanging_marked_components_with_arc.ts new file mode 100644 index 0000000..c043646 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_exchanging_marked_components_with_arc.ts @@ -0,0 +1,93 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { + CommandFormColumnDisplayName, + CommandFormFieldDisplayName, + isCommandFormColumn, + isCommandFormField, + markAsCommandFormColumn, + markAsCommandFormField, +} from '../commandFormMarkers'; + +// The cross-package contract, and the only spec that can catch the two packages drifting apart. +// +// `@cratis/arc.react` writes the field marker this package reads, and reads the column marker this +// package writes. Neither imports the other's helper — the peer range spans versions that do not +// export one — so the contract is carried entirely by the property names below being identical in +// both packages. Nothing about that is enforced by the compiler: if one side changed shape (to a +// Symbol, say, or a differently spelled flag), both packages would keep compiling, every one of +// their own specs would keep passing, and the marker would simply stop crossing the boundary. +// A field whose displayName a build transform had rewritten would then bind in a bare CommandForm +// and silently unbind inside a CommandDialog — the exact failure the marker was added to prevent, +// surviving the fix. +// +// The literals here are therefore deliberate rather than lazy: they restate what +// @cratis/arc.react's commandFormMarkers module writes, so this spec reds if either side moves. +const asArcMarksAField = (): object => { + const component = () => undefined; + Object.assign(component, { isCommandFormField: true, displayName: CommandFormFieldDisplayName }); + return component; +}; + +const asArcMarksAColumn = (): object => { + const component = () => undefined; + Object.assign(component, { isCommandFormColumn: true, displayName: CommandFormColumnDisplayName }); + return component; +}; + +const overwriteDisplayName = (component: object, name: string): void => { + (component as { displayName?: string }).displayName = name; +}; + +describe('when exchanging marked components with arc', () => { + describe('and arc marked the component', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = asArcMarksAField(); + column = asArcMarksAColumn(); + // What react-docgen-typescript does by default to every component it processes, + // which is what removes the legacy fallback and leaves only the marker. + overwriteDisplayName(field, 'RenamedByABuildTransform'); + overwriteDisplayName(column, 'RenamedByABuildTransform'); + }); + + it('should recognize the field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should recognize the column', () => { + isCommandFormColumn(column).should.be.true; + }); + }); + + describe('and this package marked the component', () => { + let field: object; + let column: object; + + beforeEach(() => { + field = markAsCommandFormField(() => undefined); + column = markAsCommandFormColumn(() => undefined); + }); + + // Asserted as raw property access rather than through the helpers on purpose: this is the + // half arc performs, and it has to hold without any code from this package running. + it('should expose the field marker under the property name arc reads', () => { + (field as Record).isCommandFormField!.should.equal(true); + }); + + it('should expose the column marker under the property name arc reads', () => { + (column as Record).isCommandFormColumn!.should.equal(true); + }); + + it('should keep the legacy field display name for a version of arc predating the marker', () => { + (field as Record).displayName!.should.equal('CommandFormField'); + }); + + it('should keep the legacy column display name for a version of arc predating the marker', () => { + (column as Record).displayName!.should.equal('CommandFormColumn'); + }); + }); +}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts new file mode 100644 index 0000000..e14e78f --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_marking_a_component.ts @@ -0,0 +1,53 @@ +// 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, + 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 { isCommandFormField?: boolean }).isCommandFormField!.should.equal(true); + }); + + it('should set the column marker', () => { + (column as { isCommandFormColumn?: boolean }).isCommandFormColumn!.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 { isCommandFormColumn?: boolean }).isCommandFormColumn === 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 new file mode 100644 index 0000000..4f8a043 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_nothing_marks_the_component.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 { isCommandFormColumn, isCommandFormField } from '../commandFormMarkers'; + +describe('when nothing marks the component', () => { + it('should not recognize an unmarked component', () => { + isCommandFormField(() => undefined).should.be.false; + }); + + it('should not recognize 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 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 + // 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).isCommandFormField = false; + isCommandFormField(disabled).should.be.false; + + const wrongType = () => undefined; + (wrongType as unknown as Record).isCommandFormField = 'yes'; + isCommandFormField(wrongType).should.be.false; + }); + + it('should not recognize nullish values', () => { + isCommandFormField(undefined).should.be.false; + isCommandFormField(null).should.be.false; + isCommandFormColumn(undefined).should.be.false; + isCommandFormColumn(null).should.be.false; + }); +}); diff --git a/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts b/Source/CommandForm/for_commandFormMarkers/when_only_the_legacy_display_name_is_present.ts new file mode 100644 index 0000000..aab8661 --- /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 recognize the field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should recognize 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/for_commandFormMarkers/when_only_the_marker_is_present.ts b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts new file mode 100644 index 0000000..5997e41 --- /dev/null +++ b/Source/CommandForm/for_commandFormMarkers/when_only_the_marker_is_present.ts @@ -0,0 +1,47 @@ +// 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'; + +// 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: 'isCommandFormField' | 'isCommandFormColumn'): 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('isCommandFormField'); + column = stampMarkerOnly('isCommandFormColumn'); + }); + + it('should recognize the field', () => { + isCommandFormField(field).should.be.true; + }); + + it('should recognize 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; + }); +}); diff --git a/Source/CommandForm/index.ts b/Source/CommandForm/index.ts index 76a65e1..c37851f 100644 --- a/Source/CommandForm/index.ts +++ b/Source/CommandForm/index.ts @@ -1,4 +1,5 @@ // Copyright (c) Cratis. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +export * from './commandFormMarkers'; export * from './fields'; diff --git a/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts b/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts index 6e6eba2..97ee419 100644 --- a/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts +++ b/Source/Dialogs/for_Dialog/when_confirming_with_close_dialog_and_result.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { Dialog } from '../Dialog'; const { closeDialog } = vi.hoisted(() => ({ closeDialog: vi.fn(), @@ -42,9 +41,17 @@ vi.mock('@cratis/arc.react/dialogs', () => ({ describe('when confirming with close dialog and result', () => { const resultPayload = { id: 'project-1', name: 'Project 1' }; - beforeEach(() => { + beforeEach(async () => { closeDialog.mockReset(); + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate Dialog under + // this file's own mocks so the confirm button is always the one that fires + // onClick — a static import here makes this spec pass or fail by luck. + vi.resetModules(); + const { Dialog } = await import('../Dialog'); + const element = React.createElement(Dialog, { title: 'Add project', visible: true, diff --git a/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts b/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts index 9d8405e..675c77c 100644 --- a/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts +++ b/Source/Dialogs/for_Dialog/when_rendered_with_is_busy.ts @@ -4,7 +4,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { vi } from 'vitest'; -import { Dialog } from '../Dialog'; vi.mock('primereact/dialog', () => { // PrimeReact 11's Dialog is compositional; each part is a pass-through that @@ -38,7 +37,15 @@ vi.mock('@cratis/arc.react/dialogs', () => ({ describe('when rendered with is busy', () => { let html: string; - beforeEach(() => { + beforeEach(async () => { + // The project runs specs with `isolate: false`, so a module imported by an + // earlier spec file stays cached with that file's mocks bound in, and the + // order files run in is not stable between runs. Re-evaluate under this + // file's own mocks so this spec neither inherits another file's stubs nor + // leaves its own behind — a static import here passes or fails by luck. + vi.resetModules(); + const { Dialog } = await import('../Dialog'); + const element = React.createElement(Dialog, { title: 'Save changes', visible: true, diff --git a/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts b/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts index 44f6634..8f839b1 100644 --- a/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts +++ b/Source/Notifications/for_toastCommandResult/when_toasting_a_command_result.ts @@ -4,9 +4,9 @@ import { vi } from 'vitest'; import type { ICommandResult } from '@cratis/arc/commands'; -// One file for all outcomes: the project runs specs with `isolate: false`, so a -// single module load + one mock avoids the cross-file mock caching that -// separate files (or vi.resetModules) would run into. +// One file for all outcomes: the project runs specs with `isolate: false`, so +// keeping every case behind one mock avoids multiplying the cross-file mock +// caching that separate files would each have to defend against. const { calls } = vi.hoisted(() => ({ calls: { success: [] as { title: string; description?: string }[], warn: [] as { title: string }[], error: [] as { title: string; description?: string }[] } })); vi.mock('primereact/toaster', () => ({ @@ -17,7 +17,19 @@ vi.mock('primereact/toaster', () => ({ }, })); -import { toastCommandResult } from '../toastCommandResult'; +// Loaded per test rather than statically imported: with `isolate: false` a module +// pulled in by an earlier spec file stays cached with that file's bindings, and +// `toastCommandResult` reaches `primereact/toaster` transitively through +// Common/CratisComponentsProvider — so a static import here binds the real toaster +// whenever another file happened to load that graph first, and the run order is not +// stable between runs. Re-evaluating under this file's own mock is what makes the +// spec deterministic. +let toastCommandResult: typeof import('../toastCommandResult').toastCommandResult; + +beforeEach(async () => { + vi.resetModules(); + ({ toastCommandResult } = await import('../toastCommandResult')); +}); const result = (over: Partial): ICommandResult => ({ isSuccess: false, isAuthorized: true, isValid: true, hasExceptions: false, validationResults: [], exceptionMessages: [], ...over } as unknown as ICommandResult);