dzikowski.github.io

Piszę o IT po polsku, więc z góry przepraszam za zagęszczenie kolokwializmów, ale co w angielskim brzmi naturalnie, w polskim często nie ma nawet odpowiedników.

ESLint. Twój kod może być piękny

Z roku na rok i miesiąca na miesiąc coraz bardziej przekonuję się do tego, jak bardzo ważna jest nie tylko znajomość języka, ale i narzędzi wspierających development. Jednym z nich jest ESLint, które sprawdzi, czy twój kod jest napisany zgodnie z założoną konwencją, a nawet częściowo go poprawi; zarówno czysty JavaScript, jak i pliki Reacta.

W skrócie wykorzystanie ESLint wygląda następująco:

  1. Instalujesz i konfigurujesz ESLint.
  2. Uruchamiasz go, albo z terminala, albo włączasz w skrypty w package.json.
  3. ESLint pokazuje, co należy poprawić w kodzie i – opcjonalnie – sam naprawia część rzeczy.

Dodatkowo można do niego podpiąć zestaw reguł przygotowany przez kogoś innego. Na potrzeby startera skorzystam z konfiguracji dla dobrych praktyk opracowanych przez Airbnb (tak, ten Airbnb). Jest to chyba najbardziej popularna konfiguracja (na GitHubie ma w chwili obecnej 52 tys. gwiazdek – na piątym miejscu jeśli chodzi o projekty javascriptowe), a w dodatku zawiera też dobre praktyki dla Reacta.

Instalacja

Airbnb przygotował nawet pakiet z konfiguracją ESLinta: eslint-config-airbnb. Na stronie tego pakietu możesz także znaleźć instrukcję, jak go zainstalować, żeby mieś pewność, że wszystkie zależności będą w pożądanych wersjach. Na Linuksie i OS X wystarczy wywołać skrypt:

(
  export PKG=eslint-config-airbnb;
  npm info "$PKG@latest" peerDependencies --json | command sed 's/[\{\},]//g ; s/: /@/g' | xargs npm install --save-dev "$PKG@latest"
)

Skrypt ten zainstaluje także samego ESLinta, dzięki czemu od razu będzie można z niego korzystać w terminalu i w skryptach NPM. Dodanie do skryptów jest proste, można na przykład dopisać coś takiego do package.json:

  "scripts": {
    "lint": "eslint . --fix",
    ...
  }

Niestety jednak ten sposób nie zadziała bezpośrednio w terminalu. Nie możesz ot tak sobie wywołać polecenia eslint, ponieważ nie jest ono zainstalowane globalnie. Globalna instalacja ESLint jest w sumie pokazywana na ich stronie (npm install -g eslint), jednak radziłbym jej zaniechać. Dzięki temu nie masz wątpliwości, czy korzystasz z odpowiedniej wersji ESLint i odpowiednich zależności, po prostu masz to, co zostało podłączone do projektu. A wywołać ESLint z terminala można w katalogu projektu, tyle że trzeba pisać trochę więcej:

./node_modules/.bin/eslint . --fix

Zanim jednak do tego dojdziemy, należy go skonfigurować.

Konfiguracja

Konfiguracja jest względnie prosta, mamy dostępny w terminalu taki kreator krok po kroku. Wystarczy wywołać:

./node_modules/.bin/eslint --init

Teraz w terminalu wybieramy odpowiednie pozycje. Chcemy skorzystać z popularnego zestawu styli:

? How would you like to configure ESLint? 
  Answer questions about your style 
❯ Use a popular style guide 
  Inspect your JavaScript file(s) 

Będzie to Airbnb:

? Which style guide do you want to follow? 
  Google 
❯ Airbnb 
  Standard 

Tak (y), używamy Reacta:

? Do you use React? (y/N) y

I na koniec można jeszcze wybrać format pliku konfiguracyjnego:

? What format do you want your config file to be in? 
❯ JavaScript 
  YAML 
  JSON 

W rezultacie zostanie utworzony plik .eslintrc.js z mniej więcej taką zawartością:

module.exports = {
  "extends": "airbnb",
  "plugins": [
    "react",
    "jsx-a11y",
    "import"
  ]
};

Pierwsze uruchomienie

Teraz wystarczy wpisać w terminalu ./node_modules/.bin/eslint ., aby ESLint przeleciał cały projekt i sprawdził, jak bardzo odbiegasz od stylów Airbnb. Pierwsze uruchomienie dla mojego startera dało w sumie 232 błędy 🙂. Poniżej próbka:

/Users/jakub/IdeaProjects/serverless-webapp-starter/app/profile/ProfileRoutes.js
   1:21   error  Strings must use singlequote                                                        quotes
   2:19   error  Absolute imports should come before relative imports                                import/first
   2:19   error  Strings must use singlequote                                                        quotes
   3:8    error  A space is required after '{'                                                       object-curly-spacing
   3:34   error  A space is required before '}'                                                      object-curly-spacing
   3:41   error  Strings must use singlequote                                                        quotes
   4:20   error  Parse errors in imported module './SignIn': Unexpected token = (8:21)               import/no-named-as-default-member
   4:20   error  Strings must use singlequote                                                        quotes
   4:20   error  Parse errors in imported module './SignIn': Unexpected token = (8:21)               import/no-named-as-default
   5:22   error  Parse errors in imported module './Register': Unexpected token = (9:21)             import/no-named-as-default
   5:22   error  Strings must use singlequote                                                        quotes
   5:22   error  Parse errors in imported module './Register': Unexpected token = (9:21)             import/no-named-as-default-member
   6:33   error  Parse errors in imported module './ConfirmRegistration': Unexpected token = (9:21)  import/no-named-as-default
   6:33   error  Parse errors in imported module './ConfirmRegistration': Unexpected token = (9:21)  import/no-named-as-default-member
   6:33   error  Strings must use singlequote                                                        quotes
   8:24   error  A space is required after '{'                                                       object-curly-spacing
   8:25   error  'user' is missing in props validation                                               react/prop-types
   8:31   error  'auth' is missing in props validation                                               react/prop-types
   8:40   error  'rest' is defined but never used                                                    no-unused-vars
   8:44   error  A space is required before '}'                                                      object-curly-spacing
   9:3    error  JSX not allowed in files with extension '.js'                                       react/jsx-filename-extension
  10:84   error  A space is required before closing bracket                                          react/jsx-tag-spacing
  11:87   error  A space is required before closing bracket                                          react/jsx-tag-spacing
  12:1    error  Line 12 exceeds the maximum line length of 100                                      max-len
  12:110  error  A space is required before closing bracket                                          react/jsx-tag-spacing
  13:71   error  A space is required before closing bracket                                          react/jsx-tag-spacing

Jak widzisz, są tu błędy różnego typu. Brakujące spacje po { , stringi w podwójnych cudzysłowach zamiast pojedynczych, przekroczona maksymalna długość linii, pliki z JSX mające złe rozszerzenie itd.

Na szczęście nie musiałem poprawiać tego wszystkiego manualnie, bo ESLint ma flagę --fix, która sprawia, że cześć błędów zostanie poprawiona automatycznie. Wywołałem:

./node_modules/.bin/eslint . --fix

I teraz z 232 zrobiły się tylko 64 błędy. Przyjrzyjmy się im bliżej.

Ciekawostka

ESLint znalazł też takie błędy, których bym się nie spodziewał. Zaskakujące było dla mnie na przykład to i to – Airbnb zaleca, żeby kilkulinijkowe listy argumentów funkcji, albo listy atrybutów obiektu kończyły się przecinkiem. Na przykład tak:

const user = {
  firstName: 'Jakub',
  lastName: 'Dzikowski',
};

Motywacja: Babel i tak usunie ostatni przecinek, więc nie ma problemu ze starymi przeglądarkami, które mogłyby sobie nie poradzić. A w dodatku, dzięki temu przecinkowi, łatwiej jest porównywać linijki w systemie wersji (są czystsze diffy). Niech będzie, to i tak jest poprawiane automatycznie.

Statyczne atrybuty

Stosunkowo częstym błędem było coś takiego:

   4:20  error  Parse errors in imported module './User': Unexpected token = (8:19)               import/no-named-as-default

Po pobieżnym przejrzeniu kodu i chwili googlowania okazało się, że standard Airbnb odradza stosowanie niektówych nowszych funkcjonalności języka, w tym także statycznych atrybutów klas.

class User {
  ...
  static signedIn = email => new User(email, true);
  ...
}

Rozwiązania są dwa. Pierwsze: rezygnuję ze statycznych atrybutów i zmieniam kod na coś takiego:

class User {
  ...
}

User.signedIn = email => new User(email, true);

Drugie rozwiązanie opisano tutaj. Można skorzystać z biblioteki babel-eslint i dodać ją jako parser do ESLinta. (Z tego właśnie rozwiązania skorzystałem w projekcie).

Czyli najpierw:

npm install babel-eslint --save-dev

Potem w pliku .eslintrc.js:

module.exports = {
  ...
  "parser": "babel-eslint",
};

Zmienne globalne

Tak, miałem takie w kodzie. Pisałem o nich kilka postów wcześniej. Chodziło o plugin, który pozwalał Webpackowi wstrzykiwać do plików javascriptowych zawartość zmiennych środowiskowych. Wtedy te zmienne są globalne – można z nich korzystać w JavaScripcie, ale nie ma nigdzie ich deklaracji. ESLint traktuje to jako użycie zmiennej bez deklaracji i pokazuje takie błędy:

/Users/jakub/IdeaProjects/serverless-webapp-starter/app/config.js
  2:11  error  'COGNITO_POOL_ID' is not defined        no-undef
  3:16  error  'COGNITO_APP_CLIENT_ID' is not defined  no-undef

Rozwiązaniem jest podanie w komentarzu, że dana zmienna jest globalna:

/* global COGNITO_POOL_ID:true */
/* global COGNITO_APP_CLIENT_ID:true */
const cognitoConfig = {
  poolId: COGNITO_POOL_ID,
  appClientId: COGNITO_APP_CLIENT_ID,
};

Zmiana rozszerzenia plików

Airbnb zaleca, że jeżeli w jakimś pliku korzystamy z JSX (czyli tego reactowego języka znaczników), to plik powinien mieć rozszerzenie nie js, ale jsx. Wystarczy zmienić nazwy plików? Nieprawda, musimy też trochę pokombinować z konfiguracją Webpacka 🙂.

Kiedy już zmienimy nazwy plików, okazuje się, że napotkamy następujące problemy (mogą one wychodzić po kolei, niekoniecznie wszystkie naraz i niekoniecznie w takiej kolejności):

  1. Będzie brakowało pliku index.js.
  2. Webpack nie będzie dorzucał do zbudowanego bundle plików JSX.
  3. Żeby zaimportować komponent Reacta, trzeba będzie użyć pełnej nazwy, np. import './ProfileMenu.jsx' zamiast import ./ProfileMenu.
  4. ESLint nie będzie analizować plików JSX.

No więc pierwszy problem wynika z tego, że w index.js był JSX, więc musiałem zmienić jego nazwę na index.jsx. W związku z tym muszę też zmienić nazwę entry w pliku webpack.config.js i problem zostanie rozwiązany:


module.exports = {
  entry: [
    './app/index.jsx',
  ],
  ...
};

Druga sprawa, mamy tak skonfigurowanego Webpacka, że babel-loader czyta tylko pliki JS. Ponieważ od teraz mamy także pliki JSX, musimy zmienić konfigurację loadera, żeby czytał też JSX. Czyli znów zmieniamy webpack.config.js:

const BabelLoader = {
  test: /\.(js|jsx)$/,
  exclude: /node_modules/,
  loader: 'babel-loader',
};

Trzeci punkt zajął mi trochę czasu, bo nie wiedziałem, dzięki czemu dla plików JS nie trzeba było podawać rozszerzenia w importach. Otóż okazało się, że wynika to z domyślnych wartości elementu resolve.extensions w konfiguracji Webpacka (zob. tutaj). Żeby można było bez rozszerzenia importować zarówno pliki JS, jak i JSX wystarczyło dodać do webpack.config.js:

module.exports = {
  ...
  resolve: {
    extensions: ['.js', '.jsx'],
  },
  ...
};

Czwarty punkt: ESLint nie sprawdza plików JSX. Z tym jest największy problem i nie znalazłem zadowalającego mnie w pełni sposobu. Po przejrzeniu kilku zgłoszeń na GitHubie okazało się, że jedynym sposobem jest podanie w wywołaniu ESLinta odpowiedniego parametru (co potwierdza oficjalna dokumentacja). Czyli w terminalu na przykład tak:

./node_modules/.bin/eslint . --ext .js --ext .jsx

Dzięki temu mam 51 błędów, kiedy myślałem, że jestem już czysty. (Choć w sumie podejrzanie szybko zredukowała mi się liczba błędów, kiedy zmieniłem nazwy plików).

/Users/jakub/IdeaProjects/serverless-webapp-starter/app/App.jsx
   5:24  error  Absolute imports should come before relative imports  import/first
  11:3   error  auth should be placed after state                     react/sort-comp

/Users/jakub/IdeaProjects/serverless-webapp-starter/app/common/messages.jsx
  27:32  error  'text' is missing in props validation  react/prop-types
  27:41  error  'rest' is defined but never used       no-unused-vars
/Users/jakub/IdeaProjects/serverless-webapp-starter/app/App.jsx
   5:24  error  Absolute imports should come before relative imports  import/first
  11:3   error  auth should be placed after state                     react/sort-comp

/Users/jakub/IdeaProjects/serverless-webapp-starter/app/common/messages.jsx
  27:32  error  'text' is missing in props validation  react/prop-types
  27:41  error  'rest' is defined but never used       no-unused-vars

Przy okazji uaktualniam skrypt NPM, żeby też sprawdzał pliki JSX (plik package.json):

  "scripts": {
    "lint": "eslint . --ext .js --ext .jsx --fix",
    ...
  }

Zmiany w plikach JSX

Niestety żaden z tych 51 błędów nie został już naprawiony automatycznie, więc krok po kroku przebijałem się przez pliki i pokornie zmieniałem kod. Po drodze postęppowałem też zgodnie z dwiema zasadami Airbnb, których nie wykrywał ESLint:

  1. Jako komponenty Reacta preferuj funkcje, a nie arrow functions (opisana tutaj).
  2. Używaj arrow functions, żeby domknąć lokalne zmienne (opisana tutaj).

Przy okazji poprawiania kodu JSX, natknąłem się też na fajne rzeczy, których nie znalazłem wcześniej w dobrych praktykach. Na przykład trzeba walidować props, które mają być przekazane do komponentu, co można zrobić na przykład w ten sposób:

Routes.propTypes = {
  user: PropTypes.object.isRequired,
};

Tyle że to jest już niepoprawne, otrzymujesz komunikat Prop type 'object' is forbidden, co jak najbardziej ma sens. Trzeba bardziej szczegółowo:

Routes.propTypes = {
  user: PropTypes.instanceOf(User).isRequired,
};

Zmian było sporo i miejscami uciążliwe, jednak liczę na to, że inwestycja się zwróci. Jeśli chcesz podejrzeć te wszystkie zmiany, zajrzyj do pull requesta. Projekcik malutki, ale skala zmian bardzo duża. Dobrze, że zająłem sie nimi tak wcześnie.

Do poczytania

Na GitHubie można przeczytać wszystkie rekomendacje od Airbnb dotyczące zarówno samego JavaScriptu, jak i Reacta. W dodatku dobre praktyki dla Reacta doczekały się nawet polskiego tłumaczenia.