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/ESLint/README.md b/ESLint/README.md index fe36d64e..53987ccd 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 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..89db0ea7 --- /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 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' } }], + }, + ], +}); 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/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_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..88b4b278 --- /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_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..b050b30a --- /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 00000000..212d2cf3 --- /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_inspecting_the_column_wrapper.ts b/Source/CommandDialog/for_CommandDialog/when_inspecting_the_column_wrapper.ts new file mode 100644 index 00000000..5713e454 --- /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_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..00531df0 --- /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 00000000..5bfa7965 --- /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 00000000..fee7c02e --- /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 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 new file mode 100644 index 00000000..e14e78fd --- /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 00000000..4f8a043b --- /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 00000000..aab86618 --- /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 00000000..5997e410 --- /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 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'; 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, + })); +} 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(),