The RichText __experimentalGetPropsForEditableTreePreparation API break the selection in the editor

[youknowriad]

I'm using the "block-editor" package on third-party project. (asblocks)
and when I add this custom RichText format (it does nothing, it's just an empty format), I can't select text reliably anymore:

import { registerFormatType } from '@wordpress/rich-text';
import './style.css';

const FORMAT_NAME = 'asblocks/caret';

export const settings = {
	title: 'AsBlocks caret',
	tagName: 'mark',
	className: 'asblocks-caret',
	attributes: {
		id: 'id',
		className: 'class',
	},
	edit() {
		return null;
	},
	__experimentalGetPropsForEditableTreePreparation() {
		return {
			carets: [],
		};
	},
	__experimentalCreatePrepareEditableTree() {
		return ( formats ) => {
			return formats;
		};
	},
};

registerFormatType( FORMAT_NAME, settings );

Nothing that If I remove the __experimentalGetPropsForEditableTreePreparation function, the text selection works properly.

Screenshot of broken text selection:

cc @ellatrix

[ellatrix]

__experimentalGetPropsForEditableTreePreparation is to return the result of something selected in a store. Maybe returning a new object on every render is causing a problem?

[msurdi]

I've recently ran into this problem too. But in my case, the selection issue was triggered because of using the provided select function to access the store, for example:

import { registerFormatType } from '@wordpress/rich-text';
import './style.css';

const FORMAT_NAME = 'asblocks/caret';

export const settings = {
	title: 'AsBlocks caret',
	tagName: 'mark',
	className: 'asblocks-caret',
	attributes: {
		id: 'id',
		className: 'class',
	},
	edit() {
		return null;
	},
	__experimentalGetPropsForEditableTreePreparation(select) {

                // Just this will trigger the problem, it's not even necessary to call a selector function for it to happen.                
                select('core/block-editor')  

		return {
			carets: [],
		};
	},
	__experimentalCreatePrepareEditableTree() {
		return ( formats ) => {
			return formats;
		};
	},
};

registerFormatType( FORMAT_NAME, settings );

if instead of using the provided select, I import it like import { select } from '@wordpress/data'; then the problem goes away.

Not sure if this makes any sense at all, I'm not that familiar with the internals of @wordpress/data.

Hope this helps to track down the root issue here. Let me know if I can provide any more information or help in any other way to debug this.

EDIT: I've finally had to undo this workaround, as it causes the format to not be applied consistently (I understand now that not using the provided select is causing the format to not re-render when it should)

[msurdi]

I've just created a pure Gutenberg example, based on one of the existing storybooks.

Here are the steps to reproduce the problem:

1. Check out the reproduce-issue-23428 branch from here
2. Run npm install and npm run storybook:dev
3. Open a browser at http://localhost:50240/?path=/story/playground-block-editor--default
4. Paste (or write) some text
5. Pick any word, and make it a link to https://example.com
6. Try to select text that includes this linked word, from right to left, as shown in the video below.
7. If you comment out the select( 'core/block-editor' ).getBlockName( blockClientId ); in the provided example, you will notice the problem goes away.

Hope this helps. Let me know if there's anything else I can help with.

Thanks!

23428.mp4

[ellatrix]

@msurdi I tried this out and I can't reproduce. I tried this branch: #42596. Could you have a look at that branch?
The video is also not loading for me.

[ellatrix]

@msurdi I do notice a problem that I can recreate consistently. If I try to select across two paragraphs, the selection disappears when I stop the selection.

If I change the code to the following, everything works again as expected:

return {
	carets: select( 'core/block-editor' ).getBlockName( blockClientId ),
};

So it seems to me that this is a excessive rererendering problem, caused by returning a new array on every call (carets: []). I don't think it has anything to do with select( 'core/block-editor' ) cause the above works fine.

I think what you need to do is ensure that __experimentalGetPropsForEditableTreePreparation returns a stable reference that only changes when necessary.

[msurdi]

Hi @ellatrix , thanks for your help with this.

tried this out and I can't reproduce. I tried this branch: #42596. Could you have a look at that branch?

That's not the branch where I modified the example, the branch where the modified example reproducing the problem exists is this one. I think you'll need to clone this other repository as I don't have permissions to create branches in this one.

The video is also not loading for me.

Do you mean the video on my previous comment? I've just passed the link to this issue to several people and they can watch it fine. I can send it to you or upload it in some other way if you still can't.

@msurdi I do notice a problem that I can recreate consistently. If I try to select across two paragraphs, the selection disappears when I stop the selection.

That might be another problem. The problem I'm describing here happens within a single paragraph, as long as it has an anchor in the middle of the selection.

If I change the code to the following, everything works again as expected:

I've just updated this example to just not return anything from this function (well, an empty object). The problem I describe still happens, even with a return {}.

I think what you need to do is ensure that __experimentalGetPropsForEditableTreePreparation returns a stable reference that only changes when necessary.

I've tried this (by making carets a top/module level constant) and the problem is still there. As I've just mentioned above, I'm also returning {} and the problem still happens. It only goes away if I don't make the select call at all.

[ellatrix]

That's not the branch where I modified the example, the branch where the modified example reproducing the problem exists is this one.

I know, I just created one with the same example but to test in the normal setup.

I think I understand the problem now. This API is just not made for returning an object with multiple select results. It's made for only a single result. I'll see what I can do to allow it.

[ellatrix]

@msurdi Does this solve your problem? https://github.com/WordPress/gutenberg/pull/42596/files

[msurdi]

It does solve the problem in the storybook example I provided, so I'm pretty sure it will fix it too in the real application.

I'm trying to get a custom build of this branch into our app so I can confirm 100% it does. But if these changes cause no harm and you want to merge that's good too, I'm pretty confident this solves it.

Thanks a lot!

[ellatrix]

Yes, I'm pretty confident. I wanted to add an e2e test, but it seems incredibly hard to write a test case that consistently fails in trunk. I tried to subscribe the test annotation format to the block editor store, which should cause a lot of re-renders on select, but the tests still pass.

[ellatrix]

So the problem here was subscribing to the block editor store, which will cause the useSelect callback to run a lot (on selection) and return a new object every time for each __experimentalGetPropsForEditableTreePreparation call. This was then returned in keys to the useSelect callback, the result of which no longer being shallow equal to the previous result, so all of RichText re-renders. Normally this should only result in a performance hit, but native selection is really sensitive to the DOM changing. I see the data-rich-text-format-boundary changing when entering and leaving a format. I wouldn't expect selection to normally collapse from this, but maybe this is the problem. The actually DOM tree seems to be stable even with all the re-renders.

[msurdi]

I solves the problem in the example I provided, but I'm still facing the exact same issue on our real use case.

But this is still making things better I think. I'll keep investigating what the problem could be and if it just something else we might be doing wrong on our application side.

If I managed to get another minimal example I'll reopen the issue.

Thanks again for your help.

[msurdi]

There's something extremely confusing to me in all of this, and it is that the problem exists only when selecting text from right to left. If you do the selection for left to right it works fine.

[ellatrix]

@msurdi I'm pretty sure that's because formatCarets in your format is returning a new array an each call.

https://github.com/Automattic/p2/blob/bb1ad221fdf02227a3f8fdb7f47e29c63af666f8/plugins/p2/src/client/core/editor/collab/lib/formats/selections/index.ts#L208

[msurdi]

Yes, I was just looking at this, and found out that the root issue of that unnecessary creation of new arrays (despite the memoization) was because of unnecessary changes of the peers dependency, up in the call stack.

I've already done a small refactor of what we return from __experimentalGetPropsForEditableTreePreparation to limit it to the minimum necessary for it to work, and can confirm this solves the problem.

Thanks!