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

Async mode in the data module #13056

Merged
merged 18 commits into from
Jan 15, 2019
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
6 changes: 6 additions & 0 deletions docs/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,12 @@
"markdown_source": "https://raw.githubusercontent.com/WordPress/gutenberg/master/packages/postcss-themes/README.md",
"parent": "packages"
},
{
"title": "@wordpress/priority-queue",
"slug": "packages-priority-queue",
"markdown_source": "https://raw.githubusercontent.com/WordPress/gutenberg/master/packages/priority-queue/README.md",
"parent": "packages"
},
{
"title": "@wordpress/redux-routine",
"slug": "packages-redux-routine",
Expand Down
2 changes: 2 additions & 0 deletions lib/packages-dependencies.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
'wp-compose',
'wp-element',
'wp-is-shallow-equal',
'wp-priority-queue',
'wp-redux-routine',
),
'wp-date' => array(
Expand Down Expand Up @@ -211,6 +212,7 @@
'wp-element',
'wp-hooks',
),
'wp-priority-queue' => array(),
'wp-redux-routine' => array(),
'wp-rich-text' => array(
'lodash',
Expand Down
7 changes: 7 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"@wordpress/notices": "file:packages/notices",
"@wordpress/nux": "file:packages/nux",
"@wordpress/plugins": "file:packages/plugins",
"@wordpress/priority-queue": "file:packages/priority-queue",
"@wordpress/redux-routine": "file:packages/redux-routine",
"@wordpress/rich-text": "file:packages/rich-text",
"@wordpress/shortcode": "file:packages/shortcode",
Expand Down
1 change: 1 addition & 0 deletions packages/data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"@wordpress/compose": "file:../compose",
"@wordpress/element": "file:../element",
"@wordpress/is-shallow-equal": "file:../is-shallow-equal",
"@wordpress/priority-queue": "file:../priority-queue",
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
"@wordpress/redux-routine": "file:../redux-routine",
"equivalent-key-map": "^0.2.2",
"is-promise": "^2.1.0",
Expand Down
10 changes: 10 additions & 0 deletions packages/data/src/components/async-mode-provider/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* WordPress dependencies
*/
import { createContext } from '@wordpress/element';

const { Consumer, Provider } = createContext( false );

export const AsyncModeConsumer = Consumer;

export default Provider;
42 changes: 32 additions & 10 deletions packages/data/src/components/with-select/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
import { Component } from '@wordpress/element';
import { isShallowEqualObjects } from '@wordpress/is-shallow-equal';
import { createHigherOrderComponent } from '@wordpress/compose';
import { createQueue } from '@wordpress/priority-queue';

/**
* Internal dependencies
*/
import { RegistryConsumer } from '../registry-provider';
import { AsyncModeConsumer } from '../async-mode-provider';

const renderQueue = createQueue();

/**
* Higher-order component used to inject state-derived props using registered
Expand Down Expand Up @@ -70,16 +74,23 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped
componentWillUnmount() {
this.canRunSelection = false;
this.unsubscribe();
renderQueue.flush( this );
}

shouldComponentUpdate( nextProps, nextState ) {
// Cycle subscription if registry changes.
const hasRegistryChanged = nextProps.registry !== this.props.registry;
const hasSyncRenderingChanged = nextProps.isAsync !== this.props.isAsync;

if ( hasRegistryChanged ) {
this.unsubscribe();
this.subscribe( nextProps.registry );
}

if ( hasSyncRenderingChanged ) {
renderQueue.flush( this );
}

// Treat a registry change as equivalent to `ownProps`, to reflect
// `mergeProps` to rendered component if and only if updated.
const hasPropsChanged = (
Expand All @@ -89,11 +100,11 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped

// Only render if props have changed or merge props have been updated
// from the store subscriber.
if ( this.state === nextState && ! hasPropsChanged ) {
if ( this.state === nextState && ! hasPropsChanged && ! hasSyncRenderingChanged ) {
return false;
}

if ( hasPropsChanged ) {
if ( hasPropsChanged || hasSyncRenderingChanged ) {
const nextMergeProps = getNextMergeProps( nextProps );
if ( ! isShallowEqualObjects( this.mergeProps, nextMergeProps ) ) {
// If merge props change as a result of the incoming props,
Expand Down Expand Up @@ -137,7 +148,13 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped
}

subscribe( registry ) {
this.unsubscribe = registry.subscribe( this.onStoreChange );
this.unsubscribe = registry.subscribe( () => {
aduth marked this conversation as resolved.
Show resolved Hide resolved
if ( this.props.isAsync ) {
renderQueue.add( this, this.onStoreChange );
} else {
this.onStoreChange();
}
} );
}

render() {
Expand All @@ -146,14 +163,19 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped
}

return ( ownProps ) => (
<RegistryConsumer>
{ ( registry ) => (
<ComponentWithSelect
ownProps={ ownProps }
registry={ registry }
/>
<AsyncModeConsumer>
{ ( isAsync ) => (
<RegistryConsumer>
{ ( registry ) => (
<ComponentWithSelect
ownProps={ ownProps }
registry={ registry }
isAsync={ isAsync }
/>
) }
</RegistryConsumer>
) }
</RegistryConsumer>
</AsyncModeConsumer>
);
}, 'withSelect' );

Expand Down
1 change: 1 addition & 0 deletions packages/data/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import * as plugins from './plugins';
export { default as withSelect } from './components/with-select';
export { default as withDispatch } from './components/with-dispatch';
export { default as RegistryProvider, RegistryConsumer } from './components/registry-provider';
export { default as __experimentalAsyncModeProvider } from './components/async-mode-provider';
export { createRegistry } from './registry';
export { plugins };

Expand Down
9 changes: 6 additions & 3 deletions packages/e2e-tests/specs/plugins/meta-attribute-block.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,12 @@ describe( 'Block with a meta attribute', () => {
await insertBlock( 'Test Meta Attribute Block' );
await page.keyboard.type( 'Meta Value' );

const persistedValues = await page.evaluate( () => Array.from( document.querySelectorAll( '.my-meta-input' ) ).map( ( input ) => input.value ) );
persistedValues.forEach( ( val ) => {
expect( val ).toBe( 'Meta Value' );
const inputs = await page.$$( '.my-meta-input' );
await inputs.forEach( async ( input ) => {
// Clicking the input selects the block,
// and selecting the block enables the sync data mode.
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
await input.click();
expect( await input.getProperty( 'value' ) ).toBe( 'Meta Value' );
} );
} );
} );
58 changes: 44 additions & 14 deletions packages/editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import {
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
import {
withSelect,
withDispatch,
__experimentalAsyncModeProvider as AsyncModeProvider,
} from '@wordpress/data';
import { compose } from '@wordpress/compose';

/**
Expand All @@ -24,6 +28,13 @@ import BlockListBlock from './block';
import BlockListAppender from '../block-list-appender';
import { getBlockDOMNode } from '../../utils/dom';

const forceSyncUpdates = ( WrappedComponent ) => ( props ) => {
return (
<AsyncModeProvider value={ false }>
<WrappedComponent { ...props } />
</AsyncModeProvider>
);
};
class BlockList extends Component {
constructor( props ) {
super( props );
Expand Down Expand Up @@ -182,37 +193,54 @@ class BlockList extends Component {
blockClientIds,
rootClientId,
isDraggable,
selectedBlockClientId,
multiSelectedBlockClientIds,
} = this.props;

return (
<div className="editor-block-list__layout">
{ map( blockClientIds, ( clientId, blockIndex ) => (
<BlockListBlock
key={ 'block-' + clientId }
index={ blockIndex }
clientId={ clientId }
blockRef={ this.setBlockRef }
onSelectionStart={ this.onSelectionStart }
rootClientId={ rootClientId }
isFirst={ blockIndex === 0 }
isLast={ blockIndex === blockClientIds.length - 1 }
isDraggable={ isDraggable }
/>
) ) }
{ map( blockClientIds, ( clientId, blockIndex ) => {
return (
<AsyncModeProvider
key={ 'block-' + clientId }
Copy link
Member

Choose a reason for hiding this comment

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

I know we had it previously, but I wonder why it matters to have the key be prefixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't

value={
selectedBlockClientId !== clientId &&
Copy link
Member

Choose a reason for hiding this comment

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

What is this value representing? For clarity and legibility, it'd be deserving of being assigned a variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It represents when we enable/disable the async mode, I can add a clarifying comments. The idea is that selected blocks are sync, the others are async.

Copy link
Member

Choose a reason for hiding this comment

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

Well, from how I interpret it, it represents whether the rendered block occurs within the current selection, something akin to a isBlockInSelection variable (or even selector, though I don't think this can help here as written).

( !! selectedBlockClientId || multiSelectedBlockClientIds.indexOf( clientId ) === -1 )
}
>
<BlockListBlock
clientId={ clientId }
index={ blockIndex }
blockRef={ this.setBlockRef }
onSelectionStart={ this.onSelectionStart }
rootClientId={ rootClientId }
isFirst={ blockIndex === 0 }
isLast={ blockIndex === blockClientIds.length - 1 }
isDraggable={ isDraggable }
/>
</AsyncModeProvider>
);
} ) }
<BlockListAppender rootClientId={ rootClientId } />
</div>
);
}
}

export default compose( [
// This component needs to always be synchronous
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
// as it's the one changing the async mode
// depending on the block selection.
forceSyncUpdates,
withSelect( ( select, ownProps ) => {
const {
getBlockOrder,
isSelectionEnabled,
isMultiSelecting,
getMultiSelectedBlocksStartClientId,
getMultiSelectedBlocksEndClientId,
getSelectedBlockClientId,
getMultiSelectedBlockClientIds,
} = select( 'core/editor' );
const { rootClientId } = ownProps;

Expand All @@ -222,6 +250,8 @@ export default compose( [
selectionEnd: getMultiSelectedBlocksEndClientId(),
isSelectionEnabled: isSelectionEnabled(),
isMultiSelecting: isMultiSelecting(),
selectedBlockClientId: getSelectedBlockClientId(),
multiSelectedBlockClientIds: getMultiSelectedBlockClientIds(),
};
} ),
withDispatch( ( dispatch ) => {
Expand Down
1 change: 1 addition & 0 deletions packages/priority-queue/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
3 changes: 3 additions & 0 deletions packages/priority-queue/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### 1.0.0 (Unreleased)

Initial release.
32 changes: 32 additions & 0 deletions packages/priority-queue/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Priority Queue

This module allows you to run a queue of callback while on the browser's idle time making sure the higher-priority work is performed first.

## Installation

Install the module

```bash
npm install @wordpress/priority-queue --save
```

_This package assumes that your code will run in an **ES2015+** environment. If you're using an environment that has limited or no support for ES2015+ such as lower versions of IE then using [core-js](https://github.com/zloirock/core-js) or [@babel/polyfill](https://babeljs.io/docs/en/next/babel-polyfill) will add support for these methods. Learn more about it in [Babel docs](https://babeljs.io/docs/en/next/caveats).

## Usage

```js
import { createQueue } from '@wordpress/priority-queue';

const queue = createQueue();

// Context objects.
const ctx1 = {};
const ctx2 = {};

// For a given context in the queue, only the last callback is executed.
queue.add( ctx1, () => console.log( 'This will be printed first' ) );
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
queue.add( ctx2, () => console.log( 'This won\'t be printed' ) );
queue.add( ctx2, () => console.log( 'This will be printed second' ) );
```

<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p>
29 changes: 29 additions & 0 deletions packages/priority-queue/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "@wordpress/priority-queue",
"version": "1.0.0-alpha.0",
"description": "Generic browser priority queue.",
"author": "The WordPress Contributors",
"license": "GPL-2.0-or-later",
"keywords": [
"wordpress",
"browser",
"async"
],
"homepage": "https://github.com/WordPress/gutenberg/tree/master/packages/priority-queue/README.md",
"repository": {
"type": "git",
"url": "https://github.com/WordPress/gutenberg.git"
},
"bugs": {
"url": "https://github.com/WordPress/gutenberg/issues"
},
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"dependencies": {
"@babel/runtime": "^7.0.0"
},
"publishConfig": {
"access": "public"
}
}
Loading