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.

Sign up for GitHub

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

Block preview: build in async rendering #60425

Merged
merged 9 commits into from
Apr 4, 2024
Merged

Block preview: build in async rendering #60425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
21937e2
Block preview: build in async rendering
ellatrix Apr 3, 2024
c771118
Fix typo
ellatrix Apr 3, 2024
eb45d34
Remove remnant comment
ellatrix Apr 3, 2024
a43cebf
Seprate Async util component
ellatrix Apr 3, 2024
935dcc4
Add spinner
ellatrix Apr 3, 2024
91d3919
Try react transition
ellatrix Apr 4, 2024
ed71d14
Remove spinners
ellatrix Apr 4, 2024
014409c
Set state sync
ellatrix Apr 4, 2024
0ec0b87
Update package.json
ellatrix Apr 4, 2024
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
2 changes: 2 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions packages/dataviews/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,6 @@ Whether the data is loading. `false` by default.

Array of layouts supported. By default, all are: `table`, `grid`, `list`.

### `deferredRendering`: `boolean`

Whether the items should be rendered asynchronously. Useful when there's a field that takes a lot of time (e.g.: previews). `false` by default.

### `onSelectionChange`: `function`

Callback that signals the user selected one of more items, and takes them as parameter. So far, only the `list` view implements it.
Expand Down
2 changes: 0 additions & 2 deletions packages/dataviews/src/dataviews.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export default function DataViews( {
paginationInfo,
supportedLayouts,
onSelectionChange = defaultOnSelectionChange,
deferredRendering = false,
} ) {
const [ selection, setSelection ] = useState( [] );
const [ openedFilter, setOpenedFilter ] = useState( null );
Expand Down Expand Up @@ -136,7 +135,6 @@ export default function DataViews( {
isLoading={ isLoading }
onSelectionChange={ onSetSelection }
selection={ selection }
deferredRendering={ deferredRendering }
setOpenedFilter={ setOpenedFilter }
/>
<Pagination
Expand Down
8 changes: 2 additions & 6 deletions packages/dataviews/src/view-grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
FlexItem,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useAsyncList } from '@wordpress/compose';

/**
* Internal dependencies
Expand Down Expand Up @@ -151,7 +150,6 @@ export default function ViewGrid( {
actions,
isLoading,
getItemId,
deferredRendering,
selection,
onSelectionChange,
} ) {
Expand All @@ -168,9 +166,7 @@ export default function ViewGrid( {
field.id
)
);
const shownData = useAsyncList( data, { step: 3 } );
const usedData = deferredRendering ? shownData : data;
const hasData = !! usedData?.length;
const hasData = !! data?.length;
return (
<>
{ hasData && (
Expand All @@ -181,7 +177,7 @@ export default function ViewGrid( {
className="dataviews-view-grid"
aria-busy={ isLoading }
>
{ usedData.map( ( item ) => {
{ data.map( ( item ) => {
return (
<GridItem
key={ getItemId( item ) }
Expand Down
11 changes: 4 additions & 7 deletions packages/dataviews/src/view-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import classNames from 'classnames';
/**
* WordPress dependencies
*/
import { useAsyncList, useInstanceId } from '@wordpress/compose';
import { useInstanceId } from '@wordpress/compose';
import {
__experimentalHStack as HStack,
__experimentalVStack as VStack,
Expand Down Expand Up @@ -128,13 +128,10 @@ export default function ViewList( {
getItemId,
onSelectionChange,
selection,
deferredRendering,
id: preferredId,
} ) {
const baseId = useInstanceId( ViewList, 'view-list', preferredId );
const shownData = useAsyncList( data, { step: 3 } );
const usedData = deferredRendering ? shownData : data;
const selectedItem = usedData?.findLast( ( item ) =>
const selectedItem = data?.findLast( ( item ) =>
selection.includes( item.id )
);

Expand Down Expand Up @@ -166,7 +163,7 @@ export default function ViewList( {
defaultActiveId: getItemDomId( selectedItem ),
} );

const hasData = usedData?.length;
const hasData = data?.length;
if ( ! hasData ) {
return (
<div
Expand All @@ -190,7 +187,7 @@ export default function ViewList( {
role="grid"
store={ store }
>
{ usedData.map( ( item ) => {
{ data.map( ( item ) => {
const id = getItemDomId( item );
return (
<ListItem
Expand Down
8 changes: 2 additions & 6 deletions packages/dataviews/src/view-table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useAsyncList } from '@wordpress/compose';
import { unseen, funnel } from '@wordpress/icons';
import {
Button,
Expand Down Expand Up @@ -369,7 +368,6 @@ function ViewTable( {
data,
getItemId,
isLoading = false,
deferredRendering,
selection,
onSelectionChange,
setOpenedFilter,
Expand All @@ -386,7 +384,6 @@ function ViewTable( {
}
} );

const asyncData = useAsyncList( data );
const tableNoticeId = useId();

if ( nextHeaderMenuToFocus ) {
Expand All @@ -409,8 +406,7 @@ function ViewTable( {
! view.hiddenFields.includes( field.id ) &&
! [ view.layout.mediaField ].includes( field.id )
);
const usedData = deferredRendering ? asyncData : data;
const hasData = !! usedData?.length;
const hasData = !! data?.length;
const sortValues = { asc: 'ascending', desc: 'descending' };

const primaryField = fields.find(
Expand Down Expand Up @@ -502,7 +498,7 @@ function ViewTable( {
</thead>
<tbody>
{ hasData &&
usedData.map( ( item, index ) => (
data.map( ( item, index ) => (
<TableRow
key={ getItemId( item ) }
item={ item }
Expand Down
1 change: 1 addition & 0 deletions packages/edit-site/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"@wordpress/plugins": "file:../plugins",
"@wordpress/preferences": "file:../preferences",
"@wordpress/primitives": "file:../primitives",
"@wordpress/priority-queue": "file:../priority-queue",
"@wordpress/private-apis": "file:../private-apis",
"@wordpress/reusable-blocks": "file:../reusable-blocks",
"@wordpress/router": "file:../router",
Expand Down
43 changes: 43 additions & 0 deletions packages/edit-site/src/components/async/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* WordPress dependencies
*/
import { useEffect, useState, flushSync } from '@wordpress/element';
import { createQueue } from '@wordpress/priority-queue';

const blockPreviewQueue = createQueue();

/**
* Renders a component at the next idle time.
* @param {*} props
*/
export function Async( { children, placeholder } ) {
const [ shouldRender, setShouldRender ] = useState( false );

// In the future, we could try to use startTransition here, but currently
// react will batch all transitions, which means all previews will be
// rendered at the same time.
// https://react.dev/reference/react/startTransition#caveats
// > If there are multiple ongoing Transitions, React currently batches them
// > together. This is a limitation that will likely be removed in a future
// > release.

useEffect( () => {
const context = {};
blockPreviewQueue.add( context, () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

wild idea not sure if good.

What if we have an argument to the "add" function that says even if there's still time remaining, schedule an update on the next idle callback window.

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically the goal: to give more time for each template to render before moving to the next, even if the template itself has some small async stuff (like useEffects...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Right you mean only one call per idle callback? I wonder if we'd still need to do the time remaining check though, maybe the browser just calls those idle callbacks in sync? I haven't tested, so not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it could be a "config" option as well when creating the queue.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll suggest this in a follow up because it will touch async list as well and we'll need to do a new round of testing. But at first glance it look to me like it solves the same issue. I'll create a new PR after this and also loop in Jarda

// Synchronously run all renders so it consumes timeRemaining.
// See https://github.com/WordPress/gutenberg/pull/48238
flushSync( () => {
setShouldRender( true );
} );
} );
return () => {
blockPreviewQueue.cancel( context );
};
}, [] );

if ( ! shouldRender ) {
return placeholder;
}

return children;
}
12 changes: 7 additions & 5 deletions packages/edit-site/src/components/page-patterns/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { usePrevious } from '@wordpress/compose';
/**
* Internal dependencies
*/
import { Async } from '../async';
import Page from '../page';
import {
LAYOUT_GRID,
Expand Down Expand Up @@ -177,10 +178,12 @@ function Preview( { item, categoryId, viewType } ) {
{ isEmpty && isTemplatePart && __( 'Empty template part' ) }
{ isEmpty && ! isTemplatePart && __( 'Empty pattern' ) }
{ ! isEmpty && (
<BlockPreview
blocks={ item.blocks }
viewportWidth={ item.viewportWidth }
/>
<Async>
<BlockPreview
blocks={ item.blocks }
viewportWidth={ item.viewportWidth }
/>
</Async>
) }
</PreviewWrapper>
</div>
Expand Down Expand Up @@ -404,7 +407,6 @@ export default function DataviewsPatterns() {
isLoading={ isResolving }
view={ view }
onChangeView={ onChangeView }
deferredRendering
supportedLayouts={ [ LAYOUT_GRID, LAYOUT_TABLE ] }
/>
</Page>
Expand Down
15 changes: 9 additions & 6 deletions packages/edit-site/src/components/page-templates-template-parts/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { privateApis as editorPrivateApis } from '@wordpress/editor';
/**
* Internal dependencies
*/
import { Async } from '../async';
import Page from '../page';
import { default as Link, useLink } from '../routes/link';
import AddNewTemplate from '../add-new-template';
Expand Down Expand Up @@ -173,7 +174,9 @@ function Preview( { item, viewType } ) {
style={ { backgroundColor } }
>
{ viewType === LAYOUT_LIST && ! isEmpty && (
<BlockPreview blocks={ blocks } />
<Async>
<BlockPreview blocks={ blocks } />
</Async>
) }
{ viewType !== LAYOUT_LIST && (
<button
Expand All @@ -186,7 +189,11 @@ function Preview( { item, viewType } ) {
( item.type === TEMPLATE_POST_TYPE
? __( 'Empty template' )
: __( 'Empty template part' ) ) }
{ ! isEmpty && <BlockPreview blocks={ blocks } /> }
{ ! isEmpty && (
<Async>
<BlockPreview blocks={ blocks } />
</Async>
) }
</button>
) }
</div>
Expand Down Expand Up @@ -419,10 +426,6 @@ export default function PageTemplatesTemplateParts( { postType } ) {
view={ view }
onChangeView={ onChangeView }
onSelectionChange={ onSelectionChange }
deferredRendering={
view.type === LAYOUT_GRID ||
! view.hiddenFields?.includes( 'preview' )
}
/>
</Page>
);
Expand Down
Loading