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

fix: stop sending ssr hydrate flag when the --with-ssr flag is absent #567

Closed
wants to merge 3 commits into from

Conversation

Aslemammad
Copy link
Contributor

While working on #552

I found out that the ci was giving hydration error while the withSSR was false. The reason was that data-hydrate was always true.

Copy link

vercel bot commented Mar 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
waku ✅ Ready (Inspect) Visit Preview Mar 4, 2024 6:43pm

@Aslemammad Aslemammad changed the title fix: stop sending ssr hydrate flag when the --with-ssr flag is absent fix: stop sending ssr hydrate flag when the --with-ssr flag is absent Mar 4, 2024
Copy link

codesandbox-ci bot commented Mar 4, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@dai-shi
Copy link
Owner

dai-shi commented Mar 5, 2024

The reason was that data-hydrate was always true.

That's kind of intentional. Can you help me understand the problem? How can I reproduce the bug?

@Aslemammad
Copy link
Contributor Author

That's kind of intentional. Can you help me understand the problem? How can I reproduce the bug?

I see, well, in #552 the error is hydration error while withSSR is false. Another way to reproduce it is to put a console log in one main.tsx files, in the first condition (when the dataset.hydrate is true). And when running the project, remove the --with-ssr flag. At that point, we still hydrate and it will show the log.

If it's intentional, then for the 07_router ci test we need to consider another solution or remove the test completely since as it was commented there, the test was not supposed to work with ssr in standalone mode.

@dai-shi
Copy link
Owner

dai-shi commented Mar 5, 2024

for the 07_router ci test we need to consider another solution

After #565 is merged, is the test breaking? sorry, if I miss some points.

@Aslemammad
Copy link
Contributor Author

After #565 is merged, is the test breaking? sorry, if I miss some points.

I haven't tried that PR yet, so let me try it and let you know.

Copy link
Owner

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test was not supposed to work with ssr in standalone mode.

let's remove it.

@dai-shi
Copy link
Owner

dai-shi commented Mar 7, 2024

Probably easier to create a new PR.

@dai-shi dai-shi closed this Mar 7, 2024
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 this pull request may close these issues.

2 participants