Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PRESS 2 498 | Audit memoization candidates #157

Merged
merged 26 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b5267af
Refactored Skeleton to be a React.memo component
officiallygod Jan 12, 2023
9ff7324
Added React Memo in Skeletons
officiallygod Jan 12, 2023
b9029a4
Revert "Added React Memo in Skeletons"
officiallygod Jan 13, 2023
848e2c9
Removed Log
officiallygod Jan 13, 2023
32b66a8
Merge branch 'trunk' into PRESS-2-498-Audit-Memoization-Candidates
officiallygod Jan 18, 2023
0582614
Memoized Global Styles Output
officiallygod Jan 20, 2023
a2a8a17
Merge branch 'trunk' into PRESS-2-498-Audit-Memoization-Candidates
officiallygod Jan 20, 2023
ec0012b
Added Memoization to Basic Info
officiallygod Jan 23, 2023
5fb81a4
Added memo to Card Header
officiallygod Jan 23, 2023
b83d5cb
Reduced API Calls for Global Styles
officiallygod Jan 24, 2023
4176df0
Merge branch 'trunk' into PRESS-2-498-Audit-Memoization-Candidates
officiallygod Feb 1, 2023
7fb0072
Lint for JS
officiallygod Feb 1, 2023
cbe694e
Header.js
officiallygod Feb 2, 2023
ff045e9
Refactored Drawer
officiallygod Feb 2, 2023
2c081c9
Drawer.js
officiallygod Feb 2, 2023
14b7615
Sidebar.js
officiallygod Feb 2, 2023
3607ff7
Refactored Experience to not use UseEffect
officiallygod Feb 2, 2023
9ce7a52
Removed lodash memoize
officiallygod Feb 6, 2023
5a4655c
Delete build/1.0.4 directory
officiallygod Feb 7, 2023
7a0a900
Revert "Delete build/1.0.4 directory"
officiallygod Feb 8, 2023
da76771
Added Build Folder
officiallygod Feb 8, 2023
eb6c504
Merge branch 'trunk' into PRESS-2-498-Audit-Memoization-Candidates
officiallygod Feb 8, 2023
3b4aa37
Merge branch 'trunk' into PRESS-2-498-Audit-Memoization-Candidates
officiallygod Feb 8, 2023
8cd9f88
Update use-global-styles-output.js
officiallygod Feb 9, 2023
20e6d04
Lint Changes
officiallygod Feb 9, 2023
d11b336
Update use-global-styles-output.js
officiallygod Feb 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/OnboardingSPA/components/CardHeader/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import React from 'react';
import { memo } from '@wordpress/element'

const CardHeader = ({ heading, subHeading, question }) => {

Expand All @@ -23,4 +24,4 @@ const CardHeader = ({ heading, subHeading, question }) => {
);
};

export default CardHeader;
export default memo(CardHeader);
22 changes: 10 additions & 12 deletions src/OnboardingSPA/components/Drawer/DrawerPanel/DesignColors.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,18 +455,16 @@ const DesignColors = () => {
}

return (
<GlobalStylesProvider>
<div className="theme-colors--drawer">
<h2>{ __( 'Color Palettes', 'wp-module-onboarding' ) }</h2>
{ /* {selectedColors?.slug &&
<div className='theme-colors--drawer--reset' onClick={resetColors}>
<div>Reset Button</div>
</div>
} */ }
{ colorPalettes && buildPalettes() }
{ colorPalettes && buildCustomPalette() }
</div>
</GlobalStylesProvider>
<div className="theme-colors--drawer">
<h2>{ __( 'Color Palettes', 'wp-module-onboarding' ) }</h2>
{ /* {selectedColors?.slug &&
<div className='theme-colors--drawer--reset' onClick={resetColors}>
<div>Reset Button</div>
</div>
} */ }
{ colorPalettes && buildPalettes() }
{ colorPalettes && buildCustomPalette() }
</div>
);
};

Expand Down
22 changes: 10 additions & 12 deletions src/OnboardingSPA/components/Drawer/DrawerPanel/DesignTypography.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,19 +202,17 @@ const DesignTypography = () => {
}

return (
<GlobalStylesProvider>
<div ref={ drawerFontOptions } className="theme-fonts--drawer">
<h2>{ __( 'Font Palettes', 'wp-module-onboarding' ) }</h2>
{ /* { selectedFont &&
<div className='theme-fonts--drawer--reset' onClick={resetFonts}>
<div>Reset Button</div>
</div>
} */ }
{ fontPalettes && buildPalettes() }
{ /* { fontPalettes && buildCustomPalette() } */ }
<div className="custom-font-palette--hidden">{ rerender }</div>
<div ref={ drawerFontOptions } className="theme-fonts--drawer">
<h2>{ __( 'Font Palettes', 'wp-module-onboarding' ) }</h2>
{ /* { selectedFont &&
<div className='theme-fonts--drawer--reset' onClick={resetFonts}>
<div>Reset Button</div>
</div>
</GlobalStylesProvider>
} */ }
{ fontPalettes && buildPalettes() }
{ /* { fontPalettes && buildCustomPalette() } */ }
<div className="custom-font-palette--hidden">{ rerender }</div>
</div>
);
};
export default DesignTypography;
4 changes: 2 additions & 2 deletions src/OnboardingSPA/components/ImageUploader/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { __ } from '@wordpress/i18n';
import { useRef, useState } from '@wordpress/element';
import { memo, useRef, useState } from '@wordpress/element';

import { ImageUploadLoader } from '../Loaders';
import { uploadImage } from '../../utils/api/uploader';
Expand Down Expand Up @@ -104,4 +104,4 @@ const ImageUploader = ({ icon, iconSetter }) => {
);
};

export default ImageUploader;
export default memo(ImageUploader);
18 changes: 11 additions & 7 deletions src/OnboardingSPA/components/LivePreview/BlockPreview/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useSelect } from '@wordpress/data';
import { BlockEditorProvider } from '@wordpress/block-editor';
import { parse } from '@wordpress/blocks';
import { useEffect, useState } from '@wordpress/element';
import { useEffect, useState, memo } from '@wordpress/element';

import AutoHeightBlockPreview from './auto';
import { useGlobalStylesOutput } from '../../../utils/global-styles/use-global-styles-output';
Expand Down Expand Up @@ -76,10 +76,9 @@ const BlockPreview = ( {
}
}, [ storedPreviewSettings ] );

return (
<div className={ `live-preview__container-${ styling }` }>
{ loading && (
<div className="live-preview__container--is-skeleton">
const SkeletonLivePreview = memo(() => {
return (
<div className="live-preview__container--is-skeleton">
Yashita101 marked this conversation as resolved.
Show resolved Hide resolved
<div className="live-preview__container--is-skeleton--box live-preview__container--is-skeleton--box-header">
<Animate
type={ 'shine' }
Expand All @@ -89,8 +88,13 @@ const BlockPreview = ( {
<div className="live-preview__container--is-skeleton--box live-preview__container--is-skeleton--box-body-1" />
<div className="live-preview__container--is-skeleton--box live-preview__container--is-skeleton--box-body-2" />
<div className="live-preview__container--is-skeleton--box live-preview__container--is-skeleton--box-footer" />
</div>
) }
</div>
);
});

return (
<div className={ `live-preview__container-${ styling }` }>
{ loading && <SkeletonLivePreview/> }
{ settings && (
<BlockEditorProvider
value={ blocks }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,22 @@ const GlobalStylesProvider = ( { children } ) => {
useDispatch( nfdOnboardingStore );

const getStylesAndPatterns = async () => {
const globalStyles = await getGlobalStyles();
if ( globalStyles?.error ) {
return updateThemeStatus( THEME_STATUS_NOT_ACTIVE );
}
let selectedGlobalStyle;
if ( storedPreviewSettings?.title && storedPreviewSettings?.settings )
selectedGlobalStyle = storedPreviewSettings;
else if ( currentData.data.theme.variation ) {
selectedGlobalStyle = globalStyles.body.filter(
( globalStyle ) =>
globalStyle.title === currentData.data.theme.variation
)[ 0 ];
} else if ( globalStyles.body[ 0 ]?.id === 0 ) {
selectedGlobalStyle = globalStyles.body[ 0 ];
else{
const globalStyles = await getGlobalStyles();
if (globalStyles?.error) {
return updateThemeStatus(THEME_STATUS_NOT_ACTIVE);
}
if ( currentData.data.theme.variation ) {
selectedGlobalStyle = globalStyles.body.filter(
( globalStyle ) =>
globalStyle.title === currentData.data.theme.variation
)[ 0 ];
} else if ( globalStyles.body[ 0 ]?.id === 0 ) {
selectedGlobalStyle = globalStyles.body[ 0 ];
}
}

if ( selectedGlobalStyle )
Expand Down
4 changes: 2 additions & 2 deletions src/OnboardingSPA/components/MiniPreview/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { __, sprintf } from '@wordpress/i18n';
import { useState, useEffect } from '@wordpress/element';
import { memo, useState, useEffect } from '@wordpress/element';

import content from './miniPreview.json';
import { translations } from '../../utils/locales/translations';
Expand Down Expand Up @@ -159,4 +159,4 @@ const MiniPreview = ({ title, desc, icon, socialData, isSocialFormOpen, setIsSoc
);
};

export default MiniPreview;
export default memo(MiniPreview);
68 changes: 42 additions & 26 deletions src/OnboardingSPA/utils/global-styles/use-global-styles-output.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { get, isEmpty, kebabCase, pickBy, reduce, set } from 'lodash';
import { get, isEmpty, kebabCase, pickBy, reduce, set, memoize } from 'lodash';
Copy link
Member

@arunshenoy99 arunshenoy99 Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for pointing this out a bit late, let's avoid adding more dependencies on lodash? It seems like Gutenberg has made a commitment to remove the lodash dependency from it's packages which we are keeping an eye on. I was going through some of the PR's there and found that https://www.npmjs.com/package/memize is being used as a replacement to the lodash memoize(attaching why in ref), can we explore this?
Ref:
Lint config of Gutenberg preventing lodash dependencies recommending memize: https://github.com/WordPress/gutenberg/blob/d9da1250c77d3dc083729b72b8143dfc7bc222ba/.eslintrc.js#L157
Why: WordPress/gutenberg#6686 (comment)


/**
* WordPress dependencies
Expand Down Expand Up @@ -811,38 +811,29 @@ export const getBlockSelectors = ( blockTypes ) => {
return result;
};

export function useGlobalStylesOutput(
previewSettings,
storedPreviewSettings
) {
const generateStylesheets = ( previewSettings, storedPreviewSettings ) => {

const hasBlockGapSupport =
storedPreviewSettings.settings.__experimentalFeatures.spacing.blockGap;
const hasFallbackGapSupport = ! hasBlockGapSupport;
const disableLayoutStyles = storedPreviewSettings.settings
storedPreviewSettings?.settings?.__experimentalFeatures.spacing.blockGap;
const hasFallbackGapSupport = !hasBlockGapSupport;
const disableLayoutStyles = storedPreviewSettings?.settings
?.disableLayoutStyles
? storedPreviewSettings.settings.disableLayoutStyles
? storedPreviewSettings?.settings?.disableLayoutStyles
: true;

if (
! previewSettings?.styles &&
! previewSettings?.settings &&
! previewSettings?.globalStyles
) {
return;
}

const requiredSettings = {
settings: previewSettings.settings,
settings: previewSettings?.settings,
styles: previewSettings?.globalStyles
? previewSettings.globalStyles
: previewSettings.styles,
? previewSettings?.globalStyles
: previewSettings?.styles,
};
const blockSelectors = getBlockSelectors( getBlockTypes() );

const blockSelectors = getBlockSelectors(getBlockTypes());
const customProperties = toCustomProperties(
requiredSettings,
blockSelectors
);

const globalStyles = toStyles(
requiredSettings,
blockSelectors,
Expand All @@ -851,15 +842,15 @@ export function useGlobalStylesOutput(
disableLayoutStyles
);

const result = storedPreviewSettings.settings.styles.filter( ( style ) => {
const result = storedPreviewSettings?.settings?.styles.filter((style) => {
if (
! (
style.hasOwnProperty( 'id' ) &&
( style.id === 'customProperty' || style.id === 'globalStyle' )
!(
style.hasOwnProperty('id') &&
(style.id === 'customProperty' || style.id === 'globalStyle')
)
)
return style;
} );
});

const stylesheets = [
...result,
Expand All @@ -875,6 +866,31 @@ export function useGlobalStylesOutput(
},
];

return stylesheets;
}

// This function serializes those objects into JSON strings to determine the cache key.
const resolver = (arg1, arg2) => JSON.stringify([arg1, arg2]);

// Make a Memoized function that runs when the input changes
const generateStylesheetsMemo = memoize(generateStylesheets, resolver);

export function useGlobalStylesOutput(
previewSettings,
storedPreviewSettings
) {

if (
!previewSettings &&
!previewSettings?.styles &&
!previewSettings?.settings &&
!previewSettings?.globalStyles
) {
return;
}

let stylesheets = generateStylesheetsMemo( previewSettings, storedPreviewSettings );

previewSettings.settings.styles = stylesheets;
previewSettings.settings.__unstableResolvedAssets =
storedPreviewSettings.settings.__unstableResolvedAssets;
Expand Down