Repository: prestojs/prestojs (language: python)

Issue:
Formatter props don't make sense
In `UiProvider` `GetFormatterForField` has generic parameter `T extends HTMLElement` which is then used in the return type `React.ComponentType<T>`. This doesn't make sense as it expects the props to the component to make `HTMLElement`. 

Requires changes to UiProvider and `getFormatterForField` 


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index df7cd2f2..cd13b97a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Changed
 
 * Fix to `InferredPaginator` that meant accessing state could be stale due to React 18 batching causing problems (eg. in `useAsyncListing`). 
+* Fix types for `getFormatterForField`. Previously required props to extend `HTMLElement` which didn't make sense - see [#179](https://github.com/prestojs/prestojs/issues/179)
 
 ## [0.0.33] - 2022-08-31
 
diff --git a/doc-site/pages/docs/ui/index.mdx b/doc-site/pages/docs/ui/index.mdx
index 72680c53..8f389938 100644
--- a/doc-site/pages/docs/ui/index.mdx
+++ b/doc-site/pages/docs/ui/index.mdx
@@ -5,25 +5,22 @@ export default ({ children, ...rest }) => <MdxPage>{children}</MdxPage>;
 
 # UI
 
-This package is concerned with providing high level components and a standard interface for customising form widgets
-and field formatters throughout the system. It doesn't provide much in terms of concrete implementations for the UI. Using the interfaces
-defined you can implement your own UI library or use a provided implementation (currently [@prestojs/ui-antd](/docs/ui-antd/) which
-integrates with [antd](https://ant.design)).
+This package is concerned with providing high level components and a standard interface for specifying how to render
+UI elements for [ViewModel](/docs/viewmodel) [Field's](doc:Field). For example, creating a form to collect data for
+a ViewModel or rendering the values of a ViewModel record.
 
-At the top level is [UiProvider](doc:UiProvider) which allows you to define the various UI specific components to use throughout
-the system. For example you can define the `formItemComponent` which is used by [@prestojs/final-form](/docs/final-form)
-or `getWidgetForField` which is used by [FieldWidget](doc:FieldWidget) to render the widget for a specific field.
+The UI components this package deals with are:
 
-[FieldFormatter](doc:FieldFormatter) is used for rendering the _value_ of a field on a form. For example the [ChoiceFormatter](doc:ChoiceFormatter)
-component renders the label for a selected choice field. This is generally used with a [ViewModel](doc:viewModelFactory) but
-doesn't have to be. `@prestojs/ui` provides implementations for this formatters as they generally just involve formatting a string
-or using standard HTML elements. To get the default implementation for a field see [getFormatterForField](doc:getFormatterForField).
+-   `formComponent` - The main form component that is used by form implementations (e.g. see [@prestojs/final-form](/docs/final-form))
+-   `formItemComponent` - The component to use for form 'items'. A form item is the input to collect a value for a field and any
+    relate information (e.g. the label for the field, help text, errors etc.).
+-   `widget` - a widget is a component used in a form to collect input from a user (e.g. a select or checkbox). This package provides a way
+    to map a [Field](doc:Field) to the widget component that should be used to render its value.
+-   `formatter` - a formatter is a component that takes a value and formats it for display. This package provides a way
+    to map a [Field](doc:Field) to the formatter component that should be used to render its value.
 
-[FieldWidget](doc:FieldWidget) is used for rendering the _input_ used to set the value in a form based on the type of a field.
-For example a [DateField](doc:DateField) may render as a calendar input widget as implemented by [DateWidget](doc:DateWidget).
-[FieldWidget](doc:FieldWidget) resolves the UI specific widget to use for a [Field](doc:Field). `@prestojs/ui` doesn't
-currently provide a default implementation for these as they tend to be highly specific. `@prestojs/ui-antd` provides an implementation
-for [antd](https://ant.design) or you can implement your own.
+This package provides concrete implementations for formatters (see [getFormatterForField](doc:getFormatterForField)). For
+form components you either need to provide an implementation or use [@prestojs/ui-antd](/docs/ui-antd) and [@prestojs/final-form](/docs/final-form).
 
 ---
 
@@ -33,25 +30,166 @@ for [antd](https://ant.design) or you can implement your own.
 yarn add @prestojs/ui
 ```
 
-Then you will need to add the [UiProvider](doc:UiProvider) at the top of your app. See [it's documentation](doc:UiProvider)
-for an example of this.
-
 ## Usage
 
+At the top level is [UiProvider](doc:UiProvider) which allows you to define the various UI specific components to use throughout
+the system. For example you can define the `formItemComponent` which is used by [@prestojs/final-form](/docs/final-form)
+or `getWidgetForField` which is used by [FieldWidget](doc:FieldWidget) to render the widget for a specific field.
+
+```js
+function RootView() {
+    return (
+        <UiProvider
+            getFormatterForField={getFormatterForField}
+            getWidgetForField={getWidgetForField}
+            formItemComponent={FormItemWrapper}
+        >
+            <App />
+        </UiProvider>
+    );
+}
+```
+
+See [UiProvider](doc:UiProvider) for more details.
+
 ### Formatters
 
-A field formatter is just a component that accepts a `value` prop and renders it:
+[FieldFormatter](doc:FieldFormatter) is used for rendering the _value_ of a field. For example the [ChoiceFormatter](doc:ChoiceFormatter)
+component renders the label for a selected choice field. This is generally used with a [ViewModel](doc:viewModelFactory) but
+doesn't have to be.
+
+```js
+class Person extends viewModelFactory(
+    {
+        id: new Field(),
+        isActive: new BooleanField({ label: 'Active?' }),
+    },
+    { pkFieldName: 'id' }
+) {}
+
+function PersonDetail({ person }) {
+    return (
+        <dl>
+            <dt>{Person.fields.isActive.label}</dt>
+            <dd>
+                <FieldFormatter field={person._f.isActive} />
+            </dd>
+        </dl>
+    );
+}
+```
+
+In the above example [BooleanFormatter](doc:BooleanFormatter) will be used as that's the default formatter for [BooleanField](doc:BooleanField).
+The advantage of this over hard-coding the formatter everywhere is you can change how a particular field type is formatted
+in a central location. If you wanted to change how `BooleanField` is formatted you could implement your own formatter. All
+a formatter is is a component that accepts, at minimum, a `value` prop:
+
+```js
+function BooleanEmojiFormatter({ value }) {
+    if (value == null || value == '') {
+        return <>'❓'</>;
+    }
+    return <>{value ? '✅' : '❌'}</>;
+}
+```
+
+To use this formatter everywhere a [BooleanField](doc:BooleanField) appears provide you own implementation of
+`getFormatterForField`:
+
+```js
+import { getFormatterForField as defaultGetFormatterForField } from '@prestojs/ui';
+
+function getFormatterForField(field) {
+    if (field instanceof BooleanField) {
+        // You can just return the formatter component directly but it's good practice to return
+        // the component & `field.getFormatterProps` which allows per field customisations. See
+        // example further below where `formatterProps` are passed to a field for how this can
+        // be used.
+        return [BooleanEmojiFormatter, field.getFormatterProps()];
+    }
+    // Fall back to the default implementation
+    return defaultGetFormatterForField(field);
+}
+
+// Then pass this to the `UiProvider` that wraps your app
+<UiProvider getFormatterForField={getFormatterForField}>{...}</UiProvider>
+```
+
+As shown in the above example `getFormatterForField` can return a 2-element array of the form `[component, defaultProps]`.
+This allows you to inject props used on a component based on the field. We could rewrite the above to use the options
+provided by the existing [BooleanFormatter](doc:BooleanFormatter) component:
 
 ```js
-function UpperCaseFormatter({ value }) {
-    if (!value) {
-        return value;
+import { getFormatterForField as defaultGetFormatterForField } from '@prestojs/ui';
+
+function getFormatterForField(field) {
+    if (field instanceof BooleanField) {
+        return [
+            BooleanFormatter,
+            {
+                trueLabel: '✅',
+                falseLabel: '❌',
+                blankLabel: '❓',
+                // Include this last so individual fields can override that above defaults
+                ...field.getFormatterProps(),
+            },
+        ];
     }
-    return value.toUpperCase();
+    // Fall back to the default implementation
+    return defaultGetFormatterForField(field);
 }
 ```
 
-When you use [FieldFormatter](doc:FieldFormatter) it just works out what formatter component to use based on what the
-`getFormatterForField` function on your [UiProvider](doc:UiProvider) returns.
+See [getFormatterForField](doc:getFormatterForField) for the default implementation.
+
+If you just need to override formatter props for a specific field you can pass that through to the field constructor
+itself using the `formatterProps` option. This is then returned by `field.getFormatterProps()`:
+
+```js
+new BooleanField({
+    label: 'Active?',
+    formatterProps: {
+        trueLabel: '✅',
+        falseLabel: '❌',
+        blankLabel: '❓',
+    },
+});
+```
 
 ### Widgets
+
+Widgets are conceptually similar to formatters but instead of rendering a value they are used to collect a value.
+For example a [DateField](doc:DateField) may render as a calendar input widget as implemented by [DateWidget](doc:DateWidget).
+
+[FieldWidget](doc:FieldWidget) resolves the UI specific widget to use for a [Field](doc:Field). `@prestojs/ui` doesn't
+currently provide a default implementation for these as they tend to be highly specific. `@prestojs/ui-antd` provides an implementation
+for [antd](https://ant.design) or you can implement your own.
+
+### Generic components
+
+With this structure in place you can write generic components that don't need to know the type of the field.
+
+For example, here's a view that renders all the fields on a record without know anything about the ViewModel.
+
+```js
+function RecordDetailView({ record }) {
+    return (
+        <dl>
+            {record._assignedFields.map(fieldName => {
+                const field = record._f[fieldName];
+                if (field.writeOnly || record._model.pkFieldNames.includes(fieldName)) {
+                    return null;
+                }
+                return (
+                    <React.Fragment key={fieldName}>
+                        <dt>{field.label}</dt>
+                        <dd>
+                            <FieldFormatter field={field} />
+                        </dd>
+                    </React.Fragment>
+                );
+            })}
+        </dl>
+    );
+}
+```
diff --git a/doc-site/pages/examples/ui/FieldFormatter/basic.tsx b/doc-site/pages/examples/ui/FieldFormatter/basic.tsx
index 6045a006..26ac719d 100644
--- a/doc-site/pages/examples/ui/FieldFormatter/basic.tsx
+++ b/doc-site/pages/examples/ui/FieldFormatter/basic.tsx
@@ -1,4 +1,13 @@
-import { FieldFormatter, NumberFormatter, UiProvider } from '@prestojs/ui';
+/**
+ * FieldFormatter usage
+ */
+import {
+    FieldFormatter,
+    FormatterComponentDefinition,
+    getFormatterForField as defaultGetFormatterForField,
+    NumberFormatter,
+    UiProvider,
+} from '@prestojs/ui';
 import {
     BooleanField,
     CharField,
@@ -7,17 +16,18 @@ import {
     viewModelFactory,
 } from '@prestojs/viewmodel';
 import React from 'react';
-// You can replace this with
-// import { getFormatterForField } from '@prestojs/ui';
-// We use custom version here to support nextjs (which is what doc site is written in)
-import defaultGetFormatterForField from '../../../../getFormatterForField';
 
-function getFormatterForField(field) {
+function getFormatterForField(field): FormatterComponentDefinition | null {
     // Provide override for CurrencyField
     if (field instanceof CurrencyField) {
         return [
             NumberFormatter,
-            { localeOptions: { style: 'currency', currency: 'USD', currencyDisplay: 'code' } },
+            {
+                localeOptions: { style: 'currency', currency: 'USD', currencyDisplay: 'code' },
+                // It's good practice to always pass this through so any per field customisations
+                // are respected. Include this last so individual fields can override the above defaults
+                ...field.getFormatterProps(),
+            },
         ];
     }
     // Otherwise fall back to defaults
@@ -29,7 +39,10 @@ class Product extends viewModelFactory(
         id: new NumberField(),
         name: new CharField(),
         price: new CurrencyField(),
-        active: new BooleanField({ label: 'Active?' }),
+        active: new BooleanField({
+            label: 'Active?',
+            formatterProps: { trueLabel: 'Active', falseLabel: 'In-active' },
+        }),
     },
     { pkFieldName: 'id' }
 ) {}
diff --git a/js-packages/@prestojs/ui/src/UiProvider.tsx b/js-packages/@prestojs/ui/src/UiProvider.tsx
index f3d6d9aa..ca7baecf 100644
--- a/js-packages/@prestojs/ui/src/UiProvider.tsx
+++ b/js-packages/@prestojs/ui/src/UiProvider.tsx
@@ -2,6 +2,20 @@ import { Field } from '@prestojs/viewmodel';
 import React, { useContext, useMemo } from 'react';
 import { FieldWidgetType } from './FieldWidgetInterface';
 
+interface FormatterComponentProps<ValueT> {
+    value?: ValueT;
+}
+
+/**
+ * A formatter component definition is either a regular component or a 2-element array where
+ * the first element is the component function and the second are the props that should be passed
+ * to that component.
+ */
+export type FormatterComponentDefinition<ValueT = any> =
+    | string
+    | React.ComponentType<FormatterComponentProps<ValueT>>
+    | [React.ComponentType<FormatterComponentProps<ValueT>> | string, Record<string, unknown>];
+
 type GetWidgetForField = <FieldValueT, ParsableValueT, SingleValueT, T extends HTMLElement>(
     field: Field<FieldValueT, ParsableValueT, SingleValueT>
 ) => FieldWidgetType<FieldValueT, T> | [FieldWidgetType<FieldValueT, T>, Record<string, unknown>];
@@ -13,22 +27,13 @@ type GetWidgetForFieldWithNull = <FieldValueT, ParsableValueT, SingleValueT, T e
     | [FieldWidgetType<FieldValueT, T>, Record<string, unknown>]
     | null;
 
-type GetFormatterForField = <FieldValueT, ParsableValueT, SingleValueT, T extends HTMLElement>(
+type GetFormatterForField = <FieldValueT, ParsableValueT, SingleValueT>(
     field: Field<FieldValueT, ParsableValueT, SingleValueT>
-) => string | React.ComponentType<T> | [React.ComponentType<T> | string, Record<string, unknown>];
+) => FormatterComponentDefinition;
 
-type GetFormatterForFieldWithNull = <
-    FieldValueT,
-    ParsableValueT,
-    SingleValueT,
-    T extends HTMLElement
->(
+type GetFormatterForFieldWithNull = <FieldValueT, ParsableValueT, SingleValueT>(
     field: Field<FieldValueT, ParsableValueT, SingleValueT>
-) =>
-    | string
-    | React.ComponentType<T>
-    | [React.ComponentType<T> | string, Record<string, unknown>]
-    | null;
+) => FormatterComponentDefinition | null;
 
 export interface FormItemProps {
     children: React.ReactNode;
@@ -123,7 +128,7 @@ export type UiProviderProps = {
  *
  * ```jsx
  * import React from 'react';
- * import { UiProvider, getFormatterForField } from '@prestojs/ui';
+ * import { UiProvider, getFormatterForField as defaultGetFormatterForField } from '@prestojs/ui';
  * import { Input } from 'antd';
  *
  * const DefaultWidget = ({ input }) => <input {...input} />;
@@ -144,7 +149,11 @@ export type UiProviderProps = {
  *     // if (field instanceof BooleanField) {
  *     //    return CustomBooleanFormatter;
  *     // }
- *     return DefaultFormatter;
+ *     const formatter = defaultGetFormatterForField(field)
+ *     if (!formatter) {
+ *         return DefaultFormatter;
+ *     }
+ *     return formatter;
  * }
  *
  * function FormItemWrapper({ children, label, help, required }) {
@@ -216,17 +225,23 @@ export default function UiProvider(props: UiProviderProps): React.ReactElement {
                 }
                 return widget;
             },
-            getFormatterForField<FieldValueT, ParsableValueT, SingleValueT, T extends HTMLElement>(
+            getFormatterForField<FieldValueT, ParsableValueT, SingleValueT>(
                 field: Field<FieldValueT, ParsableValueT, SingleValueT>
             ):
                 | string
-                | React.ComponentType<T>
-                | [React.ComponentType<T> | string, Record<string, unknown>]
+                | React.ComponentType<FormatterComponentProps<FieldValueT>>
+                | [
+                      React.ComponentType<FormatterComponentProps<FieldValueT>> | string,
+                      Record<string, unknown>
+                  ]
                 | null {
                 let formatter:
                     | string
-                    | React.ComponentType<T>
-                    | [React.ComponentType<T> | string, Record<string, unknown>]
+                    | React.ComponentType<FormatterComponentProps<FieldValueT>>
+                    | [
+                          React.ComponentType<FormatterComponentProps<FieldValueT>> | string,
+                          Record<string, unknown>
+                      ]
                     | null = null;
                 if (getFormatterForField) {
                     formatter = getFormatterForField(field);
diff --git a/js-packages/@prestojs/ui/src/getFormatterForField.ts b/js-packages/@prestojs/ui/src/getFormatterForField.ts
index 746c1b34..dfc57a0d 100644
--- a/js-packages/@prestojs/ui/src/getFormatterForField.ts
+++ b/js-packages/@prestojs/ui/src/getFormatterForField.ts
@@ -1,5 +1,6 @@
 import { Field } from '@prestojs/viewmodel';
 import React from 'react';
+import type { FormatterComponentDefinition } from './UiProvider';
 
 const LinkFormatter = React.lazy(() => import('./formatters/LinkFormatter'));
 const RangeFormatter = React.lazy(() => import('./formatters/RangeFormatter'));
@@ -64,11 +65,6 @@ const choicesMapping = new Map<string, any>([
     ['IntegerField', ChoiceFormatter],
 ]);
 
-type WidgetComponent<T> =
-    | React.ComponentType<T>
-    | [React.ComponentType<T> | string, Record<string, unknown>]
-    | string;
-
 /**
  * Returns the default formatter for a given [Field](doc:Field).
  *
@@ -83,21 +79,18 @@ type WidgetComponent<T> =
  *
  * @extract-docs
  */
-export default function getFormatterForField<
-    FieldValue,
-    ParsableValueT,
-    SingleValueT,
-    T extends HTMLElement
->(field: Field<FieldValue, ParsableValueT, SingleValueT>): WidgetComponent<T> | null {
+export default function getFormatterForField<FieldValue, ParsableValueT, SingleValueT>(
+    field: Field<FieldValue, ParsableValueT, SingleValueT>
+): FormatterComponentDefinition<FieldValue> | null {
     const { fieldClassName } = Object.getPrototypeOf(field).constructor;
     const formatter: React.FunctionComponent | string | null | undefined = field.choices
         ? choicesMapping.get(fieldClassName) || mapping.get(fieldClassName)
         : mapping.get(fieldClassName);
 
     const getReturnWithChoices = (
-        w: WidgetComponent<T>,
+        w: FormatterComponentDefinition<FieldValue>,
         f: Field<FieldValue, ParsableValueT, SingleValueT>
-    ): WidgetComponent<T> => {
+    ): FormatterComponentDefinition<FieldValue> => {
         if (f.choices) {
             if (Array.isArray(w)) {
                 return [w[0], { ...f.getFormatterProps(), ...w[1], choices: f.choices }];
diff --git a/js-packages/@prestojs/ui/src/index.ts b/js-packages/@prestojs/ui/src/index.ts
index 0bc62255..71d4d36d 100644
--- a/js-packages/@prestojs/ui/src/index.ts
+++ b/js-packages/@prestojs/ui/src/index.ts
@@ -21,6 +21,7 @@ import type {
     WidgetProps,
 } from './FieldWidgetInterface';
 import type {
+    FormatterComponentDefinition,
     FormItemProps,
     FormProps,
     TopLevelUiContextValue,
@@ -38,4 +39,5 @@ export type {
     UiContextValue,
     UiProviderProps,
     TopLevelUiContextValue,
+    FormatterComponentDefinition,
 };


Test patch:
diff --git a/js-packages/@prestojs/viewmodel/src/__tests__/ViewModelCache.test.ts b/js-packages/@prestojs/viewmodel/src/__tests__/ViewModelCache.test.ts
index f187e5f9..d39b070c 100644
--- a/js-packages/@prestojs/viewmodel/src/__tests__/ViewModelCache.test.ts
+++ b/js-packages/@prestojs/viewmodel/src/__tests__/ViewModelCache.test.ts
@@ -2970,7 +2970,7 @@ test('should support manual nested batching', async () => {
 });
 
 test('many fields should work with empty list of ids', async () => {
-    const { User, Group } = createTestModels(true, true);
+    const { User } = createTestModels(true, true);
     await User.fields.group.resolveViewModel();
 
     const cb = jest.fn();