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

[Easy] Fix lint errors! #1

Open
Flaque opened this Issue Oct 20, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@Flaque
Copy link
Member

Flaque commented Oct 20, 2017

We've added a linter! A linter is a program that you run on your code to alert you of potential errors and style problems. Specifically, we're using ESLint. You can run ours with:

yarn run lint

If you do that, you'll see a bunch of errors and warnings that look like this:

screen shot 2017-10-17 at 9 53 46 am

Each one of these are potential problems to fix in the code that you can do to help out and get oriented with the code.

You don't need to fix all of them, there's a lot. But if you just fix one, you'll be helping stop a potential bug!

Steps to fix a lint bug

Take this bug:

screen shot 2017-10-17 at 9 56 10 am

The linter is telling us a couple things. First, it tells us the file. On my computer that file is located at: /Users/Flaque/Projects/gu-port/pages/index.js, so I'll go open that up in my text editor:

// pages/index.js
import Main from "../src/Main";
import Centered from "../src/components/Centered";
import React from "react";
import api from "../src/api";

export default class extends React.Component {
  static async getInitialProps() {
    const { data } = await api.getPageList(3, 0);
    return { articles: data.pages };
  }

  render() {
    return (
      <Centered>
        <Main articles={this.props.articles} />
      </Centered>
    );
  }
}

Then our linter is telling us the line and column where it's found the problem: 15:36. That's line 15, which looks like this:

<Main articles={this.props.articles} />

Finally, our linter tells us a description of the problem and the problem's name: react/prop-types. If I've never worked in React or this code base before, the message warning 'articles' is missing in props validation might be a little confusing.

Thankfully, we've got that name: react/prop-types, which is really easy to google. I'm gonna just use eslint react/prop-types as my search term:

screen shot 2017-10-17 at 10 04 08 am

That page is the ESLint "rule" documentation. Every ESLint rule has one of these somewhere.
They generally tell you:

  1. What "bad" code looks like
  2. What "good" code looks like

This rule is described as "Prevent missing props validation in a React component definition (react/prop-types)". If you're not sure what some of those terms mean, it's probably a good idea to google them!

In this case, it helps to understand a bit about how React works. React lets you take in props as arguments to components. In the above pages/index.js code, on line 15, we see an example of prop usage:

<Main articles={this.props.articles} /> // Pass in the prop called "articles" into "Main"

Props can also have type checking, often called "prop validation" in their definition like this:

import PropTypes from "prop-types"

class HelloEs6WithPublicClassField extends React.Component {
  static propTypes = {
    name: PropTypes.string.isRequired,
  }
  render() {
    return <div>Hello {this.props.name}</div>;
  }
}

You can learn about how to create PropTypes here.

We'll follow the same patterns and make our articles an array. Notice we're not setting isRequired because there's a potential chance that we don't have any articles to render!

import Main from "../src/Main";
import Centered from "../src/components/Centered";
import React from "react";
import api from "../src/api";
import PropTypes from "prop-types";

export default class extends React.Component {
  static async getInitialProps() {
    const { data } = await api.getPageList(3, 0);
    return { articles: data.pages };
  }

  static propTypes = {
    articles: PropTypes.array
  };

  render() {
    return (
      <Centered>
        <Main articles={this.props.articles} />
      </Centered>
    );
  }
}

Now, if we run our linter again, we'll have one less bug!

screen shot 2017-10-17 at 10 21 39 am

Flaque pushed a commit that referenced this issue Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.