useDecorate return default method,not the override in custom in some case

[liangxiwei]

Description
user can add decorate method on Editable,and it's not work with editor's children because it will use default.

Recording
you can add console.log in use-decorate.ts like this:
export const DecorateContext = createContext<(entry: NodeEntry) => Range[]>( () => { console.log("editor's children call.") return [] } )

Sandbox
no
Steps
no
Expectation
no
Environment

• Slate Version: [e.g. 0.63]

Context
not need

[ulion]

I think #4152 removed Children component which caused the editor's children using incorrect decorate function.
useDecorate was fine with Children component which prevent unnecessary re-render of Elements when only the decorate method changed.
But now without Children component, indeed all Elements' and old Children's related code got re-run/render when decorate function updated anyway. performance get worse anyway.

[liangxiwei]

component

yes,u r right

[jaked]

I think this happens because in Editable, the useChildren method is called before DecorateContext.Provider is rendered, so the useDecorate call sees the default value of the context.

A simple fix would be to add a component wrapper in Editable (so useChildren is called inside the render of DecorateContext.Provider).

[ulion]

not really meaningful, since without Children component, useDecorate is indeed useless than before (directly pass the decorate func down to Element). on the other side, changing decorate function is not such a good idea in some cases, it is still too expensive because it almost re-renders all, but in some cases (e.g. remote caret) we do only want a few specific elements got re-rendered.

[jaked]

OK, my suggestion was just to address the regression (decorate is not called on all nodes in the document) in a simple way. I don't think this change had the desired effect; useChildren still runs before DecorateContext.Provider is rendered. To avoid the problem, useChildren needs to be called in the render of a component contained in DecorateContext.Provider.

It sounds like maybe you want to revert the context change? That would of course fix the problem also.

[ulion]

Well, in my fork, the Children element is still there, and I think keeping it may reduce re-render of the children if only the element properties changed. So, generally, I'd like to revert useChildren (restore the Children element) indeed, though it's not my call. And, I would suggest to use a fixed decorate function and resolve the relevant re-render in other way if it applies. As this current issue, it can be fixed in either way, remove useDecorate, or revert to Children element. but only adding a wrapper for editor.children does not mean anything, since useDecorate no longer reduces re-render like before since the Children element is gone, keeping it there but not functional makes no sense. Jake Donham ***@***.***> 于2021年7月25日周日 上午7:24写道：

…

OK, my suggestion was just to address the regression (decorate is not called on all nodes in the document) in a simple way. I don't think this change <d86e534> had the desired effect; useChildren still runs before DecorateContext.Provider is rendered. To avoid the problem, useChildren needs to be called in the render of a component contained in DecorateContext.Provider. It sounds like maybe you want to revert the context change? That would of course fix the problem also. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4277 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABKYHONZUVK2JA6MNLNY73TZNDSBANCNFSM45IPH36Q> .

-- Ulion

[jaked]

Thanks for the explanations!

[jaked]

Another possible fix would be to change useChildren so it wraps each child in a Child component, and call useDecorate etc. inside this component. This would still achieve the goal of #3966 since a separate component would be returned for each child; but useDecorate would be called at the right time.

As far as re-rendering goes, it seem like this approach would be the same as the original Children component.