From 8f844e2eefd8285929e425856f1ff9342ec97fcc Mon Sep 17 00:00:00 2001 From: Rushikesh More Date: Sun, 31 May 2026 13:01:10 +0530 Subject: [PATCH 1/3] Components: Refactor withFallbackStyles from class to function component Part of #22890 (convert class components to function components with hooks). Map componentDidMount + componentDidUpdate to a single useIsomorphicLayoutEffect with no dependency array, preserving the original synchronous-before-paint timing so the grabbed fallback styles still apply without a flash. The instance nodeRef becomes a useRef and the ref callback a stable useCallback. Pure refactor, no behavioral change. Add a regression test (the component previously had none) covering both the ref-wrapper path and the node-prop path. --- packages/components/CHANGELOG.md | 4 + .../with-fallback-styles/index.tsx | 93 ++++++++----------- .../with-fallback-styles/test/index.tsx | 43 +++++++++ 3 files changed, 84 insertions(+), 56 deletions(-) create mode 100644 packages/components/src/higher-order/with-fallback-styles/test/index.tsx diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index e12081c7fe54eb..0a02fcda9a06f4 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Internal + +- `withFallbackStyles`: Refactor from a class component to a function component with hooks ([#78837](https://github.com/WordPress/gutenberg/pull/78837)). + ### Documentation - Fix documentation typos and grammar ([#78686](https://github.com/WordPress/gutenberg/pull/78686)). diff --git a/packages/components/src/higher-order/with-fallback-styles/index.tsx b/packages/components/src/higher-order/with-fallback-styles/index.tsx index 618b86b422a60f..ad4a98aa12c0e7 100644 --- a/packages/components/src/higher-order/with-fallback-styles/index.tsx +++ b/packages/components/src/higher-order/with-fallback-styles/index.tsx @@ -6,19 +6,17 @@ import fastDeepEqual from 'fast-deep-equal/es6/index.js'; /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; -import { createHigherOrderComponent } from '@wordpress/compose'; +import { useState, useRef, useCallback } from '@wordpress/element'; +import { + createHigherOrderComponent, + useIsomorphicLayoutEffect, +} from '@wordpress/compose'; type Props = { node?: HTMLElement; [ key: string ]: any; }; -type State = { - fallbackStyles?: { [ key: string ]: any }; - grabStylesCompleted: boolean; -}; - export default ( mapNodeToProps: ( node: HTMLElement, @@ -26,69 +24,52 @@ export default ( ) => { [ key: string ]: any } ) => createHigherOrderComponent( ( WrappedComponent ) => { - return class WithFallbackStyles extends Component< Props, State > { - nodeRef?: HTMLElement; - - constructor( props: Props ) { - super( props ); - this.nodeRef = this.props.node; - this.state = { - fallbackStyles: undefined, - grabStylesCompleted: false, - }; + return function WithFallbackStyles( props: Props ) { + const [ fallbackStyles, setFallbackStyles ] = useState< + { [ key: string ]: any } | undefined + >( undefined ); + const [ grabStylesCompleted, setGrabStylesCompleted ] = + useState( false ); - this.bindRef = this.bindRef.bind( this ); - } + const nodeRef = useRef< HTMLElement | undefined >( props.node ); - bindRef( node: HTMLDivElement ) { + const bindRef = useCallback( ( node: HTMLDivElement | null ) => { if ( ! node ) { return; } - this.nodeRef = node; - } + nodeRef.current = node; + }, [] ); - componentDidMount() { - this.grabFallbackStyles(); - } - - componentDidUpdate() { - this.grabFallbackStyles(); - } - - grabFallbackStyles() { - const { grabStylesCompleted, fallbackStyles } = this.state; - if ( this.nodeRef && ! grabStylesCompleted ) { + // The original class grabbed the fallback styles synchronously in + // componentDidMount and componentDidUpdate (i.e. after every render, + // before paint). useIsomorphicLayoutEffect with no dependency array + // preserves that timing and stays SSR-safe. + useIsomorphicLayoutEffect( () => { + if ( nodeRef.current && ! grabStylesCompleted ) { const newFallbackStyles = mapNodeToProps( - this.nodeRef, - this.props + nodeRef.current, + props ); if ( ! fastDeepEqual( newFallbackStyles, fallbackStyles ) ) { - this.setState( { - fallbackStyles: newFallbackStyles, - grabStylesCompleted: - Object.values( newFallbackStyles ).every( - Boolean - ), - } ); + setFallbackStyles( newFallbackStyles ); + setGrabStylesCompleted( + Object.values( newFallbackStyles ).every( Boolean ) + ); } } - } + } ); + + const wrappedComponent = ( + + ); - render() { - const wrappedComponent = ( - - ); - return this.props.node ? ( - wrappedComponent - ) : ( -
{ wrappedComponent }
- ); - } + return props.node ? ( + wrappedComponent + ) : ( +
{ wrappedComponent }
+ ); }; }, 'withFallbackStyles' ); diff --git a/packages/components/src/higher-order/with-fallback-styles/test/index.tsx b/packages/components/src/higher-order/with-fallback-styles/test/index.tsx new file mode 100644 index 00000000000000..61765cb6f8f199 --- /dev/null +++ b/packages/components/src/higher-order/with-fallback-styles/test/index.tsx @@ -0,0 +1,43 @@ +/** + * External dependencies + */ +import { render, screen } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import withFallbackStyles from '..'; + +describe( 'withFallbackStyles', () => { + const Wrapped = ( props: { fallbackTextColor?: string } ) => ( +
{ props.fallbackTextColor ?? 'no-fallback' }
+ ); + + it( 'passes the mapped fallback styles down to the wrapped component', () => { + const mapNodeToProps = jest.fn( () => ( { + fallbackTextColor: 'rgb(1, 2, 3)', + } ) ); + const Component = withFallbackStyles( mapNodeToProps )( Wrapped ); + + render( ); + + expect( mapNodeToProps ).toHaveBeenCalled(); + expect( screen.getByText( 'rgb(1, 2, 3)' ) ).toBeInTheDocument(); + } ); + + it( 'uses the provided node prop instead of the internal ref wrapper', () => { + const node = document.createElement( 'div' ); + const mapNodeToProps = jest.fn( () => ( { + fallbackTextColor: 'rgb(4, 5, 6)', + } ) ); + const Component = withFallbackStyles( mapNodeToProps )( Wrapped ); + + render( ); + + expect( mapNodeToProps ).toHaveBeenCalledWith( + node, + expect.objectContaining( { node } ) + ); + expect( screen.getByText( 'rgb(4, 5, 6)' ) ).toBeInTheDocument(); + } ); +} ); From abf270f77a47669f6f2c84ff1b7f81ef150e0f22 Mon Sep 17 00:00:00 2001 From: Rushikesh More Date: Wed, 3 Jun 2026 13:11:52 +0530 Subject: [PATCH 2/3] Components: Simplify withFallbackStyles per review - Drop the bindRef callback and pass nodeRef directly to the wrapper div. The effect reads props.node ?? nodeRef.current, which keeps the div ref typed as HTMLDivElement while still honoring an externally supplied node. - Derive grabStylesCompleted from fallbackStyles instead of holding it in a second state, so the effect performs a single setState. - Make the unit tests read from the actual DOM node (dataset / tagName) rather than a constant, so they exercise the node wiring. - Trim the inline comment. --- .../with-fallback-styles/index.tsx | 36 +++++++----------- .../with-fallback-styles/test/index.tsx | 38 ++++++++++--------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/packages/components/src/higher-order/with-fallback-styles/index.tsx b/packages/components/src/higher-order/with-fallback-styles/index.tsx index ad4a98aa12c0e7..7605eefc75fe1f 100644 --- a/packages/components/src/higher-order/with-fallback-styles/index.tsx +++ b/packages/components/src/higher-order/with-fallback-styles/index.tsx @@ -6,7 +6,7 @@ import fastDeepEqual from 'fast-deep-equal/es6/index.js'; /** * WordPress dependencies */ -import { useState, useRef, useCallback } from '@wordpress/element'; +import { useState, useRef } from '@wordpress/element'; import { createHigherOrderComponent, useIsomorphicLayoutEffect, @@ -28,36 +28,26 @@ export default ( const [ fallbackStyles, setFallbackStyles ] = useState< { [ key: string ]: any } | undefined >( undefined ); - const [ grabStylesCompleted, setGrabStylesCompleted ] = - useState( false ); - const nodeRef = useRef< HTMLElement | undefined >( props.node ); + // Derived from fallbackStyles: the grab is complete once every + // mapped value has resolved to a truthy style. + const grabStylesCompleted = + !! fallbackStyles && + Object.values( fallbackStyles ).every( Boolean ); - const bindRef = useCallback( ( node: HTMLDivElement | null ) => { - if ( ! node ) { - return; - } - nodeRef.current = node; - }, [] ); + const nodeRef = useRef< HTMLDivElement | null >( null ); - // The original class grabbed the fallback styles synchronously in - // componentDidMount and componentDidUpdate (i.e. after every render, - // before paint). useIsomorphicLayoutEffect with no dependency array - // preserves that timing and stays SSR-safe. + // Runs before paint, matching the original componentDidMount / + // componentDidUpdate, so the fallback styles apply without a flash. useIsomorphicLayoutEffect( () => { - if ( nodeRef.current && ! grabStylesCompleted ) { - const newFallbackStyles = mapNodeToProps( - nodeRef.current, - props - ); + const node = props.node ?? nodeRef.current; + if ( node && ! grabStylesCompleted ) { + const newFallbackStyles = mapNodeToProps( node, props ); if ( ! fastDeepEqual( newFallbackStyles, fallbackStyles ) ) { setFallbackStyles( newFallbackStyles ); - setGrabStylesCompleted( - Object.values( newFallbackStyles ).every( Boolean ) - ); } } } ); @@ -69,7 +59,7 @@ export default ( return props.node ? ( wrappedComponent ) : ( -
{ wrappedComponent }
+
{ wrappedComponent }
); }; }, 'withFallbackStyles' ); diff --git a/packages/components/src/higher-order/with-fallback-styles/test/index.tsx b/packages/components/src/higher-order/with-fallback-styles/test/index.tsx index 61765cb6f8f199..e40858822166aa 100644 --- a/packages/components/src/higher-order/with-fallback-styles/test/index.tsx +++ b/packages/components/src/higher-order/with-fallback-styles/test/index.tsx @@ -9,35 +9,39 @@ import { render, screen } from '@testing-library/react'; import withFallbackStyles from '..'; describe( 'withFallbackStyles', () => { - const Wrapped = ( props: { fallbackTextColor?: string } ) => ( -
{ props.fallbackTextColor ?? 'no-fallback' }
+ const Wrapped = ( props: { fallbackColor?: string } ) => ( +
{ props.fallbackColor ?? 'no-fallback' }
); - it( 'passes the mapped fallback styles down to the wrapped component', () => { - const mapNodeToProps = jest.fn( () => ( { - fallbackTextColor: 'rgb(1, 2, 3)', + it( 'derives the fallback styles from the provided node prop', () => { + const node = document.createElement( 'span' ); + node.dataset.color = 'rgb(1, 2, 3)'; + const mapNodeToProps = jest.fn( ( el: HTMLElement ) => ( { + fallbackColor: el.dataset.color, } ) ); const Component = withFallbackStyles( mapNodeToProps )( Wrapped ); - render( ); + render( ); - expect( mapNodeToProps ).toHaveBeenCalled(); + // The rendered value proves the actual node was read by mapNodeToProps. + expect( mapNodeToProps ).toHaveBeenCalledWith( + node, + expect.objectContaining( { node } ) + ); expect( screen.getByText( 'rgb(1, 2, 3)' ) ).toBeInTheDocument(); } ); - it( 'uses the provided node prop instead of the internal ref wrapper', () => { - const node = document.createElement( 'div' ); - const mapNodeToProps = jest.fn( () => ( { - fallbackTextColor: 'rgb(4, 5, 6)', + it( 'derives the fallback styles from the internal wrapper node when no node prop is given', () => { + const mapNodeToProps = jest.fn( ( el: HTMLElement ) => ( { + fallbackColor: el.tagName.toLowerCase(), } ) ); const Component = withFallbackStyles( mapNodeToProps )( Wrapped ); - render( ); + render( ); - expect( mapNodeToProps ).toHaveBeenCalledWith( - node, - expect.objectContaining( { node } ) - ); - expect( screen.getByText( 'rgb(4, 5, 6)' ) ).toBeInTheDocument(); + // The HOC wraps in a
, so reading the node's tagName yields 'div'. + expect( mapNodeToProps ).toHaveBeenCalled(); + expect( mapNodeToProps.mock.calls[ 0 ][ 0 ].tagName ).toBe( 'DIV' ); + expect( screen.getByText( 'div' ) ).toBeInTheDocument(); } ); } ); From 14da66126628716a62b1100ca4a09610daaea9d3 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Tue, 23 Jun 2026 19:56:03 +0400 Subject: [PATCH 3/3] Latest feedback --- .../higher-order/with-fallback-styles/index.tsx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/components/src/higher-order/with-fallback-styles/index.tsx b/packages/components/src/higher-order/with-fallback-styles/index.tsx index 7605eefc75fe1f..441f26ff8a7c5f 100644 --- a/packages/components/src/higher-order/with-fallback-styles/index.tsx +++ b/packages/components/src/higher-order/with-fallback-styles/index.tsx @@ -29,18 +29,17 @@ export default ( { [ key: string ]: any } | undefined >( undefined ); - // Derived from fallbackStyles: the grab is complete once every - // mapped value has resolved to a truthy style. - const grabStylesCompleted = - !! fallbackStyles && - Object.values( fallbackStyles ).every( Boolean ); + const nodeRef = useRef< HTMLDivElement >( null ); - const nodeRef = useRef< HTMLDivElement | null >( null ); - - // Runs before paint, matching the original componentDidMount / - // componentDidUpdate, so the fallback styles apply without a flash. + // Runs before paint, so the fallback styles apply without a flash. useIsomorphicLayoutEffect( () => { const node = props.node ?? nodeRef.current; + // Derived from fallbackStyles: the grab is complete once every + // mapped value has resolved to a truthy style. + const grabStylesCompleted = + !! fallbackStyles && + Object.values( fallbackStyles ).every( Boolean ); + if ( node && ! grabStylesCompleted ) { const newFallbackStyles = mapNodeToProps( node, props );