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

Not possible to change allowedBlocks dynamically #14515

Closed
swissspidy opened this issue Mar 19, 2019 · 13 comments · Fixed by #53943
Closed

Not possible to change allowedBlocks dynamically #14515

swissspidy opened this issue Mar 19, 2019 · 13 comments · Fixed by #53943
Assignees
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@swissspidy
Copy link
Member

(Previous discussion on Slack)

Describe the bug

Background: I have a block type A that uses the InnerBlocks component with a limited list of allowedBlocks. I now want to limit the inner blocks in a way that I can insert block type B only once within InnerBlocks. Overall, in the whole post, it can occur multiple times though.

One suggestion was to modify allowedBlocks dynamically. That's what I am now doing, but currently fail to do so.

I want to change allowedBlocks depending on some props, e.g. whether the block is the first one in the block order and whether it has already been added to the page.

Unfortunately this doesn't seem to be possible at the moment.

To Reproduce

Given a block type with code like this:

export default withSelect( ( select, { clientId } ) => {
    const { getBlockOrder } = select( 'core/editor' );
    return {
        isFirstBlock: getBlockOrder().indexOf( clientId ) === 0,
    };
} )( MyBlockEdit );
// ...
// In MyBlockEdit's edit() function:

<InnerBlocks template={ myTemplate } allowedBlocks={ isFirstBlock ? [ 'my/block-a' ] : [ 'my/block-b' ] } />

Create 2 blocks of this type and notice that you're still able to insert both my/block-a and my/block-b in each.

You might even run into https://reactjs.org/docs/error-decoder.html/?invariant=185 at this point.

You can verify that allowedBlocks changes by using wp.data.select( 'core/editor' ).getBlockListSettings() in dev tools.

At the same time, the result of wp.data.select( 'core/editor' ).canInsertBlockType() does not seem to change.

That selector uses memoization, although it should change when allowedBlocks changes. Apparently it doesn't.

Expected behavior

It should be possible to dynamically filter allowedBlocks.

It should be possible to limit a block to be only used once within a parent block.

Additional context

  • Gutenberg version: 5.3 RC1
@swissspidy swissspidy added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P labels Mar 19, 2019
swissspidy added a commit to ampproject/amp-wp that referenced this issue Mar 19, 2019
Also, it cannot be added on the first page.

Currently not 100% bulletproof: WordPress/gutenberg#14515
@swissspidy swissspidy added the Needs Technical Feedback Needs testing from a developer perspective. label Mar 19, 2019
@swissspidy
Copy link
Member Author

@jorgefilipecosta Heya, since we briefly discussed this on Slack the other day (see archive link in the issue above) - do you perhaps have any suggestions for me on how to best approach this? I'd love to try to find the root cause of the issue. Are there perhaps some tests for this functionality somewhere?

@jorgefilipecosta
Copy link
Member

Hi @swissspidy, I am not being able to replicate this problem.
I used the following test block:
https://gist.github.com/jorgefilipecosta/a952fe05e470938dfc4d267e93e9af89

I added a Container block I verified that the allow blocks change after adding two blocks.

Watching your example allowedBlocks={ isFirstBlock ? [ 'my/block-a' ] : [ 'my/block-b' ] }, it seems you are passing a new array referrence on each reference, could you try passing the same references ? (maybe it is related to the problem).

@swissspidy
Copy link
Member Author

Interesting.

Actually my code is more like this:

import { LIST_A, LIST_B } from './constants.js';

export default withSelect( ( select, { clientId } ) => {
    const { getBlockOrder } = select( 'core/editor' );
    return {
        isFirstBlock: getBlockOrder().indexOf( clientId ) === 0,
    };
} )( MyBlockEdit );
// ...
// In MyBlockEdit's edit() function:

<InnerBlocks template={ myTemplate } allowedBlocks={ isFirstBlock ? LIST_A : LIST_B } />

I just shortened it for the example here.

The current actual code can be found here: https://github.com/ampproject/amp-wp/blob/3e7050ac1e03d0b69d2ceff14f01a0ff0f7d91db/assets/src/blocks/amp-story-page/edit.js#L339-L367

It's a bit different than that example at the moment, but I also just quickly changed it to that - still not working.

I'll take another look soon though to see if I am missing something.

jorgefilipecosta added a commit that referenced this issue Apr 16, 2019
## Description
This PR adds a simple end 2 end test that makes sure allowed blocks inside a parent block can be changed dynamically depending on some condition e.g: the number of child's the parent contains.

## How has this been tested?
We just need to make sure the test cases pass.

Related: #14515
@aduth aduth removed the Needs Technical Feedback Needs testing from a developer perspective. label Apr 22, 2019
@desaiuditd
Copy link
Member

I'm also facing the same issue. But my use case is to update the blocks within the template and their attributes.

Below is the snippet that I'm trying to run. https://gist.github.com/desaiuditd/c6ef602b86afa8f22034ab043766517f

In a nutshell it's allowing below use case.

// index.js
- register my custom block with the edit component.

// edit.js
- All the edit functionality. It has got 2 states.

1. Search State (search.js - not included in the gist. Imagine that it's providing Autocomplete to search for items.)
  - Search is allowing the user to find different items.
  - Upon selecting an item the edit component will switch its state to preview
2. Preview State (preview.js)
  - Preview state has got InnerBlocks with Heading, Image & Paragraph blocks
  - Heading, Image, Paragraph show the currently selected item's values
    - This is what I'm expecting. But it's not happening. 😢 

// controls.js
- Add a BlockControls component with an IconButton in the block toolbar to switch between Preview & Search state.

@swissspidy Did you ever find any solution on this?

@jorgefilipecosta
Copy link
Member

Hi @swissspidy, I made another test similar to yours where allows blocks depends on if the block is the first block or not: https://gist.github.com/jorgefilipecosta/40efacc3f573b666afd89df461a83b69

And it worked as expected:
Jun-27-2019 11-35-15

Is there any update to the issue? Did you found the root cause, or it is still happening in the last version of Gutenberg? When trying the block, I provided in your environment the problem persists?

Hi @desaiuditd the issue you are describing with templates is a different one. Templates are just a prefilling mechanism for empty InnerBlock areas. When the template changes if the InnerBlocks area is not empty there should be no expectation an update will happen.

In this comment #15965 (comment), I wrote an expanded explanation, and I suggest possible alternatives. I hope you find it helpful.

@swissspidy
Copy link
Member Author

Thanks for testing @jorgefilipecosta, I really appreciate it! In your example code, is wp.data.select( 'core/editor' ).canInsertBlockType() also returning different results as expected?

At some point we ended up replacing the inserter with our own patched version because of this, so I am not sure if the original issue is still happening. I will have to check it out.

One workaround we added was in addition to using the unstable canInsertBlockType() we also used getBlockListSettings() to manually check the list of allowedBlocks in it.

@spacedmonkey
Copy link
Member

Related: #16560

@mtias
Copy link
Member

mtias commented Aug 30, 2020

Is this issue still relevant? cc @jorgefilipecosta

@desaiuditd
Copy link
Member

desaiuditd commented May 3, 2022

Coming back to this. It is still not possible to change allowedBlocks dynamically for InnerBlocks.

I tested this on WordPress vanilla version 5.9.3 without Gutenberg plugin, as well as with Gutenberg plugin version 13.1.0.

I get below warning on the browser console as well.

react-dom.js?ver=5.9.3:1 Warning: The final argument passed to useMemo changed size between renders. The order and size of this array must remain constant.

Previous: [core/image]
Incoming: []
    at bf (http://localhost:8888/wp-content/plugins/gutenberg/build/block-editor/index.min.js?ver=0ea1dd48a433c928965110278f528535:64:7110)
    at div
    at div
    at http://localhost:8888/wp-content/plugins/gutenberg/build/block-editor/index.min.js?ver=0ea1dd48a433c928965110278f528535:64:9087
    at div
    at Edit (http://localhost:8888/wp-content/plugins/ink-yo-block/build/index.js?ver=0fe53b9a87caa39f4de10c18b3677515:29:5)

https://github.com/WordPress/gutenberg/blob/df341a8/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js#L66-L70

This block of code seems to be the culprit. Seems like, the allowedBlocks is being passed as is to the dependency array of useMemo hook. Which is a problem because allowedBlocks can become undefined, false, true, empty array [] or an array with block names [ ... ]. But the dependency array of useMemo hook can't change, as per React principles. Hence the warning. Plus the Block inserter menu is also not updated as per the new allowed blocks.

Introduced in this PR #30311. Merged in version 10.4. So likely to happen in all the versions after that. Even in WordPress core.

@ellatrix Tagging you, just in case you have any insight on this.

Below is my reproducible code.

edit.js

import { __ } from '@wordpress/i18n';
import { useBlockProps, InnerBlocks } from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';

export default function Edit( { clientId } ) {
	const childrenCount = useSelect( select => select( 'core/block-editor' ).getClientIdsOfDescendants( [ clientId ] ).length );

	// Business logic
	const maxChildrenAllowed = 3;

	return (
		<div {...useBlockProps()}>
			<p>{__('Yo Block – hello from the editor!', 'ink')}</p>
			<InnerBlocks
				allowedBlocks={
					childrenCount < maxChildrenAllowed ?
						['core/image'] :
						[]
				}
			/>
		</div>
	);
}

The error would occur as soon as I add a third child block inside the inner block.

@leclaeli
Copy link

leclaeli commented May 3, 2022

So the likely fix is to wrap the allowedBlocks variable in array.

const _allowedBlocks = useMemo( () => allowedBlocks, [ allowedBlocks ] );

@dave-slaughter
Copy link

I'm having the same issue in WordPress 6.1.1 (also tested in Gutenberg 15.1.0), and having dug around a bit would like to suggest a fix, and a workaround.

I think the root of the problem is in packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js.

// Memoize as inner blocks implementors often pass a new array on every
// render.
const _allowedBlocks = useMemo( () => allowedBlocks, allowedBlocks );

which was added in #30311 by @ellatrix

This code calls useMemo incorrectly, as according to https://beta.reactjs.org/reference/react/useMemo

"The list of dependencies must have a constant number of items and be written inline like [dep1, dep2, dep3]. React will compare each dependency with its previous value using the Object.is comparison algorithm."

And of course the allowedBlocks array can change size!

The reason that allowedBlocks won't sometimes change dynamically is that useMemo only compares the previous and current dependency arrays up until the smallest size to determine if they are different, so to it ['block1','block2'] and ['block1'] are the same (and I think [] is the same as everything), so it returns the memoized value - hence no change to allowed blocks.

I'm guessing useMemo was added because people are doing <InnerBlocks allowedBlocks={ ['block1'] } /> which sends a new reference every render, which causes updates even if the contents haven't changed.

If that's the case then the following code calls useMemo correctly and should achieve the desired result, and I think fix this issue:

const _allowedBlocks = useMemo( () => allowedBlocks, [ allowedBlocks?.toString() ] );

I'm assuming here that allowedBlocks can be undefined or an array. An alternative would be [ JSON.stringify(allowedBlocks) ].

Note that

const _allowedBlocks = useMemo( () => allowedBlocks, [ allowedBlocks ] );

won't work because Object.is([],[]) is false.

I modified this code in wp-includes\js\dist\block-editor.js and ran with SCRIPT_DEBUG, and with this naive testing it seems to work - at least the allowed blocks are changing correctly.

I managed to work around this issue by making sure useMemo thinks the allowed blocks arrays are different by having the arrays not match up to the smallest array size. Of course the easiest way to do that is to change the first element:

const ALLOWED_BLOCKS_1 = ['core/paragraph','core/image'];
const ALLOWED_BLOCKS_2 = ['core/image'];
function Edit() ...
	allowedBlocks={ condition ? ALLOWED_BLOCKS_1 : ALLOWED_BLOCKS_2 }

const ALLOWED_BLOCKS_1 = ['core/image','core/paragraph']; would not work as then the arrays match in position 0, and that's all useMemo checks up to.

Note: You might want to pass the same allowedBlocks and just modify the elements. That seems to work but you get the side effect that it comes up as allowed, but you can't actually add it.

@sunilverma209
Copy link

Hi, I'm having the same issue, any clue to fix it. I have a props if toggled then render ALLOWED_BLOCKS_1 else ALLOWED_BLOCKS_2

const ALLOWED_BLOCKS_1 = ['core/paragraph','core/image'];
const ALLOWED_BLOCKS_2 = ['core/image'];
function Edit() ...
allowedBlocks={ condition ? ALLOWED_BLOCKS_1 : ALLOWED_BLOCKS_2 }

@jorgefilipecosta
Copy link
Member

Thank you all for the extensive debugging and insights provided, I proposed a fix at #53943.

@gziolo gziolo mentioned this issue Aug 30, 2023
58 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants