preact icon indicating copy to clipboard operation
preact copied to clipboard

React incompatibility: immutable VNodes break in production.

Open zakstucke opened this issue 11 months ago • 5 comments

  • [ ] 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:

  1. See sandbox: https://codesandbox.io/s/preact-zustand-immer-qvx8hx?file=/src/index.js
  2. Click "update" button, preact errors with read only error, due to comp having been immutified thanks to produce
  3. Uncomment first line, import "preact/debug"; refresh and the same update button will now work with no errors.
  4. This logic also works fine in React.

Since it works with debug mode, you can only see the production error: image

zakstucke avatar Sep 18 '23 06:09 zakstucke

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 avatar Sep 18 '23 13:09 marvinhagemeister

@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.

zakstucke avatar Sep 18 '23 13:09 zakstucke

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 avatar Sep 19 '23 04:09 rschristian

@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:

  1. Open the repro, see in vite.config.ts devToolsEnabled explicitly set to false (defaults to true)
  2. 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]'
  3. Change to devToolsEnabled: true (the same as adding the import preact/debug it seems), reload and the error is now gone, now updates fine on click.

Hope this helps!!

zakstucke avatar Sep 19 '23 05:09 zakstucke

To take this full circle (+ I was interested to try out preact-cli), here's how you can reproduce locally with preact-cli directly:

  1. Create dir with below 2 files.
  2. npm i
  3. npm run dev --> update <button /> will work fine, preact-cli forcefully enables preact/debug
  4. Can't find a way to do it non-hackily, but comment out require("preact/debug") line 12 node_modules/preact-cli/src/lib/entry.js to disable debug. (setting NODE_ENV=production gets overriden and doesn't disable debug)
  5. 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>
  );
}

zakstucke avatar Sep 19 '23 06:09 zakstucke

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.

rschristian avatar Apr 19 '24 01:04 rschristian

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!

zakstucke avatar Apr 19 '24 10:04 zakstucke

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.

rschristian avatar Apr 19 '24 21:04 rschristian