fix: lazy load design library #3632
Conversation
WalkthroughRemoves a FavoriteButton component and a public re-export, introduces lazy loading for the Design Library modal, adds a small DesignLibraryContext module, consolidates many imports, adjusts import paths, and adds error handling and export-shape changes for the lazy design-library module. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User UI
participant Entry as Modal Entry
participant Loader as React.lazy loader
participant Susp as Suspense (fallback)
participant Chunk as Lazy Chunk (~stackable/lazy-components/design-library)
participant Modal as ModalDesignLibrary
participant List as DesignLibraryList
participant API as getDesigns / design API
User->>Entry: open design library
Entry->>Loader: trigger dynamic import
Loader-->>Susp: loading...
Susp-->>User: show fallback (null)
Loader->>Chunk: load chunk
Chunk->>Modal: mount ModalDesignLibrary
Modal->>List: request designs (getDesigns)
List->>API: fetch designs
API-->>List: designs or { error }
alt designs
List->>Modal: render items
else error
List->>Modal: propagate errors prop -> show error block
end
Modal-->>User: render UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lazy-components/design-library/design-library-list/use-shadow-root.js (1)
44-48: Fix potential TypeError when editorHead is undefined.editorHead can be falsy; calling querySelector on it will throw. Guard with optional chaining.
- const editorHead = editorBody?.ownerDocument?.head - node = editorHead.querySelector( `#${ id }` ) + const editorHead = editorBody?.ownerDocument?.head + node = editorHead?.querySelector( `#${ id }` )src/lazy-components/design-library/design-library-list/design-library-list-item.js (2)
149-168: Click on “Insert” also triggers the parent button (double toggle).Wrapper is a button with onClick; inner Insert Button bubbles up, causing two onClickDesign calls. Stop propagation in the inner handler (minimal fix).
- <Button + <Button label={ __( 'Insert', i18n ) } className={ `ugb-modal-design-library__add-multi ${ selected ? 'stk--is-selected' : '' }` } disabled={ isMultiSelectBusy } - onClick={ () => { + onClick={ (e) => { + e.preventDefault() + e.stopPropagation() setSelected( true ) onClickDesign() } }Additionally consider moving the selection onClick from the outer button to a non-interactive wrapper to avoid nested interactive controls.
Also applies to: 95-103
142-146: Possible crash: blocksForSubstitutionRef may be undefined before init.Accessing .size on undefined throws. Guard the ref value.
- { blocksForSubstitutionRef.current !== false && blocksForSubstitutionRef.current.size !== 0 && + { blocksForSubstitutionRef.current instanceof Map && blocksForSubstitutionRef.current.size !== 0 &&
🧹 Nitpick comments (7)
src/lazy-components/design-library/design-library-list/use-shadow-root.js (1)
40-43: Defensive guard for non-browser environments.If this runs in tests/SSR where document is undefined, the checks will crash. Cheap guard avoids surprises without changing behavior in Gutenberg.
- if ( document && document.head ) { + if ( typeof document !== 'undefined' && document.head ) {- if ( ! existingIds.has( 'global-styles-inline-css' ) ) { + if ( typeof document !== 'undefined' && ! existingIds.has( 'global-styles-inline-css' ) ) {Also applies to: 58-63
src/lazy-components/design-library/design-library-list/index.js (1)
162-167: IntersectionObserver option ‘scrollMargin’ is not a valid option.Only root, rootMargin, and threshold are supported. Remove scrollMargin to avoid confusion.
}, { root: rootEl, rootMargin: '500px', - scrollMargin: '500px', threshold: 0, } )src/lazy-components/design-library/design-library-list/design-library-list-item.js (1)
95-104: Nested button inside button breaks accessibility.A button wrapping other buttons is invalid HTML and harms keyboard/screen reader UX. Replace the outer button with a div (role="button", tabIndex=0) or move the click handler to a non-interactive container.
- <button + <div role="button" tabIndex={0} className={ mainClasses } ref={ ref } onClick={ onClickHost } onMouseOut={ onMouseOut } onMouseOver={ onMouseOver } - > + > ... - </button> + </div>Also applies to: 154-173
src/components/modal-design-library/index.js (1)
21-24: Lazy-loading integration LGTM; add a visible fallback for UX.Null fallback causes a blank area while loading. Show a small Spinner.
-import { - useMemo, useCallback, Suspense, lazy, -} from '@wordpress/element' +import { useMemo, useCallback, Suspense, lazy } from '@wordpress/element' +import { Spinner } from '@wordpress/components' ... - <Suspense fallback={ null }> + <Suspense fallback={<div style={{padding: 16, textAlign: 'center'}}><Spinner /></div>}>Also applies to: 32-39
src/lazy-components/design-library/index.js (3)
84-99: Avoid race conditions in getDesigns effect (late responses overwriting newer state).Use an “active” flag to ignore stale responses when selectedTab/doReset change rapidly.
useEffect( () => { - setIsBusy( true ) - setSidebarDesigns( [] ) + let active = true + setIsBusy( true ) + setSidebarDesigns( [] ) ... - } ).then( designs => { - setSidebarDesigns( designs ) - setSelectedCategory( '' ) - } ).finally( () => { - setDoReset( false ) - setIsBusy( false ) - } ) - }, [ doReset, selectedTab ] ) + } ).then( designs => { + if (!active) return + setSidebarDesigns( designs ) + setSelectedCategory( '' ) + } ).finally( () => { + if (!active) return + setDoReset( false ) + setIsBusy( false ) + } ) + return () => { active = false } + }, [ doReset, selectedTab ] )
101-111: Mirror the stale-response guard in filterDesigns effect.useEffect( () => { - filterDesigns( { + let active = true + filterDesigns( { library: sidebarDesigns, category: selectedCategory, plan: selectedPlan.key, } ).then( designs => { - setDisplayDesigns( designs ) + if (!active) return + setDisplayDesigns( designs ) } ) - }, [ sidebarDesigns, selectedPlan, selectedCategory ] ) + return () => { active = false } + }, [ sidebarDesigns, selectedPlan, selectedCategory ] )
231-239: Use a safe target and rel on external docs link.target="_docs" is nonstandard. Use _blank and add rel for security.
- <a href="https://docs.wpstackable.com/article/343-using-the-design-library#Design-Library-Style-Options-Pswi5" target="_docs"> + <a href="https://docs.wpstackable.com/article/343-using-the-design-library#Design-Library-Style-Options-Pswi5" target="_blank" rel="noopener noreferrer">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
src/components/design-library-list/images/heart-fill.svgis excluded by!**/*.svgsrc/components/design-library-list/images/heart.svgis excluded by!**/*.svgsrc/lazy-components/design-library/images/help.svgis excluded by!**/*.svgsrc/lazy-components/design-library/images/view-few.svgis excluded by!**/*.svgsrc/lazy-components/design-library/images/view-many.svgis excluded by!**/*.svgsrc/lazy-components/design-library/images/view-single.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
src/components/design-library-list/favorite-button.js(0 hunks)src/components/index.js(0 hunks)src/components/modal-design-library/index.js(2 hunks)src/lazy-components/design-library/context.js(1 hunks)src/lazy-components/design-library/design-library-list/design-library-list-item.js(1 hunks)src/lazy-components/design-library/design-library-list/index.js(1 hunks)src/lazy-components/design-library/design-library-list/use-preview-renderer.js(1 hunks)src/lazy-components/design-library/design-library-list/use-shadow-root.js(1 hunks)src/lazy-components/design-library/header-actions.js(1 hunks)src/lazy-components/design-library/index.js(4 hunks)src/lazy-components/design-library/util.js(1 hunks)src/lazy-components/style-guide/utils.js(1 hunks)
💤 Files with no reviewable changes (2)
- src/components/index.js
- src/components/design-library-list/favorite-button.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/lazy-components/design-library/design-library-list/use-preview-renderer.js
🧬 Code graph analysis (1)
src/lazy-components/design-library/index.js (5)
src/lazy-components/design-library/design-library-list/design-library-list-item.js (1)
props(29-34)src/lazy-components/design-library/design-library-list/index.js (2)
props(24-28)props(75-75)src/lazy-components/design-library/design-library-list/use-preview-renderer.js (1)
props(41-53)src/lazy-components/design-library/header-actions.js (1)
props(23-30)src/lazy-components/design-library/block-list.js (1)
props(25-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (7)
src/lazy-components/design-library/header-actions.js (1)
7-7: Consolidated component imports look good.src/lazy-components/design-library/context.js (1)
1-7: Context extraction is clean and minimal.src/lazy-components/design-library/index.js (1)
416-417: Default export switch looks good.ModalDesignLibrary is now default-exported; aligns with lazy import usage.
src/components/modal-design-library/index.js (1)
23-27: Confirm module path alias for the lazy import.Ensure '~stackable/lazy-components/design-library' resolves in your bundler config and that the chunk is produced as “design-library”.
src/lazy-components/style-guide/utils.js (1)
23-23: Verify the import path change is complete across the codebase.The import path has been updated to reflect the reorganized file structure. Please ensure that no other files still reference the old path
components/design-library-list/util.Run the following script to verify the file structure and check for any remaining references to the old import path:
src/lazy-components/design-library/util.js (1)
2-2: Verify the default.json file exists at the new location.The import path has been updated to reference
default.jsonfrom thedesign-library-listsubdirectory. Ensure the file exists at the expected location and contains the necessary data.Run the following script to verify the file location and structure:
src/lazy-components/design-library/design-library-list/use-preview-renderer.js (1)
6-6: Verify the reorganized file structure and import paths.The import paths have been updated to reflect the file's new location in the
design-library-listsubdirectory. The changes correctly adjust relative paths to access parent-level resources (../images/maps.webpand../util).Run the following script to verify the file structure and import resolution:
Also applies to: 14-14
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/design-library/index.js(2 hunks)src/lazy-components/design-library/design-library-list/index.js(3 hunks)src/lazy-components/design-library/index.js(7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/lazy-components/design-library/index.jssrc/lazy-components/design-library/design-library-list/index.js
🧬 Code graph analysis (3)
src/lazy-components/design-library/index.js (2)
src/lazy-components/design-library/design-library-list/index.js (3)
props(24-29)props(82-82)selectedTab(86-94)src/design-library/index.js (3)
getDesigns(60-79)getDesigns(60-79)designs(7-7)
src/design-library/index.js (1)
src/welcome/admin.js (1)
error(314-314)
src/lazy-components/design-library/design-library-list/index.js (1)
src/lazy-components/design-library/index.js (1)
errors(69-69)
🔇 Additional comments (6)
src/lazy-components/design-library/design-library-list/index.js (1)
5-5: LGTM: Import reorganization aligns with lazy-loading refactor.The context import has been properly relocated to a dedicated module, and the
usePresetControlsimport is now correctly placed at the file level.Also applies to: 11-11
src/design-library/index.js (2)
32-32: LGTM: Defensive operators prevent undefined access errors.The nullish coalescing operators (
??) and optional chaining (?.) provide safe fallbacks when API responses are missing expected data.Also applies to: 34-34, 38-38
66-71: LGTM: Error handling structure is sound.The error detection and logging are appropriate. The function correctly short-circuits further processing when errors occur and returns an object with an error property.
src/lazy-components/design-library/index.js (3)
6-8: LGTM: Import consolidation supports lazy-loading architecture.The reorganized imports properly separate concerns, with the context extracted to a dedicated module and component dependencies centralized to enable code-splitting.
Also applies to: 16-20
96-101: LGTM: Error extraction logic is correct.The code properly detects error responses, separates the error value for display, and ensures
sidebarDesignsreceives an empty array when errors occur.
48-48: LGTM: Default export pattern supports lazy-loading.Changing from a named export to a default export aligns with React lazy-loading patterns and enables the modal to be dynamically imported as a separate chunk.
Also applies to: 427-427
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lazy-components/design-library/index.js (1)
92-109: Add a catch handler to capture rejected promises and unexpected errors.The promise chain lacks a
.catch()handler. IfgetDesignsthrows an exception or the promise rejects for any reason, the error won't be caught, leaving users with an empty list and no indication of what went wrong.Apply this diff to add comprehensive error handling:
getDesigns( { reset: doReset, type: selectedTab, } ).then( designs => { let _designs = designs if ( typeof designs === 'object' && designs.error ) { _designs = [] setErrors( designs.error ) } setSidebarDesigns( _designs ) setSelectedCategory( '' ) + } ).catch( error => { + console.error( 'Stackable: Failed to fetch designs', error ) + setErrors( error?.message || 'An unexpected error occurred while loading designs.' ) + setSidebarDesigns( [] ) + setSelectedCategory( '' ) } ).finally( () => { setDoReset( false ) setIsBusy( false ) } )
🧹 Nitpick comments (1)
src/lazy-components/design-library/index.js (1)
96-101: Consider making the error detection check more explicit.While the current check works (arrays won't have an
errorproperty), explicitly excluding arrays makes the intent clearer and the code more maintainable.Apply this diff for improved clarity:
let _designs = designs - if ( typeof designs === 'object' && designs.error ) { + if ( designs && typeof designs === 'object' && !Array.isArray( designs ) && designs.error ) { _designs = [] setErrors( designs.error ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lazy-components/design-library/design-library-list/use-preview-renderer.js(1 hunks)src/lazy-components/design-library/index.js(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/lazy-components/design-library/design-library-list/use-preview-renderer.jssrc/lazy-components/design-library/index.js
🧬 Code graph analysis (1)
src/lazy-components/design-library/index.js (3)
src/lazy-components/design-library/design-library-list/index.js (3)
props(24-29)props(82-82)selectedTab(86-94)src/lazy-components/design-library/header-actions.js (1)
props(23-30)src/design-library/index.js (3)
getDesigns(60-79)getDesigns(60-79)designs(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: build
🔇 Additional comments (2)
src/lazy-components/design-library/design-library-list/use-preview-renderer.js (1)
6-6: LGTM! Import paths correctly adjusted for lazy-loading structure.The relative import paths have been updated to reflect the new directory structure under
lazy-components. These changes support the PR objective of lazy-loading the design library module.Also applies to: 14-14
src/lazy-components/design-library/index.js (1)
69-69: Error state initialization correctly usesnull.The error state is properly initialized as
nulland reset consistently, aligning with the type of error values returned fromgetDesigns. This addresses the concern from the previous review.Also applies to: 90-90
fixes #3597
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Breaking Change