Сложно о простом: ESLint в команде

    Маленькое введение. Скорее всего этот пост будет интересен только тем, кто знает, что такое ESLint, но всё же сделаю небольшую вводную — а то сам сильно расстраиваюсь, когда открываю публикацию, и она начинается словами “уже 10 лет мы используем ххх, о котором вы конечно же знаете, а написать мы решили про xxx.yyy, что никто никогда не делал, но наверняка это очень круто”.

    Итак, ESLint это крутой инструмент, который позволяет проводить анализ качества вашего кода, написанного на любом выбранном стандарте JavaScript. Он приводит код к более-менее единому стилю, помогает избежать глупых ошибок, умеет автоматически исправлять многие из найденных проблем и отлично интегрируется со многими инструментами разработки (привет, Jetbrains, мы любим вас!). Кстати, он, как и другие линтеры, не обязывает вас к одному какому-то конкретному стилю. Наоборот — вы можете выбрать что-то из лучших практик и доработать по своему усмотрению!


    Среди применений можно найти довольно неожиданные кейсы — например, легаси код может быть легче прогнать автоматическим исправлением линтера, приведя к современным стандартам, в которых лучше видны ошибки, чем пытаться это править как есть. В результате вполне могут получиться невалидные конструкты — но по нашим наблюдениям это означает, что до преобразования там был просто отвратительный код.

    В общем, жить без линтера в Node.JS в 2017 году — это всё равно что писать код в notepad, при этом сидя на одной руке.

    А сегодня я расскажу вам, как мы решили его внедрить, чтобы эффективно работать с ним в команде, а не по одиночке.

    Большие ребята делают проверку линтером как часть процесса их CI, и мы до этого скоро дойдём. Но пока что нам нужно реализовать первичную необходимость — запуск линтера у разработчика, с гарантией, что всё взлетит и будет работать более-менее одинаково.

    Казалось бы — что такого, добавляешь .eslintrc.json в проект, и поехали! Однако возникает вопрос — а как, куда и кем должен ставиться ESLint и пачка необходимых для нашего code style плагинов? Обычно для этого используется три подхода:

    1. Давайте положим их в devDependency;
    2. Давайте никуда их не положим. Пускай у каждого будет глобально стоять mocha\eslint\прочее.
    3. Пускай всё ставить и прогонять проверки будут таск менеджеры вроде gulp или grunt.

    Второй вариант отметается сразу — думаю, понятно, что таким образом у нас пропадает версионирование самого инструмента от проекта к проекту, да и просто ставить глобально кучу инструментов и плагинов — совсем не круто.

    Третий вариант неплох, но до сих пор мы как-то обходились без таск менеджеров. Добавлять их в проекты ради такой задачи это явный overkill.

    Первый вариант в целом является оптимальным для проектов на гитхабе, но плохо подходит для коммерческой разработки. Наш CI предусматривает проведение автотестов на тестовых серверах, а тестовые зависимости прописать кроме devDependencies просто некуда. Но проблема не в этом, а в том, что, в отличие от автотестов, инструменты для линтера не должны попадать на тестовые сервера. Хотя бы по той причине, что тогда проект при раскатке резко начинает весить 200 с лишним мегабайт вместо 30. Кому-то это может показаться незначительным, но для соблюдения PCI DSS стандартов у нас повсеместно используется довольно серьёзное шифрование любой информации, так что раскатка обновления на 200 мегабайт занимает драгоценные минуты. Так что первый вариант нас тоже не устраивает. Подытожим:

    • Линтер и его плагины не должны стоять глобально;
    • Конкретный проект должен иметь привязки к конкретным версиям инструментов;
    • Эти инструменты не должны быть ни в dependencies, ни в devDependencies.

    На первый взгляд напрашивается простое решение — сделать npm скрипт, например, npm run lint-install, который будет дёргать баш скрипт, который ставит через командную строку все пакеты указанных версий. Но, помимо топорности такого решения, так же получается, что часть зависимостей (пусть и девелоперских) выезжает из package.json в некий отдельный баш скрипт… И это вот совсем не круто. Думаем дальше, вспоминаем спецификации package.json. В общем, никто нам не помешает добавить туда и свои какие угодно секции — но хотелось бы следовать неким стандартам.

    Из спецификации package.json вспоминаем, что есть такая довольно странная и редко используемая секция, как peerDependencies:

    In some cases, you want to express the compatibility of your package with a host tool or library, while not necessarily doing a require of this host. This is usually referred to as a plugin. Notably, your module may be exposing a specific interface, expected and specified by the host documentation.

    Автоматически они не ставятся, за исключением небольшого подводного камня:
    NOTE: npm versions 1 and 2 will automatically install peerDependencies if they are not explicitly depended upon higher in the dependency tree. In the next major version of npm (npm@3), this will no longer be the case. You will receive a warning that the peerDependency is not installed instead. The behavior in npms 1 & 2 was frequently confusing and could easily put you into dependency hell, a situation that npm is designed to avoid as much as possible.

    К счастью, npm у нас был уже не 2ой, так что можно было смело использовать данную секцию, не опасаясь внезапных последствий. Но проблема пришла откуда не ждали… Оказалось, что при удалении автоматической установки peerDependencies, авторы npm… Не сделали никакого способа поставить их вручную. Так что всё, что сейчас есть для секции peerDependencies — это предупреждение о том, что они не установлены. Отчасти эти объяснимо, поскольку зависимости эти опциональны, но всё же. У меня есть подозрение, что после такого изменения все разработчики просто перенесли всё в devDependencies… И dependency hell никуда не делся.
    Кстати, не одному мне отсутствие такой опции показалось странным. Есть даже issue по этому поводу — она закрыта, но помечена как patch-welcome. То есть авторы npm в целом согласны, что это косяк — просто у них не хватает времени на исправление…

    Итак, у нас теперь есть секция, но непонятно, как её использовать. 18 лайков той же самой issue есть на вот такое решение:

     npm info . peerDependencies | sed -n 's/^{\{0,1\}[[:space:]]*'\''\{0,1\}\([^:'\'']*\)'\''\{0,1\}:[[:space:]]'\''\([^'\'']*\).*$/\1@\2/p' | xargs npm i

    По-моему, ад. Как тимлид, я просто не могу позволить, чтобы такое пришло в наши проекты…

    В общем, дальше идея пошла в сторону написания или нахождения инструмента, который сам умеет парсить package.json и вызывать npm install — но не так жёстко, как скрипт, описанный выше. Более-менее менее меня удовлетворил npm-install-peers. Минуса у него два:

    1. Если он по какой-то причине не находит установленный в системе npm (через который его сейчас вообще-то ставят), то он… Ставит его заново локально, что вызывает расход времени, трафика, и иногда всякие адовые ошибки.
    2. Он не поддерживает какие бы то ни было аргументы. Если симлинки на windows уже можно включить, и --no-bin-links уже не очень актуален, то --production всё же хочется. Для тех же зависимостей линтера это бы сильно сэкономило время установки.

    Наверное, в близком будущем я просто сделаю аналогичный инсталлер, который делает то же самое, но дёргает npm не как модуль, а через bash. Пусть не так красиво, зато всё понятно с аргументами, и второй раз npm ставить не нужно. А аргументы передавать хочется — хотя бы чтобы не ставить devDependencies от eslint и его плагинов.

    Дальше встаёт вопрос — а как собственно глобально ставить npm-install-peers? Считать, что он есть по умолчанию? Ставить молча при выполнении скрипта? Ставить локально в devDependencies? Мне ни один из вариантов не понравился. В результате удовлетворился вот таким простым решением:

    "lint-install": "npm-install-peers || echo 'Please run npm install -g npm-install-peers first'",

    Такой вариант мне показался наиболее прозрачным для разработчика.

    И всё, что остаётся — добавить скрипт для запуска линтера и собственно нужные нам peerDependencies. Скрипт:

    
        "lint": "./node_modules/eslint/bin/eslint.js app.js routes modules test App.js"
    

    Зависимости:

    
    "peerDependencies": {
        "babel-cli": "^6.23.0",
        "babel-preset-es2015": "^6.22.0",
        "eslint": "^3.16.0",
        "eslint-config-airbnb": "^14.1.0",
        "eslint-plugin-import": "^2.2.0",
        "eslint-plugin-jsx-a11y": "^4.0.0",
        "eslint-plugin-promise": "^3.4.2",
        "eslint-plugin-react": "^6.10.0",
        "eslint-plugin-standard": "^2.0.1"
      }
    

    Кстати, как побочную фичу, мы теперь можем вынести в peerDependencies всякие прочие зависимости, которые не относятся к тестам — например, божественный jsdoc-to-markdown.

    Казалось бы — простая задача… Но всяких интересных нюансов оказалось довольно много. И я вполне допускаю, что можно было сделать проще и лучше. А как вы у себя используете линтеры для корпоративных проектов?
    Support the author
    Share post
    AdBlock has stolen the banner, but banners are not teeth — they will be back

    More
    Ads

    Comments 29

      +2
      Так о чем статья? как сложно устанавливать линт?
      Где тогда описание использования утилиты в команде и т.п.?
        0
        Статья о том, как его интегрировать с учётом инфраструктуры и потребностей команды.
        Использование в команде на текущем этапе не сильно отличается от индивидуального использования. Когда добавим его в наш CI, то, конечно, новые интересные вещи появятся. Но там всё более-менее прозрачно.
          0
          Но там всё более-менее прозрачно.

          Значит установить линт — сложно, а интегрировать его с CI — тривиально?


          в отличие от автотестов, инструменты для линтера не должны попадать на тестовые сервера

          Обычно перед тестированием сначала идёт прогон линтера. Если же считаем, что код в репозитории по определению "чистый", то хотя бы хуки на коммит всё равно надо поставить.

            0
            Взять и установить линтер — просто. Установить с учётом особенностей окружения — сложнее. А интегрировать с CI действительно несложно. Если у вас уже есть в CI автотесты, то просто добавляется ещё один скрипт, который должен возвратить 0. На примере Jenkins или Travis — это действительно элементарно.

            Естественно должны быть хуки. Но читайте внимательнее — это будет чуть дальше. На текущий момент нужно было хотя бы дать разработчиком возможность использовать сам линтер. Хотя бы чтобы было время спокойно отрефакторить код до добавления хуков — а не блокировать всю разработку тем, что внезапно код перестал уходить в репозиторий.
        +3
        >Третий вариант неплох, но до сих пор мы как-то обходились без таск менеджеров. Добавлять их в проекты ради такой задачи это явный overkill.

        Вы уж меня простите, но вы там командой чтоли лендинги какие-то пилите? В 2017 году без таск-раннеров и линтинга со старта это жесть какая-та.

        В любом нормальном проекте линтер должен прогонятся после каждого изменения кода, + обязательный прогон перед пушем (вместе с тестами), чтобы не пропустить никакую фигню. И для такого не обязательно иметь большую и серьезную команду, уже с двумя разработчиками жизнь без таких вещей превращается в ад.
          +2
          Не лендинги, ровно наоборот. Микросервисы без всякого визуального обвеса. Отлично обходимся без таск раннеров.

          А отсутствие линтера обусловлено банальным наследованием легаси и большой загрузкой. Вернуться в прошлое и добавить линтер, к сожалению, не представляется возможным. Новые проекты, естественно, стартуют уже с линтером.
          0

          Не понял часть про PCI DSS: зачем соблюдать PCI DSS на тестовых серверах, и еще зачем для этого все шифровать? Прошу прощения за такой вопрос, мои знания о PCI DSS очень поверхностные.

            0
            Увы, это вопрос к нашим прокачанным по этой теме девопсам. Кажется, у них есть задача обеспечения идентичности работы тестовой и боевой среды.
              0

              А шифровать для PCI DSS зачем все? Я по диагонали читал материалы о PCI DSS, но там вроде о тотальном шифровании кода и всех зависимостей вообще не говорится. Или я пропустил, это все такая занудная и плохо структурированная тема, читал скорее для общего развития, чем из необходимости.

                0
                Шифруется весь трафик, проходящий между серверами. В том числе код при выкатке.
                  0

                  А если код на выкатку передавать в tar.gz? Трафика меньше — передача быстрее. Всё равно ведь однажды объём зависимостей увеличится.

                    0
                    Естественно, туда передаётся архивированная информация. Это архив со всем добром занимает 200 мегабайт. Подозреваю, что архивации используется с малой степенью компрессии. Ну тут выбор — экономия трафика или CPU. Логичнее использовать второе.
                    А с ростом объёма зависимостей мы успешно боремся. Со временем их становится меньше.
                      0

                      Просто у меня eslint + плагины + airbnb в исходном виде в node_modules занимают 26 Мб, так что не совсем очевидно, где тут экономия от его сокрытия.

                        0
                        По месту смогу точно сказать в первый следующий рабочий день — у меня почему-то гораздо больше вышло.
                        Но наши микросервисы занимают каждый не более 30 мегабайт, так что даже если так, то экономия выходит в два раза.
            0
            Как написано в статье, хочу быть как крутые ребята, и уже поставил в задачи внедрить его в CI, поэтому не парюсь и ставлю все зависимости в devDep. Это не сильно увеличивает время сборки проекта при деплое, около ~15 секунд (это было посчитано нашими деплойками).

            Правда коллеги приводят довод, что если какой аврал будет, то из — за этого линтера не получится в срочном порядке вылить изменения на прод, а я фыркаю на них…
              0
              Ну как же так. Обязательно нужно иметь возможность в срочном порядке вылить баг на прод!
              +2

              Бывшие C#-овцы. Используем TypeScript и TSlint и горя не знаем. И вам рекомендуем. :)
              А для прогона кода между серверами у нас гит с https.

                0
                Видимо, у вас нету билд сервера. С ним всё несколько интереснее выходит — но на мой взгляд его наличие оправдано.
                  0

                  Билд сервер — не нужен. Все собираеться за 6-8 секунд и пушиться в гит, откуда его забирает сервер. Мы билд сервер держим только для Android и Quartus, и то потому что иметь 64 Гб памяти на ноутбуках разработчиков невозможно.

                    0

                    Еще у фронтендеров на билд сервере сжатие картинок.

                0
                Мне не совсем понятна ваша проблема с devDependency. Правильно ли я понимаю, что в продакшн у вас улетает пакет со всем, что лежит в dependency и devDependency? Если так, то это достаточно странно.

                Если вы гоняете автотесты на престейбле, то возможно логичнее выделить тестраннер в отдельный пакет, и все его зависимости перенести в dependency этого пакета. Это должно сэкономить вам еще драгоценное место.

                devDependency как мне кажется должны использоваться для другого. В рамках продакш пакета это то место, где вы можете позволить себе указывать неточные версии модулей, потому что знаете, что эти модули не утекут в продакшн.

                К слову, если вы заботитесь о надежности сборки пакета, то наиболее надежным подходом будет установка через shrinkwrap из локальной директории. В этом случае вы сможете выкатить хотфикс, даже если npm сломан.
                  0
                  В продакшн, конечно, devDependency не улетает. Как я писал, речь идёт о тестовых серверах.

                  Выделять тесты в отдельный пакет, который будет в отдельном проекте и репозитории — это нарушать целостность кода. Кроме того, если мы его ставим как devDependency, то он прилетает со всеми своими вложенными devDependency.
                    0
                    Upd. Пардон, попутал, рекурсивная установка devDependencies была багом в некий момент. Но всё равно не хочется тесты в отдельный пакет выделять. А если под тест раннером вы имеете в виду не сами тесты, а окружение для из выполнения, то тут тоже всё не так просто — разные микросервисы могут иметь разный набор требуемых пакетов для тестирования. И придётся либо делать один мега-раннер, в котором собирать всё-всё-всё, либо по раннеру на каждый пакет.
                      0

                      В данном случае я не предлагал вам уносить тесты из проекта, только тестраннер.


                      Предположим вы используете какой-нибудь стандартный стек тестирования (karma + mocha + chai + istanbul) для project.
                      Хотя, раз речь идет о микросервисах, вероятно вы обходитесь без karma.
                      Определенно именно такая связка у вас должна запускаться в dev окружении. Но в продакшн вам не нужен istanbul, а значит и устанавливать его не имеет смысла.


                      Создаем еще один пакет project-testrunner и собираем у него в зависимостях все, что нужно для тестирования в продакшн.
                      Для project прописываем в некотором postinst (уж не знаю, какие именно у вас пакеты):


                      # Плохой пример поиска пакета. Лучше поставить этот пакет как глобальный npm модуль.
                      if [ -d ../project-testrunner ]; then
                          ../project-testrunner/bin/test project/test
                      else
                          echo "Осмысленная ошибка о том, что пакет тестраннера не установлен на машину"
                      fi

                      Т.е. мы все так же прогоняем тесты над директорией project/test, но делаем это с помощью внешнего пакета.
                      Для dev мы можем указать project-testrunner как devDependency и запускать например так:


                      #package.json
                      
                      "devDependencies": {
                          "project-testrunner": "*"
                      },
                      "scripts": {
                          "test": "project-testrunner ./test --coverage"
                      }

                      Конечно придется ручками написать скрипт testrunner, но это всяко лучше, чем использовать peerDependency не по назначению.
                      При этом istanbul можно указать как devDependency для project-testrunner

                        0
                        Ммм, а сам testrunner при этом где будет прописан в зависимостях? В devDependencoes его вы вроде предложили не класть.Если в dependencies, то по месту это ничего не меняет — наоборот, эта зависимость начнёт уходить на продакшн. А если он не прописан нигде и стоит на билд сервере, то это усложняет логику тестирования и приводит к неожиданным последствиям в случае разности локального и серверного testrunner'a.

                        Насчёт зависимостей для тестов — есть ещё одна хорошая issue, в которой предлагают ввести секцию testDependencies, что имхо правильно, но пока что документация по package.json говорит о том, что тесты должны быть в devDependencies.
                    0
                    "lint": "./node_modules/eslint/bin/eslint.js app.js routes modules test App.js"
                    


                    Лучше написать так:

                    "lint": "eslint app.js routes modules test App.js"
                    

                      0
                      Это если eslint глобально стоит или привязан симлинком. Мы решили, что лучше использовать локальный eslint — чтобы исключить расхождение по версиям.
                        0

                        Нет, в данном случае npm запустит локальный eslint из ./node-modules/.bin/eslint.

                          0
                          npm env = shell $PATH + node executables stuff via npm

                      Only users with full accounts can post comments. Log in, please.