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

feat: add test to re**produce #605 #1

Merged
merged 3 commits into from
Jun 16, 2022

Conversation

koobawuah
Copy link
Owner

@koobawuah koobawuah commented Jun 16, 2022

What:

This fixes a bug that assumes that tests don't run in SSR environments.

Why:

With frameworks like Next.js and Remix rendering components on the server, it's important that this library can mimic SSR environments as closely as possible.

How:

First by adding a test that reproduces the issue and then by moving this logic "into the hydrate function, so we reference document when they want it to render for real." (suggested by mpeyper).

Checklist:

  • Documentation updated
  • Tests
  • Ready to be merged
  • Added myself to contributors table

@koobawuah
Copy link
Owner Author

  1. L37, “ If container is undefined, then we pass null to hydrate since undefined is not a valid container, but null is.”

  2. L52-54, “ Since act returns a function, it introduces a new scope which means we need to check again that container is not undefined before using it. This means we could remove the outer if (container) { check if we're okay that unmount always returns act”.

@koobawuah koobawuah marked this pull request as ready for review June 16, 2022 21:58
@koobawuah koobawuah merged commit 7d8d77f into koobawuah-fix-ssr-before Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant