React testing patterns: When making a change to a leaf component breaks tests further up the tree
Here's a common scenario I encounter with React applications, where making changes becomes more difficult that we'd like it to be.
We have a Header
component, that display a navigation menu, the user avatar etc.
// Header.tsx
export function Header() {
// Whatever state management tool we are using to make an API call for us
const user = useSelf();
return <div>
{/* Other stuff here*/}
<UserAvatar user={user}/>
</div>
}
And let's we have tests for this, we're taking the recommended (Redux, Tanstack Query) approach of including our state management logic in impliciitly in our test, and mocking the API calls using a tool like MSW.
//Header.test.tsx
// MSW boilerplate here
const handlers = [
// We configure the the behaviour that will determine the `useSelf` behaviour here.
http.get('/self', () => {
return HttpResponse.json({
id: 'c7b3d8e0-5e0b-4b0f-8b3a-3b9f4b3d3b3d',
firstName: 'John',
lastName: 'Maverick',
avatarSrc: "/image.png"
})
}),
]
const server = setupServer(...handlers)
describe(Header, () => {
it("Has a user avatar", async () => {
render(<Header/>)
// Assert existence of UserAvatar here.
})
})
So far, so good, this is a nice simple test.
Let's also assume that our header is included in a root level App
component.
// App.tsx
export function App() {
return <div>
<Header/>
{/* Other content here*/}
</div>
}
Now note that for a test for this, we'll also need to define that that MSW behaviour - otherwise when we render this that Header
component will error out.
//App.test.tsx
// MSW boilerplate here
// The exact same MSW config
const handlers = [
// We configure the the behaviour that will determine the `useSelf` behaviour here.
http.get('/self', () => {
return HttpResponse.json({
id: 'c7b3d8e0-5e0b-4b0f-8b3a-3b9f4b3d3b3d',
firstName: 'John',
lastName: 'Maverick',
avatarSrc: "/image.png"
})
}),
]
const server = setupServer(...handlers)
describe(App, () => {
it("Whatever logic we want to assert about App", async () => {
render(<App/>);
// Assertions here
})
})
Now at this point, this copy pasting might be setting off alarm bells in your head, this feels like a code smell, but let's contine to make the point.
Making a change
So assuming our codebase looks like what we've got above, let's now make a change.
We're going to add functionality to that Header
component - this time adding a notifications indicator.
// Header.tsx
export function Header() {
// Whatever state management tool we are using to make an API call for us
const user = useSelf();
+ const notifications = useNotifications();
return <div>
{/* Other stuff here*/}
+ <Notifications notifications={notifications}/>
<UserAvatar user={user}/>
</div>
}
And our test:
//Header.test.tsx
// MSW boilerplate here
const handlers = [
// We configure the the behaviour that will determine the `useSelf` behaviour here.
http.get('/self', () => {
return HttpResponse.json({
id: 'c7b3d8e0-5e0b-4b0f-8b3a-3b9f4b3d3b3d',
firstName: 'John',
lastName: 'Maverick',
avatarSrc: "/image.png"
})
}),
+ http.get("/notifications", () => {
+ return HttpResponse.json([
+ {
+ id: "1234",
+ eventType: "ticket-status-changed",
+ ticketId: "abcd",
+ newStatus: "closed"
+ }
+ ])
+ })
]
const server = setupServer(...handlers)
describe(Header, () => {
- it("Has a user avatar", async () => {
+ it("Has a user avatar, notifications", async () => {
render(<Header/>)
// Assert existence of UserAvatar here.
+ // Assert about notifications here
})
})
So far, so good, this is a nice clean change here.
BUT: the problem is we also need to make a corresponding change to the App
test as well - it's otherwise going to error when the /notifications
endpoint isn't defined.
I consider this a code smell - where a code change we make breaks tests outside of the scope of what we're working on, and we have to make a series of 'just fix the tests' style changes that aren't actually related to what we're working on.
Note that in this example I've indicated just one test breaking, but it's possible, if this component was used in multiple other places, that we'd making this change in multiple places.
Solutions to this problem
1. Consider the kind of change we made above a breaking change - change our implementation to make the new functionality opt in.
The argument here is that because our implementation will break consumers (tests that indirectly consume the component) of the component, we shouldn't make a change like this, without a major version bump style warning.
Certainly - if this was a published package, being consumed by people outside of our organisation, this would be the right way to go.
We could make an 'opt in' style of change looking like this:
// Header.tsx
+ function Notifications() {
+ const notifications = useNotifications();
+ return <>{/*implementation here*/} <>
+ }
-export function Header() {
+export function Header(props?: {showNotifications?: boolean}) {
// Whatever state management tool we are using to make an API call for us
const user = useSelf();
return <div>
{/* Other stuff here*/}
+ {props?.showNotifications && <Notifications/>}
<UserAvatar user={user}/>
</div>
}
Now, we can opt-in to this change as we need.
We won't break App
until we make this change:
// App.tsx
export function App() {
return <div>
- <Header/>
+ <Header showNotifications/>
{/* Other content here*/}
</div>
}
But here's the problem. When we do make this change, then we need to make that same MSW configuration change to the App
test.
While this 'opt-in' solution is certainly less disruptive - we can take our time in transitioning to the new functionality - we still have to make these changes to all of the tests that are indirect consumers of the component.
2. Maintain a list default MSW handlers.
In this strategy we have a list of default behaviour that all tests will use.
For example it might look like this initially:
//msw-defaults.js
export const defaultHandlers = [
// We configure the the behaviour that will determine the `useSelf` behaviour here.
http.get('/self', () => {
return HttpResponse.json({
id: 'c7b3d8e0-5e0b-4b0f-8b3a-3b9f4b3d3b3d',
firstName: 'John',
lastName: 'Maverick',
avatarSrc: "/image.png"
})
})
]
//App.test.tsx
const specificHandlers = [];
const server = setupServer(...defaultHandlers, ...specificHandlers)
Now, when we make the change adding notifications to the application, we also update the list of default handlers:
//msw-defaults.js
export const defaultHandlers = [
// We configure the the behaviour that will determine the `useSelf` behaviour here.
http.get('/self', () => {
return HttpResponse.json({
id: 'c7b3d8e0-5e0b-4b0f-8b3a-3b9f4b3d3b3d',
firstName: 'John',
lastName: 'Maverick',
avatarSrc: "/image.png"
})
}),
+ http.get("/notifications", () => {
+ return HttpResponse.json([
+ // default is an empty array
+ ])
})
]
The problem with this approach is that these lists may become brittle and difficult to maintain.
For example, might be likely that there are a lot of components that want to have a different set of default handlers, - ones that represent when the session is not authenticated. So now we have two sets, and then, what about a set of handlers for when the user is an admin? Now we have three sets.
Still, this may be a sensible approach for a base level of functionality, eg. representing the set of requests when a user first logs in.
3. Wrap error boundaries around components
In this approach we simply allow the components further down the tree to error - if we don't care about them.
export function App() {
return <div>
<ComponentBoundary data-testid="header">
<Header/>
</ComponentBoundary>
{/* Other content here*/}
</div>
}
The suggestion here is that ComponentBoundary
is both a Suspense boundary and an ErrorBoundary.
This way, we can just allow the Header
component to error out if we don't care about what it's doing.
If we do want it to display, then we still need to opt in by configuring the right API behaviour.
4. Component composition
In this approach, we don't let the Header
component make a data call, instead we rely on the its parent to pass in the Notifications component, like such:
-export function Header() {
+export function Header(props?: {notificationsSlot?: React.ReactNode}) {
// Whatever state management tool we are using to make an API call for us
const user = useSelf();
return <div>
{/* Other stuff here*/}
+ {props?.notificationsSlot && props.notificationsSlot}
<UserAvatar user={user}/>
</div>
}
Now, what we write for our Header
tests doesn't really matter, we'll just be asserting on the existence of whatever we passed in.
describe(Header, () => {
- it("Has a user avatar", async () => {
+ it("Has a user avatar, notifications", async () => {
render(<Header notificationsSlot={<div>I am the notifications<div>/>)
// Assert existence of UserAvatar here.
+ // Assert existence of "I am the notifications" text here
})
})
This is a similar 'opt in' approach as #1, and it has the same problems - when we do opt in our App
component like:
// App.tsx
export function App() {
return <div>
- <Header/>
+ <Header notificationsSlot={<Notifications/>}/>
{/* Other content here*/}
</div>
}
Our App
test we will now need to provide the MSW mocking behaviour necessary in order for the Notifications
component to work.
Another thing I don't like about the component composition approach is that it's difficult to type such that only certain components are valid as children. See this Stack Overflow question noting that the accepted answer does not actually work.
5. Not really a solution - pass the notifications data in as a prop
It might be tempting to suggest that we should do something like this:
-export function Header() {
+export function Header(props?: {notificationsData?: Array<{message: string}>}) {
// Whatever state management tool we are using to make an API call for us
const user = useSelf();
return <div>
{/* Other stuff here*/}
+ {props?.notificationsData && <Notifications data={props.notificationsData}/>}
<UserAvatar user={user}/>
</div>
}
That way we don't need to do any new MSW mocking when we test our Header component.
But note that the MSW mocking on the Header
component test wasn't the problem! That's the scenario where the mocking makes sense.
It's the requiring the App
tests to know about all of the API calls down its render tree that is the problem, and we don't avoid this problem here. Our change to opt in to the notifications functionality would look something like this:
// App.tsx
export function App() {
+ const notifications = useNotifications();
return <div>
- <Header/>
+ <Header notificationsData={notifications}/>
{/* Other content here*/}
</div>
}
And now we would require new MSW mocking in the App
component.
Perhaps though - that's OK, after all, it's the App
component that is making the call in state management.
However, the caution I would give is that this approach of only doing calls into the state management at the top levels, means that you end up having 'God objects' at the top of your render tree, with lots of API calls and this can make them difficult to maintain.
Conclusions
Solutions that allow an 'opt in' to new functionality are less disruptive, but are not a panacea, ulimately if opting in requires making changes in every affected test, then it will still be a difficult task.
The problem with having state management calls at lower levels of your render tree is that components higher up the tree now need to be implicitly aware of what these API calls are. This can be mitigated by simply allowing those components to error out, though this might pose its own set of issues.
Component composition is effective way to reduce prop drilling.
Ultimately there is a tension between avoiding having implicit state management further down your tree, vs avoiding having God objects at the top of your tree.
I think the happy medium is that split a given page into sensible logical groupings, and have these be wrapped in both and error boundary and a Suspense boundary. This fits very well with the React Server Components model.
That way, at a top level the page is simply responsible for basic layout, and the logical groups can contain some complex logic, but are hopefully small enough that the logic is understandable.
The sensible logical groupings could in turn, also contain some of their own sensible logical groupings that are wrapped in error and Suspense boundaries.
Questions? Comments? Criticisms? Get in the comments! 👇
Spotted an error? Edit this page with Github