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

Removing Navigation Block causes console warning re: state #22830

Closed
getdave opened this issue Jun 2, 2020 · 6 comments
Closed

Removing Navigation Block causes console warning re: state #22830

getdave opened this issue Jun 2, 2020 · 6 comments
Labels
[Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended

Comments

@getdave
Copy link
Contributor

getdave commented Jun 2, 2020

Note this error is probably not specific to the Navigation block.

Describe the bug
When removing the Navigation block from a post the following error appears in the devtools console:

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 a useEffect cleanup function.
    in ForwardRef(ToolbarContainer) (created by ForwardRef(Toolbar))
    in ForwardRef(Toolbar) (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)

When this is fixed we may need to remove the following line from the Navigation block e2e tests added in #18869:

// Required until fix in place for:
// https://github.com/WordPress/gutenberg/issues/22830
// Ideally we'd be more specific about the error message but it
// does not support partial string matches using `expect.stringContaining()`.
expect( console ).toHaveErrored();

To reproduce
Steps to reproduce the behavior:

  1. On master branch on latest WP 5.5 Enable experiments.
  2. Create a post.
  3. Add a Navigation block.
  4. Remove the block using the block toolbar menu or by selecting an hitting the backspace key.
  5. See error in console.

Expected behavior
There should be no error in the console.

Screenshots
Screenshot 2020-06-02 at 15 12 13

Editor version (please complete the following information):

  • WordPress version: [e.g: 5.3.2]: 5.5
  • Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? [e.g: "gutenberg plugin", "default"]: Current master branch.
  • If the Gutenberg plugin is installed, which version is it? [e.g., 7.6]: Current master branch.

Desktop (please complete the following information):

  • OS: [e.g. iOS]: MacOS
  • Browser [e.g. chrome, safari]: Chrome
  • Version [e.g. 22]: 83
@getdave getdave added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block labels Jun 2, 2020
@talldan
Copy link
Contributor

talldan commented Jun 3, 2020

Just linking to my comment over here #18869 (comment)

TLDR—the above warning should be fixed, but expect( console ).toHaveErrored(); shouldn't be required in the tests.

@getdave
Copy link
Contributor Author

getdave commented Jun 3, 2020

Yeh I'm not actually sure this is anything specific to the Nav Block. Let me take a look at the advice in your comment. Much appreciated.

@getdave getdave removed the [Block] Navigation Affects the Navigation Block label Jun 3, 2020
@draganescu draganescu added the [Block] Navigation Affects the Navigation Block label Jun 4, 2020
getdave added a commit that referenced this issue Jun 10, 2020
* Create Navigation block from existing WP  Menus (interactive rebase merge)

* Update to use CustomSelect control to match Design

* Update labels and add Create from Pages to dropdown

* Update all options to be under the select dropdown

Updates to match new design by moving all UI buttons in the placeholder under the single dropdown menu.

See #18869 (comment)

* DRY up dropdown menu options

* Rename UI vars to be agnostic to Pages or Menus specifically.

* Improve code readability via naming changes

* Updates dropown UI styles to match visual Design

* Removes divider option and replace with CSS created divider

* Fix size jump depending on dropdown selection

* Fix to not allow selection of PLACEHOLDER option as valid dropdown selection.

* Simplify conditionals

Fixes #18869 (comment)

* Move constants outside of Component

Addresses #18869 (comment)

* Update create from Pages e2e spec to match new Design

* Remove unneeded wait command

* Fix avoid Menu Items requests for invalid Menus

If the selected dropdown option isn’t a Menu then we don’t want to try to fetch its menu items from the REST API.

* Fix disable Create button if Menu Items not yet resolved from API

* Fixes e2e tests for Menus specs

* Refactor out process of clicking on Create button

* Fix test checking for empty block creation if menu is empty

* Fix test don’t show dropdown options for Menus if there aren’t any

* Fix create button enable if create from empty is selected

* Fix empty nav populatoin test

* Fix test create pages from Block using new util to create empty nav block

* Update nomenclature for mock matching to “routes” not URLs

* Fix dropdown to use pointer cursor style instead of text

* Fix button not disabled if Pages are available.

* Make placeholder instruction text contextually aware

* Updates e2e test snapshot

* Disable e2e failures due to unrelated state update issue

See #22830

* Update snapshot

* Revert "Disable e2e failures due to unrelated state update issue"

This reverts commit b6f34bb.

* Update to modern syntax for looping

Addresses #18869 (review)

* Update packages/block-library/src/navigation/edit.js

Co-authored-by: Daniel Richards <[email protected]>

* Update packages/e2e-tests/specs/experiments/navigation.test.js

Co-authored-by: Daniel Richards <[email protected]>

* Update packages/block-library/src/navigation/style.scss

Co-authored-by: Daniel Richards <[email protected]>

* Update packages/block-library/src/navigation/edit.js

Co-authored-by: Daniel Richards <[email protected]>

* Update packages/block-library/src/navigation/edit.js

Co-authored-by: Daniel Richards <[email protected]>

* Update packages/block-library/src/navigation/edit.js

Co-authored-by: Daniel Richards <[email protected]>

* Fix duplicate entities

Error due to rebasd.

Fixes #18869 (comment)

* Fix if conditional to conform to coding standards

* Refactor menu selects to use core shorthand

Addresses #18869 (comment)

* Revert unintended style mod to CustomSelectControl component

Addresses #18869 (comment)

* Fix selectors to use core shorthand

* Remove CustomSelectControl fixes now in upstream

* Consistently name selector props with get prefix when function

Addresses #18869 (comment)

* Rename var to better reflect purpose and avoid ambiguity

Addresses #18869 (comment)

* Update function name to prefix with verb for clarity

Addresses #18869 (comment)

* Update case so that placeholder is a single word

Addresses #18869 (comment)

* Update packages/block-library/src/navigation/create-data-tree.js

Co-authored-by: Daniel Richards <[email protected]>

* Update packages/block-library/src/navigation/create-data-tree.js

Co-authored-by: Daniel Richards <[email protected]>

* Memozie dropdown options to avoid re-renders

Addresses #18869 (comment)

* Rename to disambiguate dropdown term

* Adds @return to docblock

* Normalise selector format

* Avoid setState re-render for same option selection

* Update to show placeholder loading state when requesting pages or menus.

* Fixes loading spinner alignment

* Update to use var to store ref to common state property

* Apply useCallback to utility function

* Apply useCallback to improve perf

* FIx e2e test to wait for dropdown to be present before interaction

Due to loading spinner we now have to wait on the dropdown before interacting.

* Janitorial - fix to single quotes

* Updates dropdown divider to rely on classname over placement

This fix will work once a complementary update has been applied to CustomSelectControl to enable the options to have a custom classname applied.

* Fix double with single quotes

* Update packages/block-library/src/navigation/edit.js

Co-authored-by: Daniel Richards <[email protected]>

* Move fixture to subfolder

Fixes #18869 (comment)

Co-authored-by: Daniel Richards <[email protected]>
@haszari
Copy link
Contributor

haszari commented Jun 14, 2020

I see this same error in WordPress 5.4.2 when clicking away from the block popover menu.

  • Open browser dev tools, show console.
  • Add a block to page, or select an existing block.
  • Click three dots menu to show block menu (Hide Block Settigns, Remove Block etc).
  • Click another block or click away to dismiss popover menu.

See this React error in console:

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 the componentWillUnmount method.
    in SlotComponent (created by Context.Consumer)
    in Slot (created by slot_fill_Slot)
    in slot_fill_Slot (created by __experimentalBlockSettingsMenuFirstItemSlot)
    in __experimentalBlockSettingsMenuFirstItemSlot (created by Dropdown)
    in div (created by MenuGroup)
    in div (created by MenuGroup)
    in MenuGroup (created by Dropdown)
    in div (created by NavigableContainer)
    in NavigableContainer (created by ForwardRef(NavigableContainer))
    in ForwardRef(NavigableContainer) (created by ForwardRef(NavigableMenu))
    in ForwardRef(NavigableMenu) (created by Dropdown)
    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 Unknown (created by FocusReturn)
    in div (created by FocusReturn)
    in FocusReturn (created by Context.Consumer)
    in WithFocusReturn(Component) (created by WithConstrainedTabbing(WithFocusReturn(Component)))
    in div (created by WithConstrainedTabbing(WithFocusReturn(Component)))
    in WithConstrainedTabbing(WithFocusReturn(Component)) (created by Popover)
    in bubbles_virtually_fill_Fill (created by slot_fill_Fill)
    in slot_fill_Fill (created by Popover)
    in span (created by Popover)
    in Popover (created by Dropdown)
    in div (created by Dropdown)
    in Dropdown (created by DropdownMenu)
    in DropdownMenu (created by BlockActions)
    in div (created by ToolbarGroupContainer)
    in ToolbarGroupContainer (created by ToolbarGroup)
    in ToolbarGroup (created by toolbar_Toolbar)
    in toolbar_Toolbar (created by BlockActions)
    in BlockActions (created by WithDispatch(BlockActions))
    in WithDispatch(BlockActions)
    in Unknown (created by WithSelect(WithDispatch(BlockActions)))
    in WithSelect(WithDispatch(BlockActions)) (created by BlockSettingsMenu)
    in BlockSettingsMenu (created by BlockToolbar)
    in div (created by BlockToolbar)
    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 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 bubbles_virtually_fill_Fill (created by slot_fill_Fill)
    in slot_fill_Fill (created by Popover)
    in Popover (created by BlockPopover)

@diegohaz
Copy link
Member

@haszari Looks like that one is related to #17355 (comment)

@diegohaz
Copy link
Member

I believe those warnings must have gone away in all blocks except Navigation (which #23281 should hopefully fix). Can someone confirm that?

@adamziel
Copy link
Contributor

adamziel commented Jul 3, 2020

In my testing:

  • The warning is there when I go back to the version from Jun 1st.
  • The warning isn't there on the latest master branch.

I'm closing this as solved, feel free to revive if the problem shows up again in the recent version of Gutenberg.

@adamziel adamziel closed this as completed Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

6 participants