Block Editor: Clear toolbar timeout by ref on unmount #20546

aduth · 2020-02-28T22:01:31Z

This pull request seeks to resolve a warning which can occur in some circumstances where a block toolbar component is unmounted while its deferred hide/show movers behavior is scheduled. The scheduling was not being cleared when the component unmounts.

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in %s.%s a useEffect cleanup function 
        in BlockToolbar (created by BlockContextualToolbar)
        in div (created by NavigableContainer)
        in NavigableContainer (created by ForwardRef(NavigableContainer))
        in ForwardRef(NavigableContainer) (created by ForwardRef(NavigableMenu))
        in ForwardRef(NavigableMenu) (created by NavigableToolbar)
        in NavigableToolbar (created by BlockContextualToolbar)
        in div (created by BlockContextualToolbar)
        in BlockContextualToolbar (created by BlockPopover)
        in div (created by Animate)
        in div (created by ForwardRef)
        in ForwardRef (created by Animate)
        in Animate (created by Popover)
        in PopoverDetectOutside (created by WithFocusOutside(PopoverDetectOutside))
        in div (created by WithFocusOutside(PopoverDetectOutside))
        in WithFocusOutside(PopoverDetectOutside) (created by Popover)
        in Fill (created by Fill)
        in Fill (created by Popover)
        in Popover (created by BlockPopover)

Testing Instructions:

I originally observed this when running the meta box end-to-end tests in #20544 when building in development mode (where the React warnings logged in development mode will flag an end-to-end test failure).

You can confirm the following fails in master:

npm run dev
npm run test-e2e packages/e2e-tests/specs/editor/plugins/meta-attribute-block.test.js

Then change to this branch. Running those commands again should show success.

aduth · 2020-02-28T22:05:07Z

Related: #14845

github-actions · 2020-02-28T22:06:43Z

Size Change: -31 B (0%)

Total Size: 866 kB

Filename	Size	Change
`build/block-editor/index.js`	104 kB	+8 B (0%)
`build/block-editor/style-rtl.css`	10.5 kB	-6 B (0%)
`build/block-editor/style.css`	10.5 kB	-6 B (0%)
`build/edit-post/index.js`	90.9 kB	-2 B (0%)
`build/edit-post/style-rtl.css`	8.53 kB	-13 B (0%)
`build/edit-post/style.css`	8.53 kB	-12 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.01 kB	0 B
`build/annotations/index.js`	3.43 kB	0 B
`build/api-fetch/index.js`	3.39 kB	0 B
`build/autop/index.js`	2.58 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	6.02 kB	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	760 B	0 B
`build/block-library/editor-rtl.css`	7.66 kB	0 B
`build/block-library/editor.css`	7.66 kB	0 B
`build/block-library/index.js`	116 kB	0 B
`build/block-library/style-rtl.css`	7.49 kB	0 B
`build/block-library/style.css`	7.5 kB	0 B
`build/block-library/theme-rtl.css`	669 B	0 B
`build/block-library/theme.css`	671 B	0 B
`build/block-serialization-default-parser/index.js`	1.65 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	57.6 kB	0 B
`build/components/index.js`	191 kB	0 B
`build/components/style-rtl.css`	15.5 kB	0 B
`build/components/style.css`	15.5 kB	0 B
`build/compose/index.js`	5.76 kB	0 B
`build/core-data/index.js`	10.5 kB	0 B
`build/data-controls/index.js`	1.03 kB	0 B
`build/data/index.js`	8.22 kB	0 B
`build/date/index.js`	5.37 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.06 kB	0 B
`build/edit-site/index.js`	4.63 kB	0 B
`build/edit-site/style-rtl.css`	2.51 kB	0 B
`build/edit-site/style.css`	2.51 kB	0 B
`build/edit-widgets/index.js`	4.42 kB	0 B
`build/edit-widgets/style-rtl.css`	2.59 kB	0 B
`build/edit-widgets/style.css`	2.58 kB	0 B
`build/editor/editor-styles-rtl.css`	325 B	0 B
`build/editor/editor-styles.css`	327 B	0 B
`build/editor/index.js`	44.6 kB	0 B
`build/editor/style-rtl.css`	4.01 kB	0 B
`build/editor/style.css`	4 kB	0 B
`build/element/index.js`	4.45 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.6 kB	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	1.92 kB	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.48 kB	0 B
`build/is-shallow-equal/index.js`	710 B	0 B
`build/keyboard-shortcuts/index.js`	2.3 kB	0 B
`build/keycodes/index.js`	1.68 kB	0 B
`build/list-reusable-blocks/index.js`	2.99 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/media-utils/index.js`	4.85 kB	0 B
`build/notices/index.js`	1.57 kB	0 B
`build/nux/index.js`	3.02 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/plugins/index.js`	2.54 kB	0 B
`build/primitives/index.js`	1.49 kB	0 B
`build/priority-queue/index.js`	780 B	0 B
`build/redux-routine/index.js`	2.84 kB	0 B
`build/rich-text/index.js`	14.3 kB	0 B
`build/server-side-render/index.js`	2.54 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.27 kB	0 B
`build/url/index.js`	4 kB	0 B
`build/viewport/index.js`	1.61 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

youknowriad · 2020-03-02T08:19:49Z

packages/block-editor/src/components/block-toolbar/utils.js

@@ -59,6 +59,8 @@ export function useDebouncedShowMovers( {
 		[ isFocused ]
 	);

+	useEffect( () => () => clearTimeout( timeoutRef.current ) );


If you want to uun this on unmount, you need to add [] as a second argument of useEffect otherwise it runs on each render.

Good catch Riad, I see it explained in React docs:

The default behavior for effects is to fire the effect after every completed render. That way an effect is always recreated if one of its dependencies changes.

If you want to run an effect and clean it up only once (on mount and unmount), you can pass an empty array ([]) as a second argument. This tells React that your effect doesn’t depend on any values from props or state, so it never needs to re-run. This isn’t handled as a special case — it follows directly from how the dependencies array always works.

https://reactjs.org/docs/hooks-reference.html#conditionally-firing-an-effect

Fixed in 02eb218.

Good catch, thanks for spotting @youknowriad, and for the fix @gziolo .

gziolo

I still could see a similar error but triggered from a different place:

The only way I could reproduce is by doing the following:

Add a few Paragraph blocks.
Save the draft and refresh the page.
Remove the block and observe JS console in Dev Tools.

It only gets triggered for the first block removed. It works perfectly fine after that when removing any blocks.

gziolo

I found a similar but apparently a different issue. Let's merge this fix for now 🚢

aduth · 2020-03-02T13:05:12Z

I removed 5.4-related project and issue assignment. As far as I can tell, this was only introduced as of #19344, so it doesn't need to be backported. Let me know if I'm wrong.

gziolo · 2020-03-02T13:09:30Z

I removed 5.4-related project and issue assignment. As far as I can tell, this was only introduced as of #19344, so it doesn't need to be backported. Let me know if I'm wrong.

I know that issue existed forever, but it might another one I shared as well 🤷‍♂ I guess, it's fine to wait for WP 5.5 rather than work on two independent fixes. Unless you think it might have some really negative consequences.

aduth · 2020-03-02T13:10:26Z

Well, the specific file changed was only introduced as of #19344. If there's a related issue, I expect it might need its own patch, since this one won't apply cleanly.

aduth added [Type] Bug An existing feature does not function as intended [Package] Block editor /packages/block-editor Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Feb 28, 2020

aduth requested a review from youknowriad February 28, 2020 22:01

aduth requested a review from ellatrix as a code owner February 28, 2020 22:01

Block Editor: Clear toolbar timeout by ref on unmount

d6024dc

aduth force-pushed the fix/browser-toolbar-unsubscribe branch from 81ac91c to d6024dc Compare February 28, 2020 23:21

youknowriad reviewed Mar 2, 2020

View reviewed changes

gziolo requested a review from jorgefilipecosta March 2, 2020 09:51

gziolo reviewed Mar 2, 2020

View reviewed changes

Add 2nd param to useEffect call

02eb218

gziolo approved these changes Mar 2, 2020

View reviewed changes

aduth merged commit 72eca2a into master Mar 2, 2020

aduth deleted the fix/browser-toolbar-unsubscribe branch March 2, 2020 12:59

github-actions bot added this to the Gutenberg 7.7 milestone Mar 2, 2020

aduth removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Mar 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Editor: Clear toolbar timeout by ref on unmount #20546

Block Editor: Clear toolbar timeout by ref on unmount #20546

aduth commented Feb 28, 2020 •

edited

Loading

aduth commented Feb 28, 2020

github-actions bot commented Feb 28, 2020 •

edited

Loading

youknowriad Mar 2, 2020

gziolo Mar 2, 2020

gziolo Mar 2, 2020

aduth Mar 2, 2020

gziolo left a comment •

edited

Loading

gziolo left a comment

aduth commented Mar 2, 2020

gziolo commented Mar 2, 2020

aduth commented Mar 2, 2020

Block Editor: Clear toolbar timeout by ref on unmount #20546

Block Editor: Clear toolbar timeout by ref on unmount #20546

Conversation

aduth commented Feb 28, 2020 • edited Loading

aduth commented Feb 28, 2020

github-actions bot commented Feb 28, 2020 • edited Loading

youknowriad Mar 2, 2020

Choose a reason for hiding this comment

gziolo Mar 2, 2020

Choose a reason for hiding this comment

gziolo Mar 2, 2020

Choose a reason for hiding this comment

aduth Mar 2, 2020

Choose a reason for hiding this comment

gziolo left a comment • edited Loading

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

aduth commented Mar 2, 2020

gziolo commented Mar 2, 2020

aduth commented Mar 2, 2020

aduth commented Feb 28, 2020 •

edited

Loading

github-actions bot commented Feb 28, 2020 •

edited

Loading

gziolo left a comment •

edited

Loading