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

Jump to bottom

Element: enable concurrent mode by implementing mount/unmount with createRoot #46467

Merged
merged 12 commits into from
Feb 1, 2023
+284 −398
Merged

Element: enable concurrent mode by implementing mount/unmount with createRoot #46467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f656947
Post Editor: Disable React strict mode
tyxla Dec 15, 2022
d86c7d8
Don't show BlockSelectionButton when there is multiselection
jsnajdr Jan 6, 2023
9ba1cbc
RichText: fix timing of synthetic input event
jsnajdr Jan 9, 2023
c33cf98
RichText: run composition end in separate microtask tick
jsnajdr Jan 9, 2023
91e4b1c
useAsyncList: fix race condition when reading nextIndex
jsnajdr Jan 25, 2023
2dd8665
Slot: mark as mounted in cDM, fixes strict mode double effects
jsnajdr Jan 25, 2023
a0a7226
font-size-picker.spec: Close sidebar after each test
jsnajdr Jan 26, 2023
5a7d2b8
font-size-picker.spec: Close typography menu so that sidebar close bu…
jsnajdr Jan 26, 2023
e1c2fef
useSlot: subscribe using useSyncExternalStore
jsnajdr Jan 27, 2023
539d35f
Enable concurrent mode by using createRoot to mount editor apps
jsnajdr Jan 30, 2023
aa9cd41
Error boundaries: show Attempt Recovery option only if onError supplied
jsnajdr Jan 30, 2023
1638d6a
Remove ErrorBoundary onError callbacks and recovery buttons
jsnajdr Jan 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
152 changes: 74 additions & 78 deletions packages/block-editor/src/components/block-tools/selected-block-popover.js
View file
Open in desktop
Expand Up @@ -35,6 +35,7 @@ function selector( select ) {

return {
editorMode: __unstableGetEditorMode(),
hasMultiSelection: hasMultiSelection(),
isMultiSelecting: isMultiSelecting(),
isTyping: isTyping(),
isBlockInterfaceHidden: isBlockInterfaceHidden(),
Expand All @@ -57,6 +58,7 @@ function SelectedBlockPopover( {
} ) {
const {
editorMode,
hasMultiSelection,
isMultiSelecting,
isTyping,
isBlockInterfaceHidden,
Expand Down Expand Up @@ -89,7 +91,8 @@ function SelectedBlockPopover( {
const showEmptyBlockSideInserter =
! isTyping && editorMode === 'edit' && isEmptyDefaultBlock;
const shouldShowBreadcrumb =
editorMode === 'navigation' || editorMode === 'zoom-out';
! hasMultiSelection &&
( editorMode === 'navigation' || editorMode === 'zoom-out' );
const shouldShowContextualToolbar =
editorMode === 'edit' &&
! hasFixedToolbar &&
Expand Down Expand Up @@ -129,86 +132,79 @@ function SelectedBlockPopover( {
clientId,
} );

if (
! shouldShowBreadcrumb &&
! shouldShowContextualToolbar &&
! showEmptyBlockSideInserter
) {
return null;
if ( showEmptyBlockSideInserter ) {
return (
<BlockPopover
clientId={ capturingClientId || clientId }
__unstableCoverTarget
bottomClientId={ lastClientId }
className={ classnames(
'block-editor-block-list__block-side-inserter-popover',
{
'is-insertion-point-visible': isInsertionPointVisible,
}
) }
__unstablePopoverSlot={ __unstablePopoverSlot }
__unstableContentRef={ __unstableContentRef }
resize={ false }
shift={ false }
{ ...popoverProps }
>
<div className="block-editor-block-list__empty-block-inserter">
<Inserter
position="bottom right"
rootClientId={ rootClientId }
clientId={ clientId }
__experimentalIsQuick
/>
</div>
</BlockPopover>
);
}

return (
<>
{ showEmptyBlockSideInserter && (
<BlockPopover
clientId={ capturingClientId || clientId }
__unstableCoverTarget
bottomClientId={ lastClientId }
className={ classnames(
'block-editor-block-list__block-side-inserter-popover',
{
'is-insertion-point-visible':
isInsertionPointVisible,
if ( shouldShowBreadcrumb || shouldShowContextualToolbar ) {
return (
<BlockPopover
clientId={ capturingClientId || clientId }
bottomClientId={ lastClientId }
className={ classnames(
'block-editor-block-list__block-popover',
{
'is-insertion-point-visible': isInsertionPointVisible,
}
) }
__unstablePopoverSlot={ __unstablePopoverSlot }
__unstableContentRef={ __unstableContentRef }
resize={ false }
{ ...popoverProps }
>
{ shouldShowContextualToolbar && showContents && (
<BlockContextualToolbar
// If the toolbar is being shown because of being forced
// it should focus the toolbar right after the mount.
focusOnMount={ isToolbarForced.current }
__experimentalInitialIndex={
initialToolbarItemIndexRef.current
}
) }
__unstablePopoverSlot={ __unstablePopoverSlot }
__unstableContentRef={ __unstableContentRef }
resize={ false }
shift={ false }
{ ...popoverProps }
>
<div className="block-editor-block-list__empty-block-inserter">
<Inserter
position="bottom right"
rootClientId={ rootClientId }
clientId={ clientId }
__experimentalIsQuick
/>
</div>
</BlockPopover>
) }
{ ( shouldShowBreadcrumb || shouldShowContextualToolbar ) && (
<BlockPopover
clientId={ capturingClientId || clientId }
bottomClientId={ lastClientId }
className={ classnames(
'block-editor-block-list__block-popover',
{
'is-insertion-point-visible':
isInsertionPointVisible,
}
) }
__unstablePopoverSlot={ __unstablePopoverSlot }
__unstableContentRef={ __unstableContentRef }
resize={ false }
{ ...popoverProps }
>
{ shouldShowContextualToolbar && showContents && (
<BlockContextualToolbar
// If the toolbar is being shown because of being forced
// it should focus the toolbar right after the mount.
focusOnMount={ isToolbarForced.current }
__experimentalInitialIndex={
initialToolbarItemIndexRef.current
}
__experimentalOnIndexChange={ ( index ) => {
initialToolbarItemIndexRef.current = index;
} }
// Resets the index whenever the active block changes so
// this is not persisted. See https://github.com/WordPress/gutenberg/pull/25760#issuecomment-717906169
key={ clientId }
/>
) }
{ shouldShowBreadcrumb && (
<BlockSelectionButton
clientId={ clientId }
rootClientId={ rootClientId }
/>
) }
</BlockPopover>
) }
</>
);
__experimentalOnIndexChange={ ( index ) => {
initialToolbarItemIndexRef.current = index;
} }
// Resets the index whenever the active block changes so
// this is not persisted. See https://github.com/WordPress/gutenberg/pull/25760#issuecomment-717906169
key={ clientId }
/>
) }
{ shouldShowBreadcrumb && (
<BlockSelectionButton
clientId={ clientId }
rootClientId={ rootClientId }
/>
) }
</BlockPopover>
);
}

return null;
}

function wrapperSelector( select ) {
Expand Down
12 changes: 10 additions & 2 deletions packages/block-editor/src/components/rich-text/use-before-input-rules.js
View file
Open in desktop
Expand Up @@ -77,9 +77,17 @@ export function useBeforeInputRules( props ) {
const { defaultView } = ownerDocument;
const newEvent = new defaultView.InputEvent( 'input', init );

// Dispatch an input event with the new data. This will trigger the
// Dispatch an `input` event with the new data. This will trigger the
// input rules.
event.target.dispatchEvent( newEvent );
// Postpone the `input` to the next event loop tick so that the dispatch
// doesn't happen synchronously in the middle of `beforeinput` dispatch.
// This is closer to how native `input` event would be timed, and also
// makes sure that the `input` event is dispatched only after the `onChange`
// call few lines above has fully updated the data store state and rerendered
// all affected components.
window.queueMicrotask( () => {
event.target.dispatchEvent( newEvent );
} );
event.preventDefault();
}

Expand Down
1 change: 1 addition & 0 deletions packages/components/src/slot-fill/bubbles-virtually/fill.js
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@ function useForceUpdate() {
const mounted = useRef( true );

useEffect( () => {
mounted.current = true;
return () => {
mounted.current = false;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/slot-fill/slot.js
View file
Open in desktop
Expand Up @@ -34,7 +34,7 @@ class SlotComponent extends Component {

componentDidMount() {
const { registerSlot } = this.props;

this.isUnmounted = false;
registerSlot( this.props.name, this );
}

Expand Down
22 changes: 6 additions & 16 deletions packages/components/src/slot-fill/use-slot.js
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@
/**
* WordPress dependencies
*/
import { useContext, useState, useEffect } from '@wordpress/element';
import { useContext, useSyncExternalStore } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -17,21 +17,11 @@ import SlotFillContext from './context';
*/
const useSlot = ( name ) => {
const { getSlot, subscribe } = useContext( SlotFillContext );
const [ slot, setSlot ] = useState( getSlot( name ) );

useEffect( () => {
setSlot( getSlot( name ) );
const unsubscribe = subscribe( () => {
setSlot( getSlot( name ) );
} );

return unsubscribe;
// Ignore reason: Modifying this dep array could introduce unexpected changes in behavior,
// so we'll leave it as=is until the hook can be properly refactored for exhaustive-deps.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ name ] );

return slot;
return useSyncExternalStore(
subscribe,
() => getSlot( name ),
() => getSlot( name )
);
};

export default useSlot;
10 changes: 4 additions & 6 deletions packages/compose/src/hooks/use-async-list/index.ts
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ function useAsyncList< T >(
config: AsyncListConfig = { step: 1 }
): T[] {
const { step = 1 } = config;
const [ current, setCurrent ] = useState( [] as T[] );
const [ current, setCurrent ] = useState< T[] >( [] );

useEffect( () => {
// On reset, we keep the first items that were previously rendered.
Expand All @@ -55,21 +55,19 @@ function useAsyncList< T >(
);
}
setCurrent( firstItems );
let nextIndex = firstItems.length;

const asyncQueue = createQueue();
const append = () => {
const append = ( nextIndex: number ) => () => {
if ( list.length <= nextIndex ) {
return;
}
setCurrent( ( state ) => [
...state,
...list.slice( nextIndex, nextIndex + step ),
] );
nextIndex += step;
asyncQueue.add( {}, append );
asyncQueue.add( {}, append( nextIndex + step ) );
};
asyncQueue.add( {}, append );
asyncQueue.add( {}, append( firstItems.length ) );
Copy link
Member

Choose a reason for hiding this comment

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

This would make a great fix for a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's true, but there are 8 commits like this in the PR. They are all self-contained fixes that are independent from each other.

At this moment I plan to cleanup the reinitializeEditor APIs, something we discussed more than a month ago but never executed. That's the last item that needs to be done before merging.

Copy link
Member

Choose a reason for hiding this comment

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

No blockers with this at all, I was just sharing thoughts.


return () => asyncQueue.reset();
}, [ list ] );
Expand Down
6 changes: 5 additions & 1 deletion packages/e2e-tests/specs/editor/various/rich-text.test.js
View file
Open in desktop
Expand Up @@ -515,11 +515,15 @@ describe( 'RichText', () => {
// text in the DOM directly, setting selection in the right place, and
// firing `compositionend`.
// See https://github.com/puppeteer/puppeteer/issues/4981.
await page.evaluate( () => {
await page.evaluate( async () => {
document.activeElement.textContent = '`a`';
const selection = window.getSelection();
// The `selectionchange` and `compositionend` events should run in separate event
// loop ticks to process all data store updates in time. Native events would be
// scheduled the same way.
selection.selectAllChildren( document.activeElement );
selection.collapseToEnd();
await new Promise( ( r ) => setTimeout( r, 0 ) );
document.activeElement.dispatchEvent(
new CompositionEvent( 'compositionend' )
);
Expand Down
12 changes: 1 addition & 11 deletions packages/edit-post/README.md
View file
Open in desktop
Expand Up @@ -478,17 +478,7 @@ _Returns_

### reinitializeEditor

Reinitializes the editor after the user chooses to reboot the editor after
an unhandled error occurs, replacing previously mounted editor element using
an initial state from prior to the crash.

_Parameters_

- _postType_ `Object`: Post type of the post to edit.
- _postId_ `Object`: ID of the post to edit.
- _target_ `Element`: DOM node in which editor is rendered.
- _settings_ `?Object`: Editor settings object.
- _initialEdits_ `Object`: Programmatic edits to apply initially, to be considered as non-user-initiated (bypass for unsaved changes prompt).
Used to reinitialize the editor after an error. Now it's a deprecated noop function.

### store

Expand Down
51 changes: 20 additions & 31 deletions packages/edit-post/src/editor.js
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ import {
store as editorStore,
experiments as editorExperiments,
} from '@wordpress/editor';
import { StrictMode, useMemo } from '@wordpress/element';
import { useMemo } from '@wordpress/element';
import { SlotFillProvider } from '@wordpress/components';
import { store as coreStore } from '@wordpress/core-data';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';
Expand All @@ -25,14 +25,7 @@ import { unlock } from './experiments';

const { ExperimentalEditorProvider } = unlock( editorExperiments );

function Editor( {
postId,
postType,
settings,
initialEdits,
onError,
...props
} ) {
function Editor( { postId, postType, settings, initialEdits, ...props } ) {
const {
hasFixedToolbar,
focusMode,
Expand Down Expand Up @@ -180,28 +173,24 @@ function Editor( {
}

return (
<StrictMode>
Copy link
Member

Choose a reason for hiding this comment

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

We should keep in mind to open a PR enabling StrictMode as soon as we land this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Strict mode enabler PR opened here: #47639

<ShortcutProvider>
<SlotFillProvider>
<ExperimentalEditorProvider
settings={ editorSettings }
post={ post }
initialEdits={ initialEdits }
useSubRegistry={ false }
__unstableTemplate={
isTemplateMode ? template : undefined
}
{ ...props }
>
<ErrorBoundary onError={ onError }>
<EditorInitialization postId={ postId } />
<Layout styles={ styles } />
</ErrorBoundary>
<PostLockedModal />
</ExperimentalEditorProvider>
</SlotFillProvider>
</ShortcutProvider>
</StrictMode>
<ShortcutProvider>
<SlotFillProvider>
<ExperimentalEditorProvider
settings={ editorSettings }
post={ post }
initialEdits={ initialEdits }
useSubRegistry={ false }
__unstableTemplate={ isTemplateMode ? template : undefined }
{ ...props }
>
<ErrorBoundary>
<EditorInitialization postId={ postId } />
<Layout styles={ styles } />
</ErrorBoundary>
<PostLockedModal />
</ExperimentalEditorProvider>
</SlotFillProvider>
</ShortcutProvider>
);
}

Expand Down