generated from cloud-gov/.github
-
Notifications
You must be signed in to change notification settings - Fork 0
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
NextJS prototype - example pages and test suite #16
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jduss4
reviewed
Feb 23, 2024
From NextJS docs: Test files should not be included inside the Pages Router because any files inside the Pages Router are considered routes. More info: https://nextjs.org/docs/pages/building-your-application/testing/jest#creating-your-first-test
using the new app routing instead of pages routing
jduss4
reviewed
Feb 28, 2024
@hursey013 @sknep wanted to get both your eyes on this before I merged |
hursey013
reviewed
Feb 29, 2024
hursey013
approved these changes
Feb 29, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! We can see some of the fundamental patterns we'll be using if we proceed with Next.js, and this provides a great example and reference, thank you!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR feedback I’m looking for:
I'll ask for feedback from all reviewers before merging.
Relates to #17
To run this prototype, see the README.
This prototype uses the following tools for testing:
There are two example pages: one page renders the users server side, and the other renders users client side. You can see this in action by viewing the source code of each page.
Update: I added an example of dynamic routing as well, accessible from the homepage
I wrote some tests to make sure each page is rendering the fake users the way it’s supposed to, as well as some unit tests for the APIs to make sure they handle errors correctly.
Note that I haven’t added any linting to the prototype yet (coming soon) so code style may be inconsistent.Update: Linting has been added but it's not part of any CI process yetJest and Nock
I settled on Jest as the test runner after reading discussion threads and blog posts on what to choose. Here are some things I like about it:
Something to be aware of is that Jest doesn’t implement a mock of
fetch
by default, so you must mock it yourself. There are many tools to choose from. After using a few (msw, whatwg-fetch) and running into roadblocks, I landed on nock, which I noticed pages-core also uses. I use nock to disable all network requests in the test suite injest.setup.js
.Also be aware of the different Jest environments, node and jsdom. Node is the default (set in
jest.config.js
) which is fine for testing things like the API. But for any tests that need a browser, I override the jest environment to be jsdom at the top of the test file .React Testing Library
I chose React Testing Library over other libraries like Enzyme because of its one guiding principle:
“The more your tests resemble the way your software is used, the more confidence they can give you. So rather than dealing with instances of rendered React components, your tests will work with actual DOM nodes.”
I like this in theory, but it makes testing React components a bit awkward in practice, especially when React hooks are involved.
RTL has a synchronous way of querying HTML output (any query with “get”) and an async way (any query with “find”). The synchronous way will not call react hooks like
useEffect
, whereas the async functions will. But you wouldn’t know that just from reading a test—I had to learn that through trial and error.RTL is straightforward to use when the user initiates the action that updates React component state, like a “click” or a “submit.” But for components that update state on page load (a very common pattern in React apps), there is no explicit user action, and so tests end up looking a little awkward.
The mitigation for this is to have only a few “smart” React components that are API-aware and use internal state, and have the rest of your lower-level components inherit the data through props.
The upside to RTL being really lightweight is that testing server-rendered components (or any component that just uses props) is very simple. Just pass the props you want to the component, and test away.
Security considerations
Prototype not headed for production anytime soon