preact
preact copied to clipboard
React incompatibility: immutable VNodes break in production.
- [ ] Check if updating to the latest Preact version resolves the issue
Describe the bug
Love the project! Migration from React was almost seamless.
Immutable VNodes
/JSX
/ReactNode
seem to break production Preact, the error is not present with preact/debug
, this is also not an issue with normal React, hence a compatibility issue. Our use case was these objects existing inside some immutable structures created with immer's produce
To Reproduce
Sandbox shows simple usecase resulting in error, using immer produce
with zustand
is a pretty standard setup.
Steps to reproduce the behavior:
- See sandbox: https://codesandbox.io/s/preact-zustand-immer-qvx8hx?file=/src/index.js
- Click "update" button, preact errors with read only error, due to
comp
having been immutified thanks toproduce
- Uncomment first line,
import "preact/debug";
refresh and the same update button will now work with no errors. - This logic also works fine in React.
Since it works with debug mode, you can only see the production error:
Codesandbox uses a pretty outdated version of the preact-cli
. Since it's one of their proprietary templates we cannot update it on our own which is unfortunate. I cannot reproduce the error in the latest version of preact-cli
or our more modern vite template.
@marvinhagemeister apologies! Thought I was using the best template. I've used the example one in the issue template and the issue still persists:
https://codesandbox.io/s/modern-preact-zustand-immer-vn7jrd?file=/src/index.js
I didn't do the react version to modify the template as little as possible, but same problem, fails without import "preact/debug"
(this template has preact-cli: ^3.0.0-rc.1
)
Its a slightly different error here, not sure if its because preact is being used rather than preact/compat
. Nevertheless, I'm running Preact 10.17.1
bundled with webpack and get the original error whilst migrating from react.
This doesn't make much sense to me, as preact-cli
(in dev) already includes an import for preact/debug
-- any difference in behavior you see stemming from commenting it out I believe is going to be unrelated to preact/debug
itself.
CodeSandbox has been quite buggy for me for a while, so I attempted to copy/paste your example into Stackblitz as well as run locally and I could not reproduce the issue. It might just be a CodeSandbox-specific issue?
@rschristian @marvinhagemeister I've now produced the same error in Stackblitz! I know this isn't CodeSandbox related, as I discovered the problem in my real project, debugging it down to specifically this simple example.
The reason you're both probably having a hard time reproducing is the vite preset preact()
plugin defaults to preact({ devToolsEnabled: true })
, which means the error doesn't show. I'm guessing something similar is happening for you guys with newer preact-cli
versions as @rschristian mentioned.
Stackblitz repro: (all deps up to date) https://stackblitz.com/edit/vitejs-vite-bd36gl?file=vite.config.ts
"preact": "^10.17.1",
"@preact/preset-vite": "^2.5.0",
"vite": "^4.4.9"
"typescript": "^5.2",
"immer": "^10.0.2",
"zustand": "^4.4.1"
Steps:
- Open the repro, see in
vite.config.ts
devToolsEnabled
explicitly set tofalse
(defaults totrue
) - Click update
<button />
, won't work, open console, see original error:Uncaught (in promise) TypeError: Cannot assign to read only property '__' of object '[object Object]'
- Change to
devToolsEnabled: true
(the same as adding theimport preact/debug
it seems), reload and the error is now gone, now updates fine on click.
Hope this helps!!
To take this full circle (+ I was interested to try out preact-cli), here's how you can reproduce locally with preact-cli
directly:
- Create dir with below 2 files.
-
npm i
-
npm run dev
--> update<button />
will work fine, preact-cli forcefully enablespreact/debug
- Can't find a way to do it non-hackily, but comment out
require("preact/debug")
line 12node_modules/preact-cli/src/lib/entry.js
to disable debug. (setting NODE_ENV=production gets overriden and doesn't disable debug) - Restart
npm run dev
, update<button />
will now fail with same error:Uncaught (in promise) TypeError: Cannot assign to read only property '__' of object '[object Object]'
package.json
{
"private": true,
"name": "preact-immut-test",
"scripts": {
"dev": "preact watch"
},
"dependencies": {
"preact-cli": "^3.5.0",
"immer": "10.0.2",
"preact": "^10.17.1",
"zustand": "4.4.1"
}
}
index.js
import { create } from "zustand";
import { produce } from "immer";
const genComp = () => <p>{Date.now().toString().slice(10)}</p>;
const useBearStore = create((set) => ({
comp: 0,
update: () =>
set(
produce((state) => {
state.comp = genComp();
})
),
}));
export default function App() {
const comp = useBearStore((state) => state.comp);
const update = useBearStore((state) => state.update);
return (
<div class="app">
<div>{comp}</div>
<button onClick={update}>Update</button>
</div>
);
}
Sorry for the long delay.
Looks like this breaks our attempt to set the new parent for a child VNode. preact/debug
is cloning the returned VNode so that it's no longer frozen when it comes time to diff, but this isn't an intentional interaction.
Having to clone objects to work around users potentially freezing them (for purposes which I still don't fully see the value of) feels pretty bad.
Hey @rschristian. Was definitely a weird one, I changed my behaviour as a result.
All that's left here is a divergence from react behaviour, not something that necessarily "should" be done.
Thanks!
I get that, and I hope it wasn't too much of a pain to track down back when you opened the issue (sorry again for it sitting for so long).
However, I don't think there's much we want to do here. How Preact interacts with returned VDOM elements should be an internal thing, and freezing objects (as immer
does) causes this implementation detail to bleed out quite badly -- hardly unique to Preact of course, the same can be said for many APIs.
As the only way to work around this is to start cloning objects (which has obvious & high perf implications), I don't think it makes much sense to support at this time.
Thanks for opening this though.