From 5c109df249c47bcd0534a897126a4ec9e4d5d193 Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 12:12:21 -0700 Subject: [PATCH 01/13] Add tests --- .../tests/stability.changeMiddleware.test.tsx | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx diff --git a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx new file mode 100644 index 0000000..d1a2da7 --- /dev/null +++ b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx @@ -0,0 +1,113 @@ +/** @jest-environment jsdom */ +/// + +import { scenario } from '@testduet/given-when-then'; +import { render } from '@testing-library/react'; +import React, { Fragment, memo } from 'react'; + +import createChainOfResponsibility, { type InferMiddleware } from '../createChainOfResponsibilityAsRenderCallback'; + +type Props = { readonly children?: never }; +type Request = number; + +scenario('stability test with changing middleware', bdd => { + bdd + .given('a TestComponent using chain of responsiblity', () => { + const { Provider, reactComponent, useBuildRenderCallback } = createChainOfResponsibility(); + + const alohaCall = jest.fn(); + + // Middleware prop change always re-render. + // Memoizing to focus on whether the component is rendering as waste. + const Aloha = memo(function Aloha() { + alohaCall(); + + return Aloha!; + }); + + const helloWorldCall = jest.fn(); + + // Middleware prop change always re-render. + // Memoizing to focus on whether the component is rendering as waste. + const HelloWorld = memo(function HelloWorld() { + helloWorldCall(); + + return Hello, World!; + }); + + const myProxyCall = jest.fn(); + + const MyProxy = memo(function MyProxy({ request }: { readonly request: number }) { + myProxyCall(); + + // We are using `useBuildRenderCallback` for less memoization. + const result = useBuildRenderCallback()(request)?.({}); + + return result ? {result} : null; + }); + + return Object.freeze({ + Aloha, + alohaCall, + HelloWorld, + helloWorldCall, + myProxyCall, + reactComponent, + TestComponent: function TestComponent({ + middleware + }: { + readonly middleware: readonly InferMiddleware[]; + }) { + return ( + + + + ); + } + }); + }) + .when('the component is rendered', ({ HelloWorld, reactComponent, TestComponent }) => + render( () => () => reactComponent(HelloWorld)]} />) + ) + .then('textContent should match', (_, { container }) => + expect(container).toHaveProperty('textContent', 'Hello, World!') + ) + .and(' should have been rendered once', ({ helloWorldCall }) => + expect(helloWorldCall).toHaveBeenCalledTimes(1) + ) + .and(' should have been rendered once', ({ myProxyCall }) => expect(myProxyCall).toHaveBeenCalledTimes(1)) + .when( + 'the component is re-rendered with a different instance but same middleware', + ({ HelloWorld, reactComponent, TestComponent }, result) => { + // Still rendering , but in a different middleware. + result.rerender( () => () => reactComponent(HelloWorld)]} />); + + return result; + } + ) + .then('textContent should match', (_, { container }) => + expect(container).toHaveProperty('textContent', 'Hello, World!') + ) + .and(' should not have been re-rendered', ({ helloWorldCall }) => + expect(helloWorldCall).toHaveBeenCalledTimes(1) + ) + // Every time middleware change, it should invalidate `useBuildRenderCallback`. + // In future, we may do more granular check to see if enhancer actually changed or not and reduce wasted rendering. + .and(' should have been rendered once', ({ myProxyCall }) => expect(myProxyCall).toHaveBeenCalledTimes(2)) + .when( + 'the component is re-rendered with a different instance but same middleware', + ({ Aloha, reactComponent, TestComponent }, result) => { + result.rerender( () => () => reactComponent(Aloha)]} />); + + return result; + } + ) + .then('textContent should match', (_, { container }) => expect(container).toHaveProperty('textContent', 'Aloha!')) + .and(' should have been rendered once', ({ alohaCall }) => expect(alohaCall).toHaveBeenCalledTimes(1)) + .and(' should have been rendered once', ({ helloWorldCall }) => + expect(helloWorldCall).toHaveBeenCalledTimes(1) + ) + .and(' should have been rendered 3 times', ({ myProxyCall }) => + expect(myProxyCall).toHaveBeenCalledTimes(3) + ); +}); From af1231743248aabcb0b870b2fe911e80e980bf4b Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 13:30:29 -0700 Subject: [PATCH 02/13] Clean up no collision prop --- ...eChainOfResponsibilityAsRenderCallback.tsx | 82 +++++++++---------- 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx index 8e4af17..548e5b0 100644 --- a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx +++ b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx @@ -145,6 +145,12 @@ type InferProviderProps> = T['~types'][ // eslint-disable-next-line @typescript-eslint/no-explicit-any type InferRequest> = T['~types']['request']; +function createComponentHandlerResult( + render: (overridingProps?: Partial | undefined) => ReactNode +): ComponentHandlerResult { + return Object.freeze({ [DO_NOT_CREATE_THIS_OBJECT_YOURSELF]: undefined, render }); +} + function createChainOfResponsibility< Request = void, Props extends BaseProps = { readonly children?: never }, @@ -177,16 +183,13 @@ function createChainOfResponsibility< | ((props: Props) => Partial & Omit) | undefined ): ComponentHandlerResult { - return Object.freeze({ - [DO_NOT_CREATE_THIS_OBJECT_YOURSELF]: undefined, - render: (overridingProps?: Partial | undefined) => ( - } - overridingProps={overridingProps} - /> - ) - }); + return createComponentHandlerResult((overridingProps?: Partial | undefined) => ( + } + overridingProps={overridingProps} + /> + )); } const ComponentWithProps = memo(function ComponentWithProps({ @@ -212,8 +215,6 @@ function createChainOfResponsibility< return ; }); - const RENDER_CALLBACK_SYMBOL = `REACT_CHAIN_OF_RESPONSIBILITY:DO_NOT_USE_THIS_RENDER_CALLBACK`; - const useBuildRenderCallback: () => UseBuildRenderCallback = () => { const { enhancer } = useContext(BuildContext); @@ -234,28 +235,19 @@ function createChainOfResponsibility< return; } - return Object.freeze({ - // Convert fallback component as renderer. - [DO_NOT_CREATE_THIS_OBJECT_YOURSELF]: undefined, - render: () => ( - // Currently, there are no ways to set `bindProps` to `fallbackComponent`. - // `fallbackComponent` do not need `overridingProps` because it is the last one in the chain, it would not have the next() function. - - ) - }); + // Convert fallback component as renderer. + return createComponentHandlerResult(() => ( + // Currently, there are no ways to set `bindProps` to `fallbackComponent`. + // `fallbackComponent` do not need `overridingProps` because it is the last one in the chain, it would not have the next() function. + + )); })(request); return ( result && - ((props: Props) => ( + ((originalProps: Props) => ( // This is render function, we cannot call any hooks here. - + )) ); }, @@ -263,20 +255,20 @@ function createChainOfResponsibility< ); }; - type RenderCallbackAsComponentProps = Props & { - // First render function does not need overrideProps. - // Override props is for upstreamer to override props before passing to downsteamers. - readonly [RENDER_CALLBACK_SYMBOL]: () => ReactNode; + type BuildRenderCallbackProps = { + readonly originalProps: Props; + readonly render: () => ReactNode; }; - const RenderCallbackAsComponent = memo(function RenderFunction({ - [RENDER_CALLBACK_SYMBOL]: render, - ...props - }: RenderCallbackAsComponentProps) { - const context = useMemo>(() => Object.freeze({ originalProps: props as Props }), [props]); + const BuildRenderCallback = memo( + function BuildRenderCallback({ originalProps, render }: BuildRenderCallbackProps) { + const context = useMemo>(() => Object.freeze({ originalProps }), [originalProps]); - return {render()}; - }); + return {render()}; + }, + (prevProps, nextProps) => + arePropsEqual(prevProps.originalProps, nextProps.originalProps) && Object.is(prevProps.render, nextProps.render) + ); function ChainOfResponsibilityProvider({ children, init, middleware }: ProviderProps) { if (!Array.isArray(middleware) || middleware.some(middleware => typeof middleware !== 'function')) { @@ -340,11 +332,15 @@ function createChainOfResponsibility< return {children}; } - function ChainOfResponsibilityProxy({ fallbackComponent, request, ...props }: ProxyProps) { + const ChainOfResponsibilityProxy = memo(function ChainOfResponsibilityProxy({ + fallbackComponent, + request, + ...props + }: ProxyProps): ReactNode { const result = useBuildRenderCallback()(request, { fallbackComponent })?.(props as Props); return result ? {result} : null; - } + }); const MemoizedChainOfResponsibilityProvider = memo>(ChainOfResponsibilityProvider); @@ -352,7 +348,7 @@ function createChainOfResponsibility< return Object.freeze({ Provider: MemoizedChainOfResponsibilityProvider as typeof MemoizedChainOfResponsibilityProvider & InferenceHelper, - Proxy: memo>(ChainOfResponsibilityProxy), + Proxy: ChainOfResponsibilityProxy, reactComponent, useBuildRenderCallback From 8efdd8d876c5baf037a378645a4e0a15b2dc91f1 Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 13:30:33 -0700 Subject: [PATCH 03/13] Add tests --- .../stability.changeFallbackComponent.tsx | 93 +++++++++++++++++++ .../tests/stability.changeMiddleware.test.tsx | 51 ++++++---- 2 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx diff --git a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx new file mode 100644 index 0000000..3cbc431 --- /dev/null +++ b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx @@ -0,0 +1,93 @@ +/** @jest-environment jsdom */ +/// + +import { scenario } from '@testduet/given-when-then'; +import { render } from '@testing-library/react'; +import React, { Fragment, memo, type ComponentType } from 'react'; + +import createChainOfResponsibility, { type InferMiddleware } from '../createChainOfResponsibilityAsRenderCallback'; + +type Props = { readonly children?: never }; +type Request = number; + +scenario('stability test with changing fallbackComponent', bdd => { + bdd + .given('a TestComponent using chain of responsiblity', () => { + const { Provider, useBuildRenderCallback } = createChainOfResponsibility(); + + const alohaCall = jest.fn(); + + const Aloha = function MyComponent() { + alohaCall(); + + return Aloha!; + }; + + const helloWorldCall = jest.fn(); + + const HelloWorld = function MyComponent() { + helloWorldCall(); + + return Hello, World!; + }; + + const middleware: readonly InferMiddleware[] = []; + + function MyProxy({ fallbackComponent }: { readonly fallbackComponent: ComponentType }) { + // We are using `useBuildRenderCallback` for less memoization. + const result = useBuildRenderCallback()(1, { fallbackComponent })?.({}); + + return result ? {result} : null; + } + + return { + Aloha, + alohaCall, + HelloWorld, + helloWorldCall, + TestComponent: function TestComponent({ + fallbackComponent + }: { + readonly fallbackComponent: ComponentType; + }) { + return ( + + + + ); + } + }; + }) + + // --- + + .when('the component is rendered with fallback component of ', ({ HelloWorld, TestComponent }) => + render() + ) + .then('textContent should match', (_, { container }) => + expect(container).toHaveProperty('textContent', 'Hello, World!') + ) + .and(' should have been rendered once', ({ helloWorldCall }) => + expect(helloWorldCall).toHaveBeenCalledTimes(1) + ) + + // --- + + .when('the component is rendered again with same fallback component', ({ HelloWorld, TestComponent }) => + render() + ) + .then('textContent should match', (_, { container }) => + expect(container).toHaveProperty('textContent', 'Hello, World!') + ) + .and(' should not have been rendered again', ({ helloWorldCall }) => + expect(helloWorldCall).toHaveBeenCalledTimes(1) + ) + + // --- + + .when('the component is rendered with fallback component of ', ({ Aloha, TestComponent }) => + render() + ) + .then('textContent should match', (_, { container }) => expect(container).toHaveProperty('textContent', 'Aloha!')) + .and(' should have been rendered once', ({ alohaCall }) => expect(alohaCall).toHaveBeenCalledTimes(1)); +}); diff --git a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx index d1a2da7..752fe97 100644 --- a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx +++ b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx @@ -3,7 +3,7 @@ import { scenario } from '@testduet/given-when-then'; import { render } from '@testing-library/react'; -import React, { Fragment, memo } from 'react'; +import React, { Fragment } from 'react'; import createChainOfResponsibility, { type InferMiddleware } from '../createChainOfResponsibilityAsRenderCallback'; @@ -17,34 +17,39 @@ scenario('stability test with changing middleware', bdd => { const alohaCall = jest.fn(); - // Middleware prop change always re-render. - // Memoizing to focus on whether the component is rendering as waste. - const Aloha = memo(function Aloha() { + const Aloha = function Aloha() { alohaCall(); return Aloha!; - }); + }; + + // Changing middleware always call `reactComponent(...)` again and would result in wasted rendering. + // Until we do equality check on the return value of `reactComponent(...)` to see if they are different instances but same result. + // Otherwise, we need to cache the `reactComponent(Aloha)` for testing purpose. + // This way of testing it is preferred over `memo(Aloha)`. + // In production, web devs should do `memo(Aloha)` instead. + const renderAloha = reactComponent(Aloha); const helloWorldCall = jest.fn(); - // Middleware prop change always re-render. - // Memoizing to focus on whether the component is rendering as waste. - const HelloWorld = memo(function HelloWorld() { + const HelloWorld = function HelloWorld() { helloWorldCall(); return Hello, World!; - }); + }; + + const renderHelloWorld = reactComponent(HelloWorld); const myProxyCall = jest.fn(); - const MyProxy = memo(function MyProxy({ request }: { readonly request: number }) { + const MyProxy = function MyProxy({ request }: { readonly request: number }) { myProxyCall(); // We are using `useBuildRenderCallback` for less memoization. const result = useBuildRenderCallback()(request)?.({}); return result ? {result} : null; - }); + }; return Object.freeze({ Aloha, @@ -52,7 +57,8 @@ scenario('stability test with changing middleware', bdd => { HelloWorld, helloWorldCall, myProxyCall, - reactComponent, + renderAloha, + renderHelloWorld, TestComponent: function TestComponent({ middleware }: { @@ -66,8 +72,11 @@ scenario('stability test with changing middleware', bdd => { } }); }) - .when('the component is rendered', ({ HelloWorld, reactComponent, TestComponent }) => - render( () => () => reactComponent(HelloWorld)]} />) + + // --- + + .when('the component is rendered', ({ renderHelloWorld, TestComponent }) => + render( () => () => renderHelloWorld]} />) ) .then('textContent should match', (_, { container }) => expect(container).toHaveProperty('textContent', 'Hello, World!') @@ -76,11 +85,14 @@ scenario('stability test with changing middleware', bdd => { expect(helloWorldCall).toHaveBeenCalledTimes(1) ) .and(' should have been rendered once', ({ myProxyCall }) => expect(myProxyCall).toHaveBeenCalledTimes(1)) + + // --- + .when( 'the component is re-rendered with a different instance but same middleware', - ({ HelloWorld, reactComponent, TestComponent }, result) => { + ({ renderHelloWorld, TestComponent }, result) => { // Still rendering , but in a different middleware. - result.rerender( () => () => reactComponent(HelloWorld)]} />); + result.rerender( () => () => renderHelloWorld]} />); return result; } @@ -94,10 +106,13 @@ scenario('stability test with changing middleware', bdd => { // Every time middleware change, it should invalidate `useBuildRenderCallback`. // In future, we may do more granular check to see if enhancer actually changed or not and reduce wasted rendering. .and(' should have been rendered once', ({ myProxyCall }) => expect(myProxyCall).toHaveBeenCalledTimes(2)) + + // --- + .when( 'the component is re-rendered with a different instance but same middleware', - ({ Aloha, reactComponent, TestComponent }, result) => { - result.rerender( () => () => reactComponent(Aloha)]} />); + ({ renderAloha, TestComponent }, result) => { + result.rerender( () => () => renderAloha]} />); return result; } From 3741bb83df30c4b295e822c1ba6953b4fd2ed790 Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 14:03:24 -0700 Subject: [PATCH 04/13] Simplify --- .../createChainOfResponsibilityAsRenderCallback.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx index 548e5b0..b73de32 100644 --- a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx +++ b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx @@ -235,12 +235,8 @@ function createChainOfResponsibility< return; } - // Convert fallback component as renderer. - return createComponentHandlerResult(() => ( - // Currently, there are no ways to set `bindProps` to `fallbackComponent`. - // `fallbackComponent` do not need `overridingProps` because it is the last one in the chain, it would not have the next() function. - - )); + // `fallbackComponent` do not need `overridingProps` because it is the last one in the chain, it would not have the next() function. + return reactComponent(fallbackComponent); })(request); return ( From 231b41521d5ebc92db94090720da19b74b90fb17 Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 14:56:16 -0700 Subject: [PATCH 05/13] Add test for side effect --- ...eChainOfResponsibilityAsRenderCallback.tsx | 20 ++-- .../src/preview/tests/sideEffect.test.tsx | 99 +++++++++++++++++++ .../tests/stability.changeMiddleware.test.tsx | 39 ++++---- .../tests/stability.changeProps.test.tsx | 25 +++-- 4 files changed, 147 insertions(+), 36 deletions(-) create mode 100644 packages/react-chain-of-responsibility/src/preview/tests/sideEffect.test.tsx diff --git a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx index b73de32..71d43e1 100644 --- a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx +++ b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx @@ -178,6 +178,7 @@ function createChainOfResponsibility< function reactComponent

( component: ComponentType

, + // For `bindProps` of type function, do not do side-effect in it, it may not be always called in all scenarios. bindProps?: | (Partial & Omit) | ((props: Props) => Partial & Omit) @@ -256,15 +257,14 @@ function createChainOfResponsibility< readonly render: () => ReactNode; }; - const BuildRenderCallback = memo( - function BuildRenderCallback({ originalProps, render }: BuildRenderCallbackProps) { - const context = useMemo>(() => Object.freeze({ originalProps }), [originalProps]); + // Do not memoize . + // `bindProps` may have side effect and we want to be re-rendered to capture the side-effect. + // To prevent wasted render, web devs should memoize it themselves. + function BuildRenderCallback({ originalProps, render }: BuildRenderCallbackProps) { + const context = useMemo>(() => Object.freeze({ originalProps }), [originalProps]); - return {render()}; - }, - (prevProps, nextProps) => - arePropsEqual(prevProps.originalProps, nextProps.originalProps) && Object.is(prevProps.render, nextProps.render) - ); + return {render()}; + } function ChainOfResponsibilityProvider({ children, init, middleware }: ProviderProps) { if (!Array.isArray(middleware) || middleware.some(middleware => typeof middleware !== 'function')) { @@ -328,7 +328,7 @@ function createChainOfResponsibility< return {children}; } - const ChainOfResponsibilityProxy = memo(function ChainOfResponsibilityProxy({ + const ChainOfResponsibilityProxy = function ChainOfResponsibilityProxy({ fallbackComponent, request, ...props @@ -336,7 +336,7 @@ function createChainOfResponsibility< const result = useBuildRenderCallback()(request, { fallbackComponent })?.(props as Props); return result ? {result} : null; - }); + }; const MemoizedChainOfResponsibilityProvider = memo>(ChainOfResponsibilityProvider); diff --git a/packages/react-chain-of-responsibility/src/preview/tests/sideEffect.test.tsx b/packages/react-chain-of-responsibility/src/preview/tests/sideEffect.test.tsx new file mode 100644 index 0000000..f2b649e --- /dev/null +++ b/packages/react-chain-of-responsibility/src/preview/tests/sideEffect.test.tsx @@ -0,0 +1,99 @@ +/** @jest-environment jsdom */ +/// + +import { scenario } from '@testduet/given-when-then'; +import { render } from '@testing-library/react'; +import React, { Fragment } from 'react'; + +import createChainOfResponsibility, { type InferMiddleware } from '../createChainOfResponsibilityAsRenderCallback'; + +type Props = { readonly children?: never }; +type Request = void; + +scenario('side effect in bindProps', bdd => { + bdd + .given('a TestComponent using chain of responsiblity', () => { + const { Provider, Proxy, reactComponent, useBuildRenderCallback } = createChainOfResponsibility(); + + let count = 0; + const renderCall = jest.fn(); + + const MyComponent = function MyComponent({ value }: Props & { readonly value: number }) { + renderCall(); + + return Hello, World! ({value}); + }; + + const middleware: readonly InferMiddleware[] = [ + // `bindProps` has side effect. + () => () => () => reactComponent(MyComponent, { value: ++count }) + ]; + + return { + middleware, + MyComponent, + Provider, + Proxy, + renderCall, + useBuildRenderCallback + }; + }) + .and.oneOf([ + [ + 'rendered using useBuildRenderCallback()', + ({ middleware, Provider, renderCall, useBuildRenderCallback }) => { + function MyProxy() { + const result = useBuildRenderCallback()()?.({}); + + return result ? {result} : null; + } + + return { + renderCall, + TestComponent: function TestComponent() { + return ( + + + + ); + } + }; + } + ], + [ + 'rendered using ', + ({ middleware, Provider, Proxy, renderCall }) => { + return { + renderCall, + TestComponent: function TestComponent() { + return ( + + + + ); + } + }; + } + ] + ]) + + // --- + + .when('the component is rendered', ({ TestComponent }) => render()) + .then('textContent should match', (_, { container }) => + expect(container).toHaveProperty('textContent', 'Hello, World! (1)') + ) + .and('should have rendered once', ({ renderCall }) => expect(renderCall).toHaveBeenCalledTimes(1)) + + // --- + + .when('the component is rendered again', ({ TestComponent }, result) => { + result.rerender(); + + return result; + }) + .then('textContent should match', (_, { container }) => + expect(container).toHaveProperty('textContent', 'Hello, World! (2)') + ) + .and('should have rendered twice', ({ renderCall }) => expect(renderCall).toHaveBeenCalledTimes(2)); +}); diff --git a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx index 752fe97..adb9da4 100644 --- a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx +++ b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeMiddleware.test.tsx @@ -15,30 +15,30 @@ scenario('stability test with changing middleware', bdd => { .given('a TestComponent using chain of responsiblity', () => { const { Provider, reactComponent, useBuildRenderCallback } = createChainOfResponsibility(); + let count = 0; const alohaCall = jest.fn(); - const Aloha = function Aloha() { + const Aloha = function Aloha({ value }: { value: number }) { alohaCall(); - return Aloha!; + return Aloha! ({value}); }; - // Changing middleware always call `reactComponent(...)` again and would result in wasted rendering. - // Until we do equality check on the return value of `reactComponent(...)` to see if they are different instances but same result. - // Otherwise, we need to cache the `reactComponent(Aloha)` for testing purpose. - // This way of testing it is preferred over `memo(Aloha)`. - // In production, web devs should do `memo(Aloha)` instead. - const renderAloha = reactComponent(Aloha); + // Changing middleware should always call `reactComponent(...)` again. + // This is to make sure bindProps function with side-effect should be properly re-rendered. + const renderAloha = reactComponent(Aloha, () => ({ value: ++count })); const helloWorldCall = jest.fn(); - const HelloWorld = function HelloWorld() { + const HelloWorld = function HelloWorld({ value }: { value: number }) { helloWorldCall(); - return Hello, World!; + return Hello, World! ({value}); }; - const renderHelloWorld = reactComponent(HelloWorld); + const renderHelloWorld = reactComponent(HelloWorld, () => ({ + value: ++count + })); const myProxyCall = jest.fn(); @@ -79,7 +79,7 @@ scenario('stability test with changing middleware', bdd => { render( () => () => renderHelloWorld]} />) ) .then('textContent should match', (_, { container }) => - expect(container).toHaveProperty('textContent', 'Hello, World!') + expect(container).toHaveProperty('textContent', 'Hello, World! (1)') ) .and(' should have been rendered once', ({ helloWorldCall }) => expect(helloWorldCall).toHaveBeenCalledTimes(1) @@ -98,10 +98,11 @@ scenario('stability test with changing middleware', bdd => { } ) .then('textContent should match', (_, { container }) => - expect(container).toHaveProperty('textContent', 'Hello, World!') + expect(container).toHaveProperty('textContent', 'Hello, World! (2)') ) - .and(' should not have been re-rendered', ({ helloWorldCall }) => - expect(helloWorldCall).toHaveBeenCalledTimes(1) + .and(' should have been re-rendered', ({ helloWorldCall }) => + // Should be called again to make sure side effect in bindProps is captured. + expect(helloWorldCall).toHaveBeenCalledTimes(2) ) // Every time middleware change, it should invalidate `useBuildRenderCallback`. // In future, we may do more granular check to see if enhancer actually changed or not and reduce wasted rendering. @@ -117,10 +118,12 @@ scenario('stability test with changing middleware', bdd => { return result; } ) - .then('textContent should match', (_, { container }) => expect(container).toHaveProperty('textContent', 'Aloha!')) + .then('textContent should match', (_, { container }) => + expect(container).toHaveProperty('textContent', 'Aloha! (3)') + ) .and(' should have been rendered once', ({ alohaCall }) => expect(alohaCall).toHaveBeenCalledTimes(1)) - .and(' should have been rendered once', ({ helloWorldCall }) => - expect(helloWorldCall).toHaveBeenCalledTimes(1) + .and(' should not have been rendered', ({ helloWorldCall }) => + expect(helloWorldCall).toHaveBeenCalledTimes(2) ) .and(' should have been rendered 3 times', ({ myProxyCall }) => expect(myProxyCall).toHaveBeenCalledTimes(3) diff --git a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeProps.test.tsx b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeProps.test.tsx index fd2f180..7ec6f4b 100644 --- a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeProps.test.tsx +++ b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeProps.test.tsx @@ -15,17 +15,19 @@ scenario('stability test with changing props', bdd => { .given('a TestComponent using chain of responsiblity', () => { const { Provider, reactComponent, useBuildRenderCallback } = createChainOfResponsibility(); + let count = 0; const renderCall = jest.fn(); - const MyComponent = memo(function MyComponent(_: Props & { readonly dummy: string }) { + const MyComponent = memo(function MyComponent({ value }: Props & { readonly value: number }) { renderCall(); - return Hello, World!; + return Hello, World! ({value}); }); const middleware: readonly InferMiddleware[] = [ - // With a changing props, the should be re-rendered. - () => () => () => reactComponent(MyComponent, () => ({ dummy: crypto.randomUUID() })) + // Bind props change every time it is called. + // With a changing props, the should always be re-rendered. + () => () => () => reactComponent(MyComponent, () => ({ value: ++count })) ]; function MyProxy({ request }: { readonly request: number }) { @@ -46,19 +48,26 @@ scenario('stability test with changing props', bdd => { renderCall ] as const; }) + + // --- + .when('the component is rendered', ([TestComponent]) => render()) .then('textContent should match', (_, { container }) => - expect(container).toHaveProperty('textContent', 'Hello, World!') + expect(container).toHaveProperty('textContent', 'Hello, World! (1)') ) .and('should have been rendered once', ([_, renderCall]) => expect(renderCall).toHaveBeenCalledTimes(1)) - .when('the component is re-rendered with a different request', ([TestComponent], result) => { - // With same request, it should still re-render because props changed. + + // --- + + .when('the component is re-rendered with same request', ([TestComponent], result) => { + // With same request, it should still call enhancer because props could be changed. result.rerender(); return result; }) .then('textContent should match', (_, { container }) => - expect(container).toHaveProperty('textContent', 'Hello, World!') + expect(container).toHaveProperty('textContent', 'Hello, World! (2)') ) + // Because bindProps is a side-effect (change every time it is called), thus, the component must be rendered again. .and('should have been rendered twice', ([_, renderCall]) => expect(renderCall).toHaveBeenCalledTimes(2)); }); From 36f01d0ed46b598af156b352f2f126889ef354ac Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 15:07:29 -0700 Subject: [PATCH 06/13] Add entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5182d68..3993623 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- (Preview) 💢 Changed signature to return wrapped return value, instead of plain `ComponentType`, by [@compulim](https://github.com/compulim) in PR [#91](https://github.com/compulim/react-chain-of-responsibility/pull/91), [#92](https://github.com/compulim/react-chain-of-responsibility/pull/92), and [#99](https://github.com/compulim/react-chain-of-responsibility/pull/99) +- (Preview) 💢 Changed signature to return wrapped return value, instead of plain `ComponentType`, by [@compulim](https://github.com/compulim) in PR [#91](https://github.com/compulim/react-chain-of-responsibility/pull/91), [#92](https://github.com/compulim/react-chain-of-responsibility/pull/92), [#99](https://github.com/compulim/react-chain-of-responsibility/pull/99), and [#100](https://github.com/compulim/react-chain-of-responsibility/pull/100) - Use `handler-chain` package, by [@compulim](https://github.com/compulim) in PR [#93](https://github.com/compulim/react-chain-of-responsibility/pull/93) - Bumped dependencies, in PR [#97](https://github.com/compulim/react-chain-of-responsibility/pull/97) - Development dependencies From 0f448e8cacdb6f875b75b5c67520a1e3feccf321 Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 15:11:53 -0700 Subject: [PATCH 07/13] Fix typing for react@16 --- .../createChainOfResponsibilityAsRenderCallback.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx index 71d43e1..ba6bf71 100644 --- a/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx +++ b/packages/react-chain-of-responsibility/src/preview/createChainOfResponsibilityAsRenderCallback.tsx @@ -328,15 +328,11 @@ function createChainOfResponsibility< return {children}; } - const ChainOfResponsibilityProxy = function ChainOfResponsibilityProxy({ - fallbackComponent, - request, - ...props - }: ProxyProps): ReactNode { + function ChainOfResponsibilityProxy({ fallbackComponent, request, ...props }: ProxyProps) { const result = useBuildRenderCallback()(request, { fallbackComponent })?.(props as Props); return result ? {result} : null; - }; + } const MemoizedChainOfResponsibilityProvider = memo>(ChainOfResponsibilityProvider); From aedd643425603fd225253de39f57632fabdabcf3 Mon Sep 17 00:00:00 2001 From: William Wong Date: Sat, 26 Jul 2025 15:14:04 -0700 Subject: [PATCH 08/13] Fix ESLint --- .../src/preview/tests/stability.changeFallbackComponent.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx index 3cbc431..9d6fef4 100644 --- a/packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx +++ b/packages/react-chain-of-responsibility/src/preview/tests/stability.changeFallbackComponent.tsx @@ -3,9 +3,9 @@ import { scenario } from '@testduet/given-when-then'; import { render } from '@testing-library/react'; -import React, { Fragment, memo, type ComponentType } from 'react'; +import React, { Fragment, type ComponentType } from 'react'; -import createChainOfResponsibility, { type InferMiddleware } from '../createChainOfResponsibilityAsRenderCallback'; +import createChainOfResponsibility, { type InferMiddleware } from '../createChainOfResponsibilityAsRenderCallback.tsx'; type Props = { readonly children?: never }; type Request = number; From dd6ff47a1063ea1dd4d721b2fe9bbf652350f8db Mon Sep 17 00:00:00 2001 From: William Wong Date: Sun, 3 Aug 2025 12:34:51 -0700 Subject: [PATCH 09/13] Add missing deps --- package-lock.json | 12 ++++++++++++ packages/integration-test/package.json | 3 +++ packages/react-chain-of-responsibility/package.json | 11 +++++++++++ 3 files changed, 26 insertions(+) diff --git a/package-lock.json b/package-lock.json index eae23d0..2899fe4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12791,6 +12791,8 @@ "@types/jest": "^30.0.0", "@types/node": "^24.0.15", "@types/react": "^18.3.23", + "@types/react-dom": "^18.3.7", + "@types/react-test-renderer": "^18.3.1", "esbuild": "^0.25.8", "escape-string-regexp": "^4.0.0", "jest": "^30.0.4", @@ -12819,6 +12821,16 @@ "@testduet/given-when-then": "^0.1.0-main.334801c" } }, + "packages/react-chain-of-responsibility/node_modules/@types/react-test-renderer": { + "version": "18.3.1", + "resolved": "https://registry.npmjs.org/@types/react-test-renderer/-/react-test-renderer-18.3.1.tgz", + "integrity": "sha512-vAhnk0tG2eGa37lkU9+s5SoroCsRI08xnsWFiAXOuPH2jqzMbcXvKExXViPi1P5fIklDeCvXqyrdmipFaSkZrA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/react": "^18" + } + }, "packages/react-chain-of-responsibility/node_modules/escape-string-regexp": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz", diff --git a/packages/integration-test/package.json b/packages/integration-test/package.json index 20f53ef..045a8a8 100644 --- a/packages/integration-test/package.json +++ b/packages/integration-test/package.json @@ -51,6 +51,9 @@ "@types/react-dom": [ "18" ], + "@types/react-test-renderer": [ + "18" + ], "react": [ "18" ], diff --git a/packages/react-chain-of-responsibility/package.json b/packages/react-chain-of-responsibility/package.json index e0f2f89..ee86332 100644 --- a/packages/react-chain-of-responsibility/package.json +++ b/packages/react-chain-of-responsibility/package.json @@ -74,6 +74,7 @@ "@testing-library/react": "^12", "@types/react": "^16", "@types/react-dom": "^16", + "@types/react-test-renderer": "^16", "react": "16.8.0", "react-dom": "16.8.0", "react-test-renderer": "16.8.0" @@ -84,6 +85,7 @@ "@testing-library/react": "^12", "@types/react": "^17", "@types/react-dom": "^17", + "@types/react-test-renderer": "^17", "react": "17.0.0", "react-dom": "17.0.0", "react-test-renderer": "17.0.0" @@ -93,6 +95,7 @@ "devDependencies": { "@types/react": "^18", "@types/react-dom": "^18", + "@types/react-test-renderer": "^18", "react": "18.0.0", "react-dom": "18.0.0", "react-test-renderer": "18.0.0" @@ -108,6 +111,12 @@ "@types/react": [ "18" ], + "@types/react-dom": [ + "18" + ], + "@types/react-test-renderer": [ + "18" + ], "escape-string-regexp": [ "4" ], @@ -135,6 +144,8 @@ "@types/jest": "^30.0.0", "@types/node": "^24.0.15", "@types/react": "^18.3.23", + "@types/react-dom": "^18.3.7", + "@types/react-test-renderer": "^18.3.1", "esbuild": "^0.25.8", "escape-string-regexp": "^4.0.0", "jest": "^30.0.4", From ba7c6c37a355fa629486ae6f8ce91f7742587f09 Mon Sep 17 00:00:00 2001 From: William Wong Date: Sun, 3 Aug 2025 12:38:41 -0700 Subject: [PATCH 10/13] Add missing react-dom --- package-lock.json | 24 +++++++++++++----------- packages/integration-test/package.json | 4 +++- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2899fe4..5cae869 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4407,16 +4407,6 @@ "@types/react": "^18.0.0" } }, - "node_modules/@types/react-test-renderer": { - "version": "19.1.0", - "resolved": "https://registry.npmjs.org/@types/react-test-renderer/-/react-test-renderer-19.1.0.tgz", - "integrity": "sha512-XD0WZrHqjNrxA/MaR9O22w/RNidWR9YZmBdRGI7wcnWGrv/3dA8wKCJ8m63Sn+tLJhcjmuhOi629N66W6kgWzQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/react": "*" - } - }, "node_modules/@types/stack-utils": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz", @@ -12738,14 +12728,26 @@ "@tsconfig/strictest": "^2.0.5", "@types/jest": "^30.0.0", "@types/react": "^18.3.23", - "@types/react-test-renderer": "^19.1.0", + "@types/react-dom": "^18.3.7", + "@types/react-test-renderer": "^18.3.1", "jest": "^30.0.4", "jest-environment-jsdom": "^30.0.4", "react": "^18.3.1", + "react-dom": "^18.3.1", "react-test-renderer": "^18.3.1", "typescript": "^5.8.3" } }, + "packages/integration-test/node_modules/@types/react-test-renderer": { + "version": "18.3.1", + "resolved": "https://registry.npmjs.org/@types/react-test-renderer/-/react-test-renderer-18.3.1.tgz", + "integrity": "sha512-vAhnk0tG2eGa37lkU9+s5SoroCsRI08xnsWFiAXOuPH2jqzMbcXvKExXViPi1P5fIklDeCvXqyrdmipFaSkZrA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/react": "^18" + } + }, "packages/pages": { "name": "react-chain-of-responsibility-pages", "version": "0.0.0-0", diff --git a/packages/integration-test/package.json b/packages/integration-test/package.json index 045a8a8..3fe0291 100644 --- a/packages/integration-test/package.json +++ b/packages/integration-test/package.json @@ -71,10 +71,12 @@ "@tsconfig/strictest": "^2.0.5", "@types/jest": "^30.0.0", "@types/react": "^18.3.23", - "@types/react-test-renderer": "^19.1.0", + "@types/react-dom": "^18.3.7", + "@types/react-test-renderer": "^18.3.1", "jest": "^30.0.4", "jest-environment-jsdom": "^30.0.4", "react": "^18.3.1", + "react-dom": "^18.3.1", "react-test-renderer": "^18.3.1", "typescript": "^5.8.3" }, From 4e9a3e89aa1aded50869c1f6a30ce07ccc38723f Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 5 Aug 2025 02:27:45 -0700 Subject: [PATCH 11/13] Add entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3993623..a5d8253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- (Preview) 💢 Changed signature to return wrapped return value, instead of plain `ComponentType`, by [@compulim](https://github.com/compulim) in PR [#91](https://github.com/compulim/react-chain-of-responsibility/pull/91), [#92](https://github.com/compulim/react-chain-of-responsibility/pull/92), [#99](https://github.com/compulim/react-chain-of-responsibility/pull/99), and [#100](https://github.com/compulim/react-chain-of-responsibility/pull/100) +- (Preview) 💢 Changed signature to return wrapped return value, instead of plain `ComponentType`, by [@compulim](https://github.com/compulim) in PR [#91](https://github.com/compulim/react-chain-of-responsibility/pull/91), [#92](https://github.com/compulim/react-chain-of-responsibility/pull/92), [#99](https://github.com/compulim/react-chain-of-responsibility/pull/99), [#100](https://github.com/compulim/react-chain-of-responsibility/pull/100), and [#101](https://github.com/compulim/react-chain-of-responsibility/pull/101) - Use `handler-chain` package, by [@compulim](https://github.com/compulim) in PR [#93](https://github.com/compulim/react-chain-of-responsibility/pull/93) - Bumped dependencies, in PR [#97](https://github.com/compulim/react-chain-of-responsibility/pull/97) - Development dependencies From ecfe3fe2d103614b166bdb6e529d641125a0cb5b Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 5 Aug 2025 03:23:27 -0700 Subject: [PATCH 12/13] Set minimum to react@16.8.6 --- packages/integration-test/package.json | 4 ++-- packages/pages/package.json | 4 ++-- packages/react-chain-of-responsibility/package.json | 9 +++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/integration-test/package.json b/packages/integration-test/package.json index 3fe0291..7d0cd82 100644 --- a/packages/integration-test/package.json +++ b/packages/integration-test/package.json @@ -23,8 +23,8 @@ "switch:react-16": { "devDependencies": { "@types/react": "^16", - "react": "16.8.0", - "react-test-renderer": "16.8.0" + "react": "16.8.6", + "react-test-renderer": "16.8.6" } }, "switch:react-17": { diff --git a/packages/pages/package.json b/packages/pages/package.json index 3355176..35dc0fd 100644 --- a/packages/pages/package.json +++ b/packages/pages/package.json @@ -26,8 +26,8 @@ "@types/react-dom": "^16" }, "dependencies": { - "react": "16.8.0", - "react-dom": "16.8.0" + "react": "16.8.6", + "react-dom": "16.8.6" } }, "switch:react-17": { diff --git a/packages/react-chain-of-responsibility/package.json b/packages/react-chain-of-responsibility/package.json index ee86332..407c0e7 100644 --- a/packages/react-chain-of-responsibility/package.json +++ b/packages/react-chain-of-responsibility/package.json @@ -75,9 +75,9 @@ "@types/react": "^16", "@types/react-dom": "^16", "@types/react-test-renderer": "^16", - "react": "16.8.0", - "react-dom": "16.8.0", - "react-test-renderer": "16.8.0" + "react": "16.8.6", + "react-dom": "16.8.6", + "react-test-renderer": "16.8.6" } }, "switch:react-17": { @@ -93,6 +93,7 @@ }, "switch:react-18": { "devDependencies": { + "@testing-library/react": "^16", "@types/react": "^18", "@types/react-dom": "^18", "@types/react-test-renderer": "^18", @@ -161,7 +162,7 @@ "typescript": "^5.8.3" }, "peerDependencies": { - "react": ">=16.8.0" + "react": ">=16.8.6" }, "dependencies": { "handler-chain": "^0.1.0", From c145810ba9fef28c810ab2e0dc9e8ed31d375968 Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 5 Aug 2025 03:25:22 -0700 Subject: [PATCH 13/13] Add react-dom to switch --- packages/integration-test/package.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/integration-test/package.json b/packages/integration-test/package.json index 7d0cd82..a874c18 100644 --- a/packages/integration-test/package.json +++ b/packages/integration-test/package.json @@ -23,21 +23,27 @@ "switch:react-16": { "devDependencies": { "@types/react": "^16", + "@types/react-dom": "^16", "react": "16.8.6", + "react-dom": "16.8.6", "react-test-renderer": "16.8.6" } }, "switch:react-17": { "devDependencies": { "@types/react": "^17", + "@types/react-dom": "^17", "react": "17.0.0", + "react-dom": "17.0.0", "react-test-renderer": "17.0.0" } }, "switch:react-18": { "devDependencies": { "@types/react": "^18", + "@types/react-dom": "^18", "react": "18.0.0", + "react-dom": "18.0.0", "react-test-renderer": "18.0.0" } },