Skip to content

feat(server): add SSR test for renderHook #799

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 18, 2022
Merged

feat(server): add SSR test for renderHook #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5247d27
refactor(server/pure): remove unnecessary type annotation
Feb 27, 2022
93d927d
feat: add ssr.test.ts
Feb 27, 2022
59c752f
chore: update contributors table
Feb 27, 2022
c37e8a4
refactor: remove act call in render in sever/pure
jsjoeio Jun 10, 2022
9d35e1c
Merge branch 'main' into jsjoeio-add-test
jsjoeio Jun 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .all-contributorsrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@
"avatar_url": "https://avatars3.githubusercontent.com/u/3806031?v=4",
"profile": "https://jsjoe.io",
"contributions": [
"tutorial"
"tutorial",
"test"
]
},
{
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
<td align="center"><a href="https://github.com/VinceMalone"><img src="https://avatars0.githubusercontent.com/u/2516349?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Vince Malone</b></sub></a><br /><a href="https://github.com/testing-library/react-hooks-testing-library/commits?author=VinceMalone" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/doppelmutzi"><img src="https://avatars1.githubusercontent.com/u/4130968?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Sebastian Weber</b></sub></a><br /><a href="#blog-doppelmutzi" title="Blogposts">📝</a></td>
<td align="center"><a href="https://gillchristian.xyz"><img src="https://avatars2.githubusercontent.com/u/8309423?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Christian Gill</b></sub></a><br /><a href="https://github.com/testing-library/react-hooks-testing-library/commits?author=gillchristian" title="Documentation">📖</a></td>
<td align="center"><a href="https://jsjoe.io"><img src="https://avatars3.githubusercontent.com/u/3806031?v=4?s=100" width="100px;" alt=""/><br /><sub><b>JavaScript Joe</b></sub></a><br /><a href="#tutorial-jsjoeio" title="Tutorials">✅</a></td>
<td align="center"><a href="https://jsjoe.io"><img src="https://avatars3.githubusercontent.com/u/3806031?v=4?s=100" width="100px;" alt=""/><br /><sub><b>JavaScript Joe</b></sub></a><br /><a href="#tutorial-jsjoeio" title="Tutorials">✅</a> <a href="https://github.com/testing-library/react-hooks-testing-library/commits?author=jsjoeio" title="Tests">⚠️</a></td>
</tr>
<tr>
<td align="center"><a href="http://frontstuff.io"><img src="https://avatars1.githubusercontent.com/u/5370675?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Sarah Dayan</b></sub></a><br /><a href="#platform-sarahdayan" title="Packaging/porting to new platform">📦</a></td>
Expand Down
18 changes: 18 additions & 0 deletions src/__tests__/ssr.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @jest-environment node
*/
Comment on lines +1 to +3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be nice if we could have sone server only tests run in the node environment without jsdom to make sure we don't break it in the future, but I'm not sure how we would do that for just some tests?

✍🏼 This technique is handy for running specific tests in a Node environment.

See docs.

import { useState } from 'react'

// This verifies that renderHook can be called in
// a SSR-like environment.
describe('renderHook', () => {
function useLoading() {
const [loading, setLoading] = useState(false)
return { loading, setLoading }
}
runForRenderers(['server'], ({ renderHook }) => {
test('should not throw in SSR environment', () => {
expect(() => renderHook(() => useLoading())).not.toThrowError('document is not defined')
})
})
})
14 changes: 6 additions & 8 deletions src/server/pure.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,17 @@ function createServerRenderer<TProps, TResult>(
) {
let renderProps: TProps | undefined
let container: HTMLDivElement | undefined
let serverOutput: string = ''
let serverOutput = ''
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🧹 Since we give serverOutput a default value of '', TS can infer string type. The type annotation here is not needed.

const testHarness = createTestHarness(rendererProps, wrapper, false)

return {
Copy link
Member

Choose a reason for hiding this comment

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

github won't let me comment on the actual line, but the error in the test is being caused by the act call on line 22. For some reason, when you use [email protected] this tried to use the document which throws an error when testing in the node environment, which doesn't occur in newer versions.

You can replicate this by running:

npm run install:react-16.9.0
npm test -- --no-watch src/__tests__/ssr.test.ts

In the context of server rendering, the act call is not really necessary here. Dropping it out still passes all of our tests and allows your new test to pass with the older version:

    render(props?: TProps) {
      renderProps = props
      try {
        serverOutput = ReactDOMServer.renderToString(testHarness(props))
      } catch (e: unknown) {
        rendererProps.setError(e as Error)
      }
    },

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow, I don't think I would have figured that out on my own 😅 I'll test this out and make the necessary changes. Thanks a bunch!

render(props?: TProps) {
renderProps = props
act(() => {
try {
serverOutput = ReactDOMServer.renderToString(testHarness(props))
} catch (e: unknown) {
rendererProps.setError(e as Error)
}
})
try {
serverOutput = ReactDOMServer.renderToString(testHarness(props))
} catch (e: unknown) {
rendererProps.setError(e as Error)
}
},
hydrate() {
if (container) {
Expand Down