Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to upgrade and getting errors #766

Closed
hipstersmoothie opened this issue Jul 1, 2024 · 26 comments · Fixed by #781
Closed

Trying to upgrade and getting errors #766

hipstersmoothie opened this issue Jul 1, 2024 · 26 comments · Fixed by #781

Comments

@hipstersmoothie
Copy link
Contributor

I'm trying to upgrade to the latest beta and I get this error when starting my app

TypeError: Cannot assign to read only property 'page' of object '#<Object>'
    at /Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2334:23
    at wakeChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:1992:5)
    at initializeModelChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2186:9)
    at resolveModelChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2129:5)
    at resolveModel (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2716:5)
    at processFullRow (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:3338:9)
    at processBinaryChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:3471:7)
    at progress (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:3558:5)
Cannot process SSR TypeError: Cannot assign to read only property 'page' of object '#<Object>'
    at /Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2334:23
    at wakeChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:1992:5)
    at initializeModelChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2186:9)
    at resolveModelChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2129:5)
    at resolveModel (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:2716:5)
    at processFullRow (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:3338:9)
    at processBinaryChunk (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:3471:7)
    at progress (/Users/andrewlisowski/Documents/fwoosh-3/node_modules/.pnpm/react-server-dom-webpack@19.0.0-rc.0_react-dom@19.0.0-rc.0_react@19.0.0-rc.0__react@19.0.0-rc_p6bqj3dy2a2jctw2w7d4lhay7m/node_modules/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js:3558:5)

Should I report this upstream? This all works on the non-beta version

My repo is here. The upgrade work is on the upgrade-waku branch.

Set up steps:

pnpm I
pnpm turbo run build
cd examples/react
pnpm fwoosh dev

Navigate to localhost:3000/bench

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

The build step pnpm turbo run build failed for me (a different pnpm version?), but I found you have two react-server-dom-webpack packages.

   "react-server-dom-webpack": "19.0.0-beta-e7d213dfb0-20240507",
   "react-server-dom-webpack": "19.0.0-rc.0",

Can you make sure to use rc.0 for everything?

I'm not sure if it relates to the error though. The monorepo setup seems tough.

@hipstersmoothie
Copy link
Contributor Author

hipstersmoothie commented Jul 1, 2024

I think the real problem is the errors can be really hard to tie to something that caused the error

I fixed up those errors and still get the same runtime error

in Markdown.tsx I changed this line tableCell: TableCell, to tableCell: "td", and the render works.

So it seems like in this case it's having trouble rendering the client component

CleanShot 2024-06-30 at 19 22 59@2x

@hipstersmoothie
Copy link
Contributor Author

In all cases the bug is complaining about a prop. In the case of the table cell it doesn't like node

@hipstersmoothie
Copy link
Contributor Author

hipstersmoothie commented Jul 1, 2024

Okay that prop was getting passed to the dom, and that seems to break something

If I omit the prop the app renders fine

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

So it seems like in this case it's having trouble rendering the client component

interesting as recent changes are basically for the server.
yeah, it's pretty hard to debug.

@hipstersmoothie
Copy link
Contributor Author

hipstersmoothie commented Jul 1, 2024

All errors seem to be when passing objects to client components

just commenting as I see things in case any future dev hits this, this is probably a problem upstream.

I have been getting into situations where I get not found and no output at all though. refreshing once or twice fixes it

CleanShot 2024-06-30 at 20 34 54@2x

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

All errors seem to be when passing objects to client components

from server components? So, is it easy to create a very small reproduction?

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

this is probably a problem upstream.

oh, okay.

I have been getting into situations where I get not found and no output at all though. refreshing once or twice fixes it

Yeah, i've also seen it before. Not sure how it causes it yet.

@hipstersmoothie
Copy link
Contributor Author

hipstersmoothie commented Jul 1, 2024

from server components? So, is it easy to create a very small reproduction?

yeah


I can try to pare it down

@hipstersmoothie
Copy link
Contributor Author

Okay got it down to just 1 file. hopefully I didn't miss any setup steps

Here is the repo

Setup should be

pnpm I
cd packages/fwoosh
pnpm waku dev

Then navigate to http://localhost:3000/iframe/test

CleanShot 2024-06-30 at 22 58 39@2x

When I remove the context it seems to work (but I want the context 😅 )

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

I was hoping to run it on StackBlitz:
https://stackblitz.com/github/hipstersmoothie/waku-repro/tree/main/packages/fwoosh

But, it can't install deps in the subfolder. 😅

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

Hm, I can reproduce the error locally.

When I remove the context it seems to work (but I want the context 😅 )

But, it sounds like we could make a smaller repro, even without monorepo.

We have an e2e test with e2e/fixtures/ssr-context-provider. I wonder how it's different.

@hipstersmoothie
Copy link
Contributor Author

i've simplified it further too

I'll remove all the monorepo stuff if that helps

@hipstersmoothie
Copy link
Contributor Author

monorepo stuff removed

@hipstersmoothie
Copy link
Contributor Author

Okay simplified it as much as I think I can. It seems to have to do with the re-use of page and story. If I move the empty object to the args it works.

@hipstersmoothie
Copy link
Contributor Author

doing this also works

CleanShot 2024-06-30 at 23 26 29@2x

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

Okay simplified it as much as I think I can.

This is great!!! But, I wish more if it can fit in e2e fixture.
src/pages/_layout.tsx isn't necessary.
I think it can be just one page src/pages/index.tsx.
I wonder if src/Button.stories.tsx is required for the reproduction.
Do we need both Decorator.tsx files?

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

Hmm, one decorator is a server component and the other is a client component.

@hipstersmoothie
Copy link
Contributor Author

Yeah the decorators are needed and the way I compose them is too

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

I think it can be as simple as:

<PageContextProvider value={page}>
  <ClientHolder page={page}>
    <ServerHolder page={page}>
      <ClientButton />
    </ServerHolder>
  </ClientHolder>
</PageContextProvider>

Or something like that. (Well "simple" might be subjective)

@hipstersmoothie
Copy link
Contributor Author

i'm finding I can only repro if the loop is there

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

So, it looks like when we pass page object to a client component, the page object got frozen (?) and the error occurs.

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

i'm finding I can only repro if the loop is there

Oh, really. That's interesting.

@hipstersmoothie
Copy link
Contributor Author

I was wrong. what's important is the order of the decorators

CenteredDecorator > A11yDecorator (doesn't work)
A11yDecorator > CenteredDecorator (works)

@hipstersmoothie
Copy link
Contributor Author

hipstersmoothie commented Jul 1, 2024

example is just this now

import { StoryContextProvider } from "../../components/Context";
import A11yDecorator from "../../A11yDecorator";
import CenteredDecorator from "../../CenteredDecorator";

export default function Iframe() {
  const page = {};

  return (
    <StoryContextProvider value={{ page }}>
      <CenteredDecorator page={page}>
        <A11yDecorator page={page}>
          foo
        </A11yDecorator>
      </CenteredDecorator>
    </StoryContextProvider>
  );
}

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2024

Nice! We could further reduce files and lines, but it's pretty understandable now. Thanks.

So, I did this change:

diff --git a/package.json b/package.json
index 8ad6a33..763059d 100644
--- a/package.json
+++ b/package.json
@@ -3,9 +3,9 @@
   "version": "0.1.0",
   "private": true,
   "dependencies": {
-    "react": "19.0.0-rc.0",
-    "react-dom": "19.0.0-rc.0",
-    "react-server-dom-webpack": "19.0.0-rc.0",
+    "react": "19.0.0-beta-e7d213dfb0-20240507",
+    "react-dom": "19.0.0-beta-e7d213dfb0-20240507",
+    "react-server-dom-webpack": "19.0.0-beta-e7d213dfb0-20240507",
     "waku": "^0.21.0-beta.0"
   },
   "devDependencies": {

And, it works. So, it looks like an upstream issue. You want to find the exact PR in React repo that causes this error and see if it's intentional.

himself65 pushed a commit to himself65/waku that referenced this issue Jul 10, 2024
I was hoping to use `-rc.0` for a while, but it has a bug and
`-rc.<num>` has not been maintained. Let's use nightly rc version.

close dai-shi#766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants