renderElement does not support React Hooks

[bigaru]

Description

It seems that the function renderElement from packages/element/serialize.js is not able to process React Hooks. As soon as a React hook is used inside save function, the error Invalid hook call. is thrown.

I double-checked this error against React's probable causes:

1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app

However, everything was fine. After using the debugger, I came to the conlusion that the function renderElement does not support the React hooks.

Furthermore, on the top of the serialize.js file is mentioned that the some parts are based on fast-react-render (Btw which is outdated and they even recommend to use vanilla React). That is why I strongly suspect that the serializer is not able to handle React hooks.

Step-by-step reproduction instructions

1. npx @wordpress/create-block example
2. modify index.js as follows:

import { useRef } from '@wordpress/element'

registerBlockType(metadata.name, {
	edit: Edit,
	save: () => {
		const ref = useRef()
		return <div></div>
	},
})

1. npx wp-env start && npm start
2. edit any post
3. use the block example and save the post
4. reload the page and open web console

Screenshots, screen recording, code snippet

No response

Environment info

• @wordpress/scripts 23.7.0
• @wordpress/element 4.13.0
• @wordpress/env 4.9.0

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

[talldan]

@bigaru What's the reason for implementing a React hook in a save function?

In terms of the save function, I think that this is that it isn't a bug but a feature. I wouldn't like to see support for hooks as they shouldn't be used in a block save function. The save function should be pure so that block validation doesn't fail. Hooks only introduce impurity.

[bigaru]

Hi @talldan

I see your point with block validation. However, not all hooks make your code impure. The counter-examples are useId and useContext:

const ThemeContext = createContext('#000')

const DeeplyNestedElement = () => {
	const themeColor = useContext(ThemeContext)
	return <div style={{ background: themeColor }}>Foo Bar</div>
}

const Root = () => (
	<ThemeContext.Provider value="#000">
		<DeeplyNestedElement />
	</ThemeContext.Provider>
)

Currently the behavior between ClassComponent and FunctionalComponent is inconsistent:

// works fine in save
class FooComponent extends React.Component {
	constructor(props) {
		super(props)
		this.value = 'FooBar'
	}

	componentDidMount() {
		this.setState({ value: 'NewFoo' })
	}

	render() {
		return <div {...this.props}>{this.value}</div>
	}
}

// fails in save
const FooComponent = (props) => {
	const [value, setValue] = useState('FooBar')
	useEffect(() => setValue('NewFoo'))
	return <div {...props}>{value}</div>
}

Even if the developer writes pure code. There are a lot of react libraries which use hooks underneath. For example, the code below is pure (at least from developer POV). Since the pie-chart library uses hooks, it will fail in save function.

import { PieChart } from 'react-minimal-pie-chart'

const Chart = ({ attributes }) => {
	const { value1, value2, } = attributes

	return (
		<PieChart
			data={[
				{ title: 'Uno', value: value1, color: '#E38627' },
				{ title: 'Dos', value: value2, color: '#C13C37' },
			]}
		/>
	)
}

Overall, it still seems to me that this behavior is not intentional. Furthermore I found a related issue #15873 .

[talldan]

The counter-examples are useId and useContext

If the goal of useId is for a unique id, then there's no way it would be guaranteed to generate the same id on each render of save. If it did, it could just as easily be implemented as a normal function, but it would have to be more like a hashing function, so probably not useful. Ids usually need to be generated in the block edit to work correctly.

I can see how useContext is frustrating if you're using a component library. The problem with using a component library like that is that there's no guarantee that it won't cause a block invalidation. For the same reason, none of the core blocks use @wordpress/components or any other component library for save functions. I understand that you may have a more advanced use case. If you need to use a library you'd be better off looking for a library that has purely presentational components and making sure you have good regression testing in place for upgrades.

There is possibly a future situation where it may need to work differently, which is to support block hydration (#38224).

Overall, it still seems to me that this behavior is not intentional. Furthermore I found a related issue #15873 .

It looks like the same discussion on that issue as here though. This can definitely be considered a bug in renderToString, but at the same time there's currently no need to support hooks in a block save function.

[talldan]

Ids usually need to be generated in the block edit to work correctly.

Worth mentioning that even then that can cause issues - a unique id might be in a reusable block, and that reusable block could be inserted twice, and then there would be duplicate ids.

The only safe option is a dynamic block.

[bigaru]

My main motivation is that I can use the same react component inside the save function and block hydration. 😄

It is good to see that there is ongoing progress for a better approach of block hydration:

• Universal blocks and block hydration #38224
• Tracking issue: Custom Elements Hydration 🧩 block-interactivity-experiments#39

I understand your reasoning

This can definitely be considered a bug in renderToString, but at the same time there's currently no need to support hooks in a block save function.

So it's up to you, whether you like to keep this issue open as a reminder.

[talldan]

My main motivation is that I can use the same react component inside the save function and block hydration. 😄

Interesting. Good to hear you're exploring this. Have you found workaround for the renderToString problem? I do think hydration would be a good reason for exploring a fix. I don't know if it's on the radar of the contributors that have discussed block hydration so far (cc @michalczaplinski), and I don't know what priority it would be. It may be something that's still a low priority for the maintainers, so a PR fixing this would have to be a contribution.

If there were a fix, I do also wonder it could be worth thinking about ways to warn developers of normal static blocks about the pitfalls of using hooks or other kinds of impurity. Maybe block validation warnings are already enough, but the trouble is that a validation issue may not happen until much later.

[bigaru]

Have you found workaround for the renderToString problem?

I don't want to clutter my code with if-else. Probably the simplest solution is, to render an empty <div> in save function and later on to call ReactDom.render instead of hydrate with a js-script.

If there were a fix, I do also wonder it could be worth thinking about ways to warn developers of normal static blocks about the pitfalls of using hooks or other kinds of impurity.

Maybe we could create a custom eslint plugin for that.

[luisherranz]

Our current idea is to replace the real React hooks with our own pure implementation. For example, useState could be:

const noop = () => {};

// Export of useState in `@wordpress/element` for the `save.js` entry points
export const useState = (defaultValue) => [defaultValue, noop];

That way, we don't have to fix the serializer, and the same export different code in different contexts mechanism could be used for other purposes:

• Export different code depending on the context (Edit, Save, or View) block-interactivity-experiments#11

But we are also open to other ideas 🙂

[bigaru]

I looked at the code and it seems very interesting. I like your solution, it will definitely fix my problem 😁

However, when I'm thinking about long term maintenance, I have concerns:
I've to admit that I have no clue with about the code and the intent of the serializer. As I understand, the serializer is based on the fast-react-render and fast-react-render was created before react 16. Therefore, I assume this problem occurs because react 17 offers new features like hooks in the meantime.

That is why I'm wondering, is it possible to use the plain react render method underneath and make those customizations on top of it? So when the next react version is released, the serializer will support all new features out of the box.

[luisherranz]

Yes, that's another option @gziolo and I briefly explored a while ago. If you want to give it a try, please do! 🙂

These are the primary unit tests that should pass: https://github.com/WordPress/gutenberg/blob/trunk/packages/element/src/test/serialize.js

[jlandrum]

I like the idea of pure hooks, particularly of the ones we know can be mimick'd without true react. Has anyone moved forward on this? If not - I might see about giving it a shot (if I can find the time) - would the intent be to replace the already exposed hooks, or maybe make it something like @wordpress/save-element or @wordpress/element/save?

I actually came across this in making some components specifically for allowing me to use the same React component both for editor and save - my blocks are all isolated with a prop - save. edit.js does not set it, but save.js does.

Originally I had a context used like <State save={save}>... and two components, <SaveOnly> and `

I ended up just having a top-level object hold the state and removed the context, making just set that object's save property to true/false depending. It works, but it's dirty.

I also have to pass blockProps in from edit.js/save.js, given that useBlockProps() also causes this error and conditional hooks are "bad". I mean, they are bad but in quotes because I don't think in the context of Gutenberg they'd actually cause any issues. I easily could just keep my components wrapped in a <div {...useBlockProps()... or <div {...useBlockProps.save()}... but I personally prefer to keep those functions being direct - so that the root node is determined by the component itself and not the save/edit functions.