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

infinite rerenders when using useInstanceId in template-part preview #63713

Closed
2 tasks done
silaskoehler opened this issue Jul 18, 2024 · 9 comments
Closed
2 tasks done
Labels
Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended

Comments

@silaskoehler
Copy link

Description

I get infinite rerenders when i use useInstanceId() in my custom block and save it to an attribute if it changes.
for testing purpose i created a block using pnpx @wordpress/create-block@latest --variant dynamic and change the edit.js to this:

import { useBlockProps } from '@wordpress/block-editor';
import { useInstanceId } from '@wordpress/compose';

export default function Edit( { setAttributes, attributes } ) {
	const { uniqueId } = attributes;

	const instanceId = useInstanceId( Edit );

	useEffect( () => {
		console.log( uniqueId, instanceId );

		setAttributes( {
			uniqueId: instanceId,
		} );
	}, [ uniqueId ] );

	return <div { ...useBlockProps() }>uniqueId: { uniqueId }</div>;
}

In the template preview in the site editor (/wp-admin/site-editor.php?postType=wp_template) my block flickers and in the console I see infinite rerenders. The error only occurs when my block is in a Temaplte part.

Am I doing something wrong or is there a bug because it does a render every time I save the attribute. He doesn't do that in the block editor.

Step-by-step reproduction instructions

  1. Use default Theme and WP 6.6
  2. create a block using @wordpress/create-block@latest
  3. edit the edit.js as described above
  4. go to the template preview
  5. go to the console

Screenshots, screen recording, code snippet

Bildschirmaufnahme.2024-07-18.um.16.50.43.mov

Environment info

  • WordPress Studio
  • WordPress 6.6
  • PHP 8.1
  • Theme: Twenty Twenty-Four
  • no Plugins besides the custom block plugin

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@silaskoehler silaskoehler added the [Type] Bug An existing feature does not function as intended label Jul 18, 2024
@talldan
Copy link
Contributor

talldan commented Jul 19, 2024

It's not something I can reproduce. I think your effect dependency should be instanceId, btw.

@Mamaduka Mamaduka added the Needs Technical Feedback Needs testing from a developer perspective. label Jul 19, 2024
@silaskoehler
Copy link
Author

sorry for missing the useEffect dependency. Also i forget to import useEffect.
I updated it to:

import { useBlockProps } from '@wordpress/block-editor';
import { useInstanceId } from '@wordpress/compose';
import { useEffect } from '@wordpress/element';

export default function Edit( { setAttributes, attributes } ) {
	const { uniqueId } = attributes;

	const instanceId = useInstanceId( Edit );

	useEffect( () => {
		console.log( uniqueId, instanceId );

		setAttributes( {
			uniqueId: instanceId,
		} );
	}, [ uniqueId,instanceId ] );

	return <div { ...useBlockProps() }>uniqueId: { uniqueId }</div>;
}

you can find the plugin here: https://github.com/Taigistal/wp-bug-preview-template-preview-rerenders
Also i created a playground here:

please tell me, if you can reproduce the error.

@talldan
Copy link
Contributor

talldan commented Jul 19, 2024

Thanks for the playground link, that's super helpful.

I can reproduce it now just on that screen, so it must be related to the previews. 🤔

It also only happens with the Template previews in the listing, not the Template Parts 🤔 🤔

@talldan
Copy link
Contributor

talldan commented Jul 19, 2024

It's definitely a weird one.

I think preferably try to avoid setting attributes like this in blocks, it can cause some unexpected issues.

If you do need to, you can try something like this:

/**
 * WordPress dependencies
 */
import {
	useBlockProps,
	store as blockEditorStore,
} from '@wordpress/block-editor';
import { useInstanceId } from '@wordpress/compose';
import { useEffect } from '@wordpress/element';
import { useDispatch } from '@wordpress/data';

export default function Edit( { setAttributes, attributes } ) {
	const { uniqueId } = attributes;
	const { __unstableMarkNextChangeAsNotPersistent } =
		useDispatch( blockEditorStore );

	const instanceId = useInstanceId( Edit );

	useEffect( () => {
		if ( ! uniqueId ) {
			__unstableMarkNextChangeAsNotPersistent();
			setAttributes( {
				uniqueId: instanceId,
			} );
		}
	}, [ instanceId, uniqueId ] );

	return <div { ...useBlockProps() }>uniqueId: { uniqueId }</div>;
}

I'm guessing you only need to set the uniqueId when it doesn't exist already, because otherwise you're changing the id quite a lot. If you need to setAttributes in an effect, I'd also suggest using __unstableMarkNextChangeAsNotPersistent. While it's not a stable Gutenberg API, it helps to avoid unwanted undo levels in the editor.

Also fair warning that useInstanceId is not really the right tool for the job here. It won't create a uniqueId the way you think it will. Each time the editor loads it starts counting from zero again, so I think you can have duplicate ids if insert this block in two different posts and then copy it from one into the other. It'd be better to use something that generates a uuid.

@silaskoehler
Copy link
Author

silaskoehler commented Jul 20, 2024

In my case i needed a unique Id for each Instance of my block so i generated one with the package uuid. My problem then was that this Id is not unique if i duplicate the block.

I found a similar case here in the query block. I'm not sure if i understand why the Id from useInstanceId isn't unique anyway? Has it just to be unique for its innerBlocks?

Is it bad practice to work with a unique id for each block instance anyway? Should you avoid it when ever you can?

Is there an easier way to make sure that i have a unique Id for each block instance? Is a custom store the solution? So that I assign IDs and check for duplicates in the store? That seems a bit too complicated to me, doesn't it?

So many questions 😅. I hope not too many stupid questions. Thank you for your thoughts.

@silaskoehler
Copy link
Author

An addition: Now i realize that the solution above (with __unstableMarkNextChangeAsNotPersistent ) only works when i set the attribute if it is false

if ( ! uniqueId ) {

but not if i set it if it not the same as the instanceId.

if ( !uniqueId || uniqueId !== instanceId ) {

But that's what i need to get a unique Id when duplicating the block.

@talldan
Copy link
Contributor

talldan commented Jul 22, 2024

I found a similar case here in the query block.

That's interesting. I wouldn't be surprised if that causes some bugs. It looks like it's been there since the inception of the query block (#22199)

I'm not sure if i understand why the Id from useInstanceId isn't unique anyway?

useInstanceId is suitable for what I'd call 'in memory' ids, but not ids that are saved.

In the block editor, useInstanceId is mostly used in the UI that makes up the editor chrome, often for the html id of form elements (e.g. in the block inspector). This HTML isn't saved anywhere, each time you load the editor the UI is created from scratch, and the ids are generated anew. There will never be any clashes because the counter is correct within a session (the html is fully controlled by react and useInstanceId when re-rendered).

The problem with using it for a block attribute is that ids end up being saved, and that means the id is now being used somewhere outside the control of useInstanceId. You can more easily get clashes because the counter resets to 0 (or maybe it starts at 1, not sure) each time you load the editor, so the generated ids are often in the same range.

UUIDs are a better solution since there's higher entropy, but still not perfect, there's the issue with duplicating blocks, or a pattern with an id within it being used twice in the same post.

Using dynamic rendering (A PHP callback for your block) is probably the best current solution. The HTML Tag Processor is one way to enhance a block's HTML.

The general problem of unique ids has been discussed a bit in #17246, and unfortunately there haven't been any solutions (though I do have some new ideas).

@silaskoehler
Copy link
Author

Thanks for your helpful explanation!

In my specific case i use filters to add responsive breakpoint settings to the core/columns block. For that i need the unique Id. My new solution is to use the Instance ID in the editor and a uuid generated by wp_generate_uuid4 inside the render_block filter. I dont save the Id to the attributes anymore. It works.

So one last question i think:

I use useInstanceId in both filters here and it is the same Id per Block. But why?
Are the objects BlockListBlock and BlockEdit identical? is useInstanceId linked to the block in other ways then the object i put into the function?

// editor.BlockListBlock
const withClientIdClassName = createHigherOrderComponent(
	( BlockListBlock ) => {
		return ( props ) => {
			const instanceId = useInstanceId( BlockListBlock );

			// ... 
		};
	},
	'withClientIdClassName'
);

addFilter(
	'editor.BlockListBlock',
	'luther-responsive-columns/with-client-id-class-name',
	withClientIdClassName
);
// editor.BlockEdit
const LutherResponsiveColumnsBlockEdit = createHigherOrderComponent(
	( BlockEdit ) => {
		return ( props ) => {
			const instanceId = useInstanceId( BlockEdit );

			// ...
		}
	},
	'withInspectorControl'
);

addFilter(
	'editor.BlockEdit',
	'luther-responsive-columns/with-inspector-controls',
	LutherResponsiveColumnsBlockEdit
);

@talldan
Copy link
Contributor

talldan commented Jul 25, 2024

Are the objects BlockListBlock and BlockEdit identical?

I think that's usually the case now, yes. Older versions of blocks (apiVersion of 1) used to have an automatically added wrapper, which the BlockListBlock filter could be used to modify. For newer blocks that don't have that the BlockListBlock filter seems to be quite duplicative of BlockEdit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants