This post comes in three parts. In the first, I present the problem I was facing when upgrading a site that was running on WordPress 5.2.x and no Gutenberg plugin. The second part is about the solution I found, which I think is no more than a workaround fix, actually. In the third part, I will throw some questions at you, hoping for feeback, maybe even issue reports relate to that, your thoughts on how the Block Editor code changed from WordPress 5.2 to 5.3…
Encounter of the Unexpected Kind
For a client project, we don’t use the Gutenberg plugin, which means that we only rely on the Block Editor features that are shipped with WordPress itself. Until last week, we were always using the most recent WordPress 5.2.x release.
When we upgraded to some RC version of 5.3, it all seemed good. There were some deprecation warnings in the console, but all in all it seemed to work. Until you want to publish a post.
Hm, that sucks. But the error related to React hooks is rather interesting. Well, and the fact that we don’t use any hooks. So I went digging into the code. Both our codebase, but also WordPress (or rather the Gutenberg packages included in WordPress).
Where Does It Break in Our Code?
Using a debugging technique that I would call binary-search-powered comment-out—build—confirm, I was able to find that there were two pieces in our codebase that were triggering the above error(s). This explains the blue bubble with the 2 in it. The two coe fragments that broke were two items that we registered for the Publication Checklist module, which you might know as the basis for what we now have in Altis.
Now, we have over a dozen checklist items, and they are all fairly simple an also similar. So what mad thos two special?
Turns out the specialty is that they are using withDispatch. To better illustrate the issue at hand, I will share a slimmed-down version of the code. But first I would need to explain how the Publication Checklist works.
The Publication Checklist
Anyone can register a checklist item by hooking into a custom JavaScript action that the checklist module fires. This action passes on a callback, registerItem, that allows you to register a custom item with the module’s store. Such an item is an object literal that has to have a render prop, that the checklist then executes to, well, render the item content. Usually, this render prop is a reference to some React component.
Now, here is a minimum working example of the code, including one of the two checklist items. We start with Checklist component:
import ChecklistItem from './ChecklistItem';
const Checklist = ( { baseClassName, items } ) => {
return (
<ul className={ `${ baseClassName }__items` }>
{ items.map( ( item ) => (
<ChecklistItem
key={ item.name }
baseClassName={ baseClassName }
{ ...item }
/>
) ) }
</ul>
);
};It takes the entirety of registered items, an maps each item to a dedicated React component. This is for performance reasons, as well as easier debugging or at least understanding when looking through the created component tree.
That ChecklistItem is a class-based component, but let’s just see what the render method is all about:
render() {
const {
baseClassName,
name,
render,
status,
} = this.props;
return (
<li className={ `${ baseClassName }__item ...` }>
{ render( {
renderStatusIcon: this.renderStatusIcon,
status,
} ) }
</li>
);
}So this component doesn’t do any moe than calling the render prop, which actually is a render prop, resulting in anything that React is able to render. This could be a JSX expression, a string, null, or an array of one or more of these things. The callback passed as render receives the instance-bound renderStatusIcon function an the current status of the item.
Well, and this is where things break. But we’d have to see in another place why that is…
A Custom Publiation Checklist Item
For one of the two breaking publication checklist items, the render property in the item object, which is then passed on to the CheklistItem component from before, is a referene to the following container:
import Item from '../components/Item';
// Some other imports...
export const mapDispatchToProps = ( dispatch ) => {
const { clientId } = getBylinesBlock();
const {
closePublishSidebar,
openGeneralSidebar,
} = dispatch( 'core/edit-post' );
return {
openSidebar() {
openGeneralSidebar( 'edit-post/block' );
closePublishSidebar();
selectBlock( clientId );
},
};
};
export default withDispatch( mapDispatchToProps )( Item );Basically, this is just a wrapper for the Item component (see first line), and it injects a new callbak prop, openSidebar to be used by the component. An this Item component is fairly simple:
const BylineItem = ( {
openSidebar,
renderStatusIcon,
status,
} ) => {
const content = status === 'COMPLETED'
? __( 'Article byline', 'tfpress' )
: (
<Button
className="button-link"
onClick={ openSidebar }
>
{ __( 'Article byline missing', 'tfpress' ) }
</Button>
);
return (
<Fragment>
{ renderStatusIcon() }
{ content }
</Fragment>
);
};This component renders a status icon and some text, both based on the current status of the item, and passes the openSidebar prop to a core Button component. That’s it.
That all doesn’t really seem to be an issue, does it?
Where Does It Break in WordPress?
As mentioned before, the only difference I coul find between the two checklist items that break and the rest of the items is that the former use withDispatch. There are other items using compose and one or more other higher-order components (HOC), but none uses withDispatch.
So I went looking what might have changed in the @wordpress/data package. … A lot!
@wordpress/data
The whole HOC has been refactored, and now uses both custom and default React hooks. Here is the (slightly reformatted) export of withDispatch:
const withDispatch = ( mapDispatchToProps ) => createHigherOrderComponent(
( Original ) => ( ownProps ) => {
const mapDispatch = ( dispatch, registry ) => {
return mapDispatchToProps(
dispatch,
ownProps,
registry
);
};
return <Original
{ ...ownProps }
{ ...useDispatchWithMap( mapDispatch, [] ) }
/>;
},
'withDispatch'
);In this code, useDispatchWithMap is a custom React hook that itself makes use of no less than four more hooks, including useRef, useMemo, use(Layout)Effect, and the custom useRegistry.
The Rules of Hooks
As one of the error messages says, there are certain rules around how you can or cannot use React hooks. Part of one of the rules is that you cannot call a React hook inside a class component.
Now, the only class component in the code I shared before is the ChecklistItem component. So this is where I started trying to fix this.
How I (Hopefully) Fixed This
Publication checklist items are usually about certain aspects of the current post. And while editing a post, some if not all of these aspects might change. That’s why publication checklist items are subject to change, too. Therefore, I decided to make the ChecklistItem component a class component, because that way I could define a smart renderStatusIcon instance method that I can pass along to the render prop (or rather: whatever React component is referened by that).
What I did could be a long post in an of itself, but in essence I tried a lot of different things that I thought will solve the issue of the invalid hook call. Some of that on a call with Dan, Kevin, Onyeka and Than. Thanks again for your time!
How to Error!
I extracted the renderStatusIcon function to the file scope, and made ChecklistItem a function component. Error!
Instead of calling the render method inside the JSX expression in the function body, I turned this into an IIFC, an immediately-invoked function component ;). Meaning I replaced render() with (() => render())(). Error!
In the checklist item container, I replaced the export of withDispatch( mapDispatchToProps )( Item ) with a wrapped version: ( props ) => withDispatch( mapDispatchToProps )( Item )( props ). Yes, I know, this is stupid. And also: Error!
I also replaced the Item component reference with a wrapped version: ( props ) => <Item { ...props } />. Because why not? Error! Oh, right.
The Fix
The last thing I did, which I thought was even more stupid than what I tried before was aliasing the render prop to some capitalized name and then rendering out a JSX expression. So instead of this:
render() {
const {
baseClassName,
name,
render,
status,
} = this.props;
return (
<li className={ `${ baseClassName }__item ...` }>
{ render( {
renderStatusIcon: this.renderStatusIcon,
status,
} ) }
</li>
);
}I now have this:
render() {
const {
baseClassName,
name,
render: Content,
status,
} = this.props;
return (
<li className={ `${ baseClassName }__item ...` }>
<Content
renderStatusIcon={ this.renderStatusIcon }
status={ status }
/>
</li>
);
}… It worked.
Now, reading the Rules of Hooks again, I see that it’s stated that on should not call Hooks from regular JavaScript functions. Hm, OK. I didn’t get that before. But I also don’t really understand this—it must have something to do with how React hooks work internally, and how React treats a JSX expression differently than calling a callback that then renders something for React.
Questions
OK. I have a lot of questions, and I hope that some of you might be able to povide some answers. Or maybe even more questions. 🙂
Did I do something (obviously) stupid here?
Did anyone else encounter this, or a similar error? Maybe in Gutenberg context, maybe plain React (with hooks)?
Is there some kind of rule, which I don’t know about, that one should never execute render props as a plain JavaScript function call? Meaning should one always alias those to some capitalized constructor of a new custom React component? If so, why not use a capitalized prop in the first place? Gutenberg is not doing that too, for example, the edit and save props of a block. I didn’t look how those components are actually rendered, but if these function references are just called as a regular function, this should break, too, if anyone wraps their edit component in withDispatch. I do believe that a lot of us did that before, so if there are no issues to be seen there, then I guess Gutenberg renders both edit and save as custom React components.
Is this change to withDispatch, and potentially other components, not a breaking change? I mean, the public API did not change, and the behavior of both the component and the withDispatch higher-order component did not change, too. But the context in which they can be used changed!
If my API says, in plain English, “Please give me callback to render whatever you want.”, I wouldn’t want to need to know what kind of callback that is, right? I don’t care if it uses hooks or not. Is that maybe something that should be fixed in React?
Is there something that we should take out of this lesson an make it a part of the Engineering handbook?
Do you have any related questions?
Thanks!
I hope this is useful for anyone.
Leave a Reply