Skip to content

Commit

Permalink
linting post
Browse files Browse the repository at this point in the history
  • Loading branch information
@vernak2539
vernak2539 committed Apr 26, 2024
1 parent d3a8ded commit a015d64
Showing 1 changed file with 45 additions and 130 deletions.
175 changes: 45 additions & 130 deletions src/content/blog/2024/04/the-importance-of-linting.mdx
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
---
title: "Goodbye Enzyme"
description: "You won't be missed..."
pubDate: "2024-04-19"
tags:
[
"frontend",
"javascript",
"typescript",
"react",
"enzyme",
"react testing library",
"testing",
]
title: "The Importance of Linting"
description: "Yes, you should lint your code*..."
pubDate: "2024-04-27"
tags: ["frontend", "javascript", "typescript", "linting"]
---

<style>
Expand All @@ -24,152 +15,76 @@ tags:
import { Tweet } from "@astro-community/astro-embed-twitter";
import Banner from "../../../../components/Banner/index.astro";

Yes, you should lint your code\*.

https://x.com/mjackson/status/1782974201001038316
Showing my age again, I remember when there were so many linters for JS. There was [JSLint](https://www.jslint.com/), [JSHint](https://jshint.com/), [ESLint](https://eslint.org/), and likely many more.

Finally, ESLint emerged as the default linter, with almost all companies adopting it.

I was on Twitter (yes, it's "_Twitter_") and saw this tweet from [Michael Jackson](https://twitter.com/mjackson):

I've showed my age in the [previous post](./snapshot-testing-sucks.md), so I won't do it again. But, for the
purposes of this post, I will say that I've been a professional in the industry prior to [Enzyme](https://enzymejs.github.io/enzyme/),
during its height, and after it (i.e. now).
<Tweet id="https://twitter.com/mjackson/status/1782974201001038316" />

This leads me to say, using my _expert_ opinion, "Goodbye Enzyme, and good riddance. You will not be missed."
After I got over my initial rage of someone suggesting that linting was not important, I realised it is all situational.

That may seem harsh, but I stand behind it. I've recently removed Enzyme from multiple, highly used codebases at my work
and have see the horrors caused by Enzyme. I'll cover some of these in this article, comparing it to [React Testing Library][rtl].
Before we get too far, let's talk about [Prettier](https://prettier.io). Prettier is an opinonated code formatter, not a linter, with the main goal of _enforcing consistent code style_.

## Enzyme? React Testing Library (RTL)? What are they?
This is super different from a linter, which is more about _finding and fixing problems in your code_.

They are both testing frameworks aiming to make it easier to test React components. They both have their own philosophies
and ways of doing things.
Now that we got that cleared up, we'll be focusing fully on linters!

### Enzyme
## Should you lint your code?

> Enzyme is a JavaScript Testing utility for React that makes it easier to test your React Components' output.
> You can also manipulate, traverse, and in some ways simulate runtime given the output.
This is a great question. I usually ask myself a few questions before deciding when to lint my code.

### RTL
1. How big is the project?
2. How many people are working on the project?
3. Is the project a personal, work, or OSS project?
4. Is the project a PoC or a production-ready project?
5. Do I care about errors??? (a bit of a joke, but also serious)

> You want to write maintainable tests that give you high confidence that your components are working for your users.
> As a part of this goal, you want your tests to avoid including implementation details so refactors of your components
> (changes to implementation but not functionality) don't break your tests and slow you and your team down.
### Yes, I should lint my code!

<Tweet id="https://twitter.com/kentcdodds/status/977018512689455106" />
I'd lint my code if:

These differing philosophies are the main reason why I prefer RTL over Enzyme. Enzyme is more focused on the "output" of
a component. Additionally, they talk about traversing, manipulating, and simulating the output, which does not align
with how actual users would be interacting with it.
- It's a big project is and a lot of people working on it
- A lot of people contribute to the project
- The project will be used in production
- I do care about catching errors before they happen
- It's a work project

On the other hand, RTL is more focused on the "functionality" of a component. It is more concerned with how a user would
interact with the component, and how the component would respond to that interaction. ❤️
### Nah, I can get away without...

<Banner type="info">
I'm going to use personal anecdote. These may not match your experience.
</Banner>
- I know it's a work PoC, and it will be thrown away after (is this really ever the case though?)
- It's a personal project, and I'm the only one working on it
- I'm only working on it, and I want to move fast (I'll still use a formatter)

## Problems with Enzyme
## Yeah, but has it really saved you??

### Testing async scenarios is difficult
Yes... There's been so many things that linters have caught at my places of work.

One of the biggest issues I've had with Enzyme is testing async scenarios. Unless you're purely working on a presentational component, most components will have some form of async natur. Something like data loading, form input and validation, etc.
One that helps all the time is [`import/resolver`](https://github.com/import-js/eslint-plugin-import). This is less helpful now that we have amazing IDEs that do a lot of these things automatically, but it still catches things!

You'll need to perform an action, wait for the async action to complete, then move on with your test.
Essentially, this catches incorrect imports. If you try to import something from a file that doesn't exist, BOOM, error.

This is a nightmare to do in Enzyme. Code using `setTimeout`, `setInterval`, `setImmediate`, `await Promise.resolve()` or similar is likely trying to account for these async actions. It's HACKY... and I never liked doing it, even though there was no other way.
## Does it always save you??

RTL handles this much better in two ways, maybe more.
And no, it doesn't always save you. We recently had a problem where the linter didn't throw an error when an undefined function was called.

First, RTL provides a `waitFor` utility. It allows you to wait for a condition to be true before moving on with the test. This can be used in a sync or async manner. Using it in an async manner will ensure that the async action has completed before moving on.
The function was supposed to be imported into the file, but, unfortunately it wasn't. This caused a runtime error.

Second, the [`@testing-library/user-event` package](https://testing-library.com/docs/user-event/intro/) provides utility methods to help deal with user interactions. When a user interaction triggers an async action via a command like `click`, `type`, `clear`, etc., the command can be awaited to ensure the async action has completed.
After some investigation, it turns out [ESLint's `no-undef` rule](https://eslint.org/docs/latest/rules/no-undef) would've saved us. We thought it was enabled, but unfortunately, it wasn't. This was because:

This functionality is only in the newer versions of `user-event`, but it's already proved amazingly helpful.
> ... `plugin:@typescript-eslint/recommended` turns off `no-undef` because it's covered by TS, so double reporting is somewhat useless.
### Simulating interactions
The reasoning makes sense, as TS should catch these errors (and the files in the offending PR were TS). But, it didn't. Why?

When testing user interactions, Enzyme simulates events in a programmatic way by dispatching DOM events.
TL;DR - we'd disabled type checking because we were incrementally adopting TS across our codebase.

IMO, this doesn't mimic real user interaction as there may be multiple events that can fire in a single interaction.
You can bet that we've enabled that rule now!

For example, how many events fire when clicking a button? Well, there's quite a few:
## Long story short

1. `mouseover`
2. `mousedown`
3. `mouseup`
4. `click` (interesting that this is after `mouseup`... hmmm haha)
5. `mouseout`

If you're only simulating a "click" you're missing out on a lot of the events that would happen during an interaction. This leads
to less confidence in your tests (IMO).

<Banner type="info">
Checkout my in-depth post about [Simulating
Events](../../2023/04/simulating-js-events.mdx) for more context!
</Banner>

Or, if we take an example directly from their [website](https://testing-library.com/docs/user-event/intro/):

> For example, when a user types into a text box, the element has to be focused, and then keyboard and input events are fired and the selection and value on the element are manipulated as they type.
RTL, on the other hand, uses the `@testing-library/user-event` package to simulate user interactions (you can still use the
`fireEvent` method though if you really want to). This takes into account the multitude events that could happen when a user
interacts with a component.

### Having access to props

This is a killer... and I hate it. I've seen it abused so many times.

You can see a basic form below.

```js
const Component = ({ onSubmit }) => {
return (
<form onSubmit={onSubmit}>
<input type="text" name="input1" />
<input type="text" name="input2" />
<button type="submit" onSubmit={onSubmit}>
Submit
</button>
</form>
);
};
```

It's likely there's some form of validation on the inputs and once the form is submitted some action has to take place.

In Enzyme, you can access the props of a component and call the `onSubmit` function directly, allowing something like:

```js
it("should submit form", () => {
const someAsyncAction = jest.fn();
const onSubmit = () => {
someAsyncAction();
};
const wrapper = shallow(<Component onSubmit={onSubmit} />);

wrapper.find("form").props("onSubmit")();

expect(someAsyncAction).toHaveBeenCalled();
});
```

This is hard to look at. There's no user interaction and no validity checks. We're just calling a function directly (yes,
I have seen this in real life).

While this is a simple example, you can see how it would open the door for lazy testing (and it does).

## /endrant

While Enzyme was an absolute _killer_ when it first came out, enabling the testing of UI components like we'd not seen before, it's
not fit for purpose anymore.

It actively encourages bad practices, can't be used past v16 of React (with an official adapter),
isn't maintained, and has a philosophy that doesn't align with how users interact with components.

Please, please consider using [RTL][rtl] for your testing needs. It's a much better framework that encourages better testing.

Goodbye Enzyme, and good riddance. You will not be missed.
You should lint your code if you work in a professional setting where many people contribute to a long-lived codebase (where you care about consistency and standards as time goes on).

<script async src="https://platform.twitter.com/widgets.js"></script>

[rtl]: (https://testing-library.com/docs/react-testing-library/intro/).

0 comments on commit a015d64

Please sign in to comment.