diff --git a/.changeset/son-4389-controlled-react-input-wrapper.md b/.changeset/son-4389-controlled-react-input-wrapper.md new file mode 100644 index 0000000000..9801284c5a --- /dev/null +++ b/.changeset/son-4389-controlled-react-input-wrapper.md @@ -0,0 +1,9 @@ +--- +"@justeattakeaway/pie-wrapper-react": minor +--- + +[Added] - Controlled-input-aware React wrapper generation for input-like components. + +`@lit/react`'s `createComponent` pushes the `value` prop onto the custom element whenever it changes between renders, but it does not replicate the controlled-input value tracking that ReactDOM applies to native form controls. During fast typing with debounced parent state, a stale `value` could be written back to the element and overwrite the user's latest keystroke. + +Components that expose a `value` field and dispatch an `input` event now receive a generated wrapper that replicates React's controlled-input contract: `value` is only written to the element when the controlled value genuinely changes since it was last applied, so in-flight user input is never clobbered. Components that are not input-like are unaffected and continue to use the existing `createComponent` export. diff --git a/.changeset/son-4389-pie-text-input-controlled.md b/.changeset/son-4389-pie-text-input-controlled.md new file mode 100644 index 0000000000..86c15fe177 --- /dev/null +++ b/.changeset/son-4389-pie-text-input-controlled.md @@ -0,0 +1,5 @@ +--- +"@justeattakeaway/pie-text-input": patch +--- + +[Fixed] - The React wrapper no longer drops keystrokes when the `value` prop lags behind user input (for example with debounced parent state). The generated wrapper now follows React's controlled-input contract and only writes `value` to the element when it genuinely changes. diff --git a/packages/tools/pie-wrapper-react/__tests__/__snapshots__/wrapper.test.js.snap b/packages/tools/pie-wrapper-react/__tests__/__snapshots__/wrapper.test.js.snap index 7b96426c41..762ceac8b0 100644 --- a/packages/tools/pie-wrapper-react/__tests__/__snapshots__/wrapper.test.js.snap +++ b/packages/tools/pie-wrapper-react/__tests__/__snapshots__/wrapper.test.js.snap @@ -50,6 +50,87 @@ export const ButtonComponent = ButtonComponentReact as React.ForwardRefExoticCom " `; +exports[`React Wrapper > should generate a controlled-input wrapper for components with a value field and an input event 1`] = ` +"import * as React from 'react'; +import { createComponent, type EventName } from '@lit/react'; +import { PieTextInput as PieTextInputLit } from './index'; +import { type TextInputProps } from './defs'; + +export * from './defs'; + +const PieTextInputReact = createComponent({ + displayName: 'PieTextInput', + elementClass: PieTextInputLit, + react: React, + tagName: 'pie-text-input', + events: { + onInput: 'input' as EventName, + onChange: 'change' as EventName, + }, +}); + +type PieTextInputEvents = { + onInput?: (event: InputEvent) => void; + onChange?: (event: CustomEvent) => void; +}; + +type PieTextInputWrapperProps = React.PropsWithChildren, 'children'>> + & React.RefAttributes & PieTextInputEvents; + +// Use a layout effect on the client (so value writes happen before paint) and a +// regular effect on the server to avoid React's "useLayoutEffect does nothing on +// the server" warning during SSR. +const useIsomorphicPieTextInputLayoutEffect = typeof window !== 'undefined' ? React.useLayoutEffect : React.useEffect; + +const PieTextInputControlled = React.forwardRef((props, forwardedRef) => { + const { value, children, ...restProps } = props as PieTextInputWrapperProps & { value?: string | number | null }; + const elementRef = React.useRef(null); + const lastAppliedValueRef = React.useRef(undefined); + + const setElementRef = React.useCallback((node: PieTextInputLit | null) => { + elementRef.current = node; + + if (typeof forwardedRef === 'function') { + forwardedRef(node); + } else if (forwardedRef) { + (forwardedRef as React.MutableRefObject).current = node; + } + }, [forwardedRef]); + + useIsomorphicPieTextInputLayoutEffect(() => { + const element = elementRef.current; + + if (!element) { + return; + } + + // Only write when the controlled value has actually changed since we last + // applied it. This prevents a re-render carrying an already-applied value + // from overwriting characters the user has typed in the meantime. + if (value !== lastAppliedValueRef.current) { + lastAppliedValueRef.current = value; + + const nextValue = value == null ? '' : String(value); + + if (element.value !== nextValue) { + element.value = nextValue; + } + } + }); + + return React.createElement( + PieTextInputReact, + { ...restProps, ref: setElementRef } as unknown as React.ComponentProps, + children, + ); +}); + +PieTextInputControlled.displayName = 'PieTextInput'; + +export const PieTextInput = PieTextInputControlled as React.ForwardRefExoticComponent; +" +`; + exports[`React Wrapper > should leave the events object empty if the component contains no custom events 1`] = ` "import * as React from 'react'; import { createComponent } from '@lit/react'; diff --git a/packages/tools/pie-wrapper-react/__tests__/wrapper.test.js b/packages/tools/pie-wrapper-react/__tests__/wrapper.test.js index 6c808879e0..e92977b4a5 100644 --- a/packages/tools/pie-wrapper-react/__tests__/wrapper.test.js +++ b/packages/tools/pie-wrapper-react/__tests__/wrapper.test.js @@ -50,4 +50,79 @@ describe('React Wrapper', () => { expect(wrapper).toMatchSnapshot(); expect(wrapper.includes(result)).toBe(true); }); + + it('should generate a controlled-input wrapper for components with a value field and an input event', () => { + const inputManifest = { + schemaVersion: '1.0.0', + readme: '', + modules: [ + { + kind: 'javascript-module', + path: 'pie-wrapper-react', + declarations: [ + { + kind: 'class', + description: '', + name: 'PieTextInput', + members: [ + { kind: 'field', name: 'value' }, + ], + events: [ + { name: 'input', type: { text: 'InputEvent' } }, + { name: 'change', type: { text: 'CustomEvent' } }, + ], + superclass: { name: 'LitElement', package: 'lit' }, + tagName: 'pie-text-input', + customElement: true, + }, + ], + }, + ], + }; + + const wrapper = addReactWrapper(inputManifest); + + // The controlled wrapper replaces the direct createComponent export. + expect(wrapper).toContain('React.forwardRef'); + expect(wrapper).toContain('lastAppliedValueRef'); + expect(wrapper).toContain("PieTextInputControlled.displayName = 'PieTextInput'"); + expect(wrapper).toContain('export const PieTextInput = PieTextInputControlled'); + // The value prop must not be forwarded to the underlying createComponent element. + expect(wrapper).not.toContain('export const PieTextInput = PieTextInputReact'); + expect(wrapper).toMatchSnapshot(); + }); + + it('should not generate a controlled-input wrapper for components without an input event', () => { + const noInputManifest = { + schemaVersion: '1.0.0', + readme: '', + modules: [ + { + kind: 'javascript-module', + path: 'pie-wrapper-react', + declarations: [ + { + kind: 'class', + description: '', + name: 'PieSelect', + members: [ + { kind: 'field', name: 'value' }, + ], + events: [ + { name: 'change', type: { text: 'CustomEvent' } }, + ], + superclass: { name: 'LitElement', package: 'lit' }, + tagName: 'pie-select', + customElement: true, + }, + ], + }, + ], + }; + + const wrapper = addReactWrapper(noInputManifest); + + expect(wrapper).toContain('export const PieSelect = PieSelectReact'); + expect(wrapper).not.toContain('lastAppliedValueRef'); + }); }); diff --git a/packages/tools/pie-wrapper-react/scripts/add-react-wrapper.js b/packages/tools/pie-wrapper-react/scripts/add-react-wrapper.js index 480c0487db..cd5c0243a4 100755 --- a/packages/tools/pie-wrapper-react/scripts/add-react-wrapper.js +++ b/packages/tools/pie-wrapper-react/scripts/add-react-wrapper.js @@ -91,6 +91,100 @@ function formatEventName (eventName) { .join(''); } +/** + * Determines whether a component behaves like a controlled text input. + * + * A component is treated as a controlled input when it exposes a `value` field + * and dispatches an `input` event. These are the components for which `@lit/react`'s + * generic wrapper is insufficient, because it does not replicate the controlled-input + * value tracking that ReactDOM applies to native form controls. + * + * @param {object} componentClass - the component declaration from the custom elements manifest + * @return {boolean} - true if the component should receive the controlled-input wrapper + */ +function isControlledInputComponent (componentClass) { + const hasValueField = (componentClass.members || []) + .some((member) => member.kind === 'field' && member.name === 'value'); + const hasInputEvent = (componentClass.events || []) + .some((event) => event.name === 'input'); + + return hasValueField && hasInputEvent; +} + +/** + * Generates a controlled-input-aware React wrapper for input-like components. + * + * `@lit/react`'s `createComponent` writes the `value` prop onto the custom element + * whenever it changes between renders, but it does not implement the value tracking + * that ReactDOM applies to native form controls. As a result a stale `value` (for + * example while a user is typing and parent state updates are debounced) can overwrite + * the user's latest keystroke. + * + * The generated wrapper replicates React's controlled-input contract: `value` is only + * written to the element when the controlled value genuinely changes since it was last + * applied, so in-flight user input is never clobbered. The `value` prop is therefore not + * forwarded to the underlying `createComponent` element and is managed here instead. + * + * @param {string} componentName - the component class name (e.g. `PieTextInput`) + * @param {string} propsTypeDefinition - the composed React props type for the component + * @return {string} - the source code for the controlled wrapper and its export + */ +function generateControlledWrapper (componentName, propsTypeDefinition) { + return `type ${componentName}WrapperProps = ${propsTypeDefinition}; + +// Use a layout effect on the client (so value writes happen before paint) and a +// regular effect on the server to avoid React's "useLayoutEffect does nothing on +// the server" warning during SSR. +const useIsomorphic${componentName}LayoutEffect = typeof window !== 'undefined' ? React.useLayoutEffect : React.useEffect; + +const ${componentName}Controlled = React.forwardRef<${componentName}Lit, ${componentName}WrapperProps>((props, forwardedRef) => { + const { value, children, ...restProps } = props as ${componentName}WrapperProps & { value?: string | number | null }; + const elementRef = React.useRef<${componentName}Lit | null>(null); + const lastAppliedValueRef = React.useRef(undefined); + + const setElementRef = React.useCallback((node: ${componentName}Lit | null) => { + elementRef.current = node; + + if (typeof forwardedRef === 'function') { + forwardedRef(node); + } else if (forwardedRef) { + (forwardedRef as React.MutableRefObject<${componentName}Lit | null>).current = node; + } + }, [forwardedRef]); + + useIsomorphic${componentName}LayoutEffect(() => { + const element = elementRef.current; + + if (!element) { + return; + } + + // Only write when the controlled value has actually changed since we last + // applied it. This prevents a re-render carrying an already-applied value + // from overwriting characters the user has typed in the meantime. + if (value !== lastAppliedValueRef.current) { + lastAppliedValueRef.current = value; + + const nextValue = value == null ? '' : String(value); + + if (element.value !== nextValue) { + element.value = nextValue; + } + } + }); + + return React.createElement( + ${componentName}React, + { ...restProps, ref: setElementRef } as unknown as React.ComponentProps, + children, + ); +}); + +${componentName}Controlled.displayName = '${componentName}'; + +export const ${componentName} = ${componentName}Controlled as React.ForwardRefExoticComponent<${componentName}WrapperProps>;`; +} + /** * This function generates a react wrapper to enable custom lit components to be used in react apps. * @@ -186,8 +280,12 @@ export function addReactWrapper (customElementsObject) { const componentPropsExportName = `${component.class.name.replace(/^Pie/, '')}Props`; - // Create the main source code - componentSrc = `import * as React from 'react'; + // The composed React props type, shared by the standard and controlled exports. + const propsTypeDefinition = `React.PropsWithChildren, 'children'>> + & React.RefAttributes<${component.class.name}Lit>${eventsTypeDefinition ? ` & ${eventsTypeName}` : ''}${component.reactBaseType ? ' & ReactBaseType' : ''}`; + + // Shared header: imports, the createComponent element, and optional types. + const header = `import * as React from 'react'; import { createComponent${component.class.events?.length > 0 ? ', type EventName' : ''} } from '@lit/react'; import { ${component.class.name} as ${component.class.name}Lit } from './index'; import { type ${componentPropsExportName} } from './defs'; @@ -208,11 +306,21 @@ ${component.reactBaseType}` : '' eventsTypeDefinition ? ` ${eventsTypeDefinition}` : '' -} +}`; -export const ${component.class.name} = ${component.class.name}React as React.ForwardRefExoticComponent, 'children'>> - & React.RefAttributes<${component.class.name}Lit>${eventsTypeDefinition ? ` & ${eventsTypeName}` : ''}${component.reactBaseType ? ' & ReactBaseType' : ''}>; + // Input-like components need a controlled-input-aware wrapper; everything else + // can be exported directly from the generic createComponent element. + if (isControlledInputComponent(component.class)) { + componentSrc = `${header} + +${generateControlledWrapper(component.class.name, propsTypeDefinition)} +`; + } else { + componentSrc = `${header} + +export const ${component.class.name} = ${component.class.name}React as React.ForwardRefExoticComponent<${propsTypeDefinition}>; `; + } let reactFile; if (component.path !== 'pie-wrapper-react') {