Итак, ESLint это крутой инструмент, который позволяет проводить анализ качества вашего кода, написанного на любом выбранном стандарте JavaScript. Он приводит код к более-менее единому стилю, помогает избежать глупых ошибок, умеет автоматически исправлять многие из найденных проблем и отлично интегрируется со многими инструментами разработки (привет, Jetbrains, мы любим вас!). Кстати, он, как и другие линтеры, не обязывает вас к одному какому-то конкретному стилю. Наоборот — вы можете выбрать что-то из лучших практик и доработать по своему усмотрению!
Среди применений можно найти довольно неожиданные кейсы — например, легаси код может быть легче прогнать автоматическим исправлением линтера, приведя к современным стандартам, в которых лучше видны ошибки, чем пытаться это править как есть. В результате вполне могут получиться невалидные конструкты — но по нашим наблюдениям это означает, что до преобразования там был просто отвратительный код.
В общем, жить без линтера в Node.JS в 2017 году — это всё равно что писать код в notepad, при этом сидя на одной руке.
А сегодня я расскажу вам, как мы решили его внедрить, чтобы эффективно работать с ним в команде, а не по одиночке.
Большие ребята делают проверку линтером как часть процесса их CI, и мы до этого скоро дойдём. Но пока что нам нужно реализовать первичную необходимость — запуск линтера у разработчика, с гарантией, что всё взлетит и будет работать более-менее одинаково.
Казалось бы — что такого, добавляешь .eslintrc.json в проект, и поехали! Однако возникает вопрос — а как, куда и кем должен ставиться ESLint и пачка необходимых для нашего code style плагинов? Обычно для этого используется три подхода:
- Давайте положим их в devDependency;
- Давайте никуда их не положим. Пускай у каждого будет глобально стоять mocha\eslint\прочее.
- Пускай всё ставить и прогонять проверки будут таск менеджеры вроде 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. Минуса у него два:
- Если он по какой-то причине не находит установленный в системе npm (через который его сейчас вообще-то ставят), то он… Ставит его заново локально, что вызывает расход времени, трафика, и иногда всякие адовые ошибки.
- Он не поддерживает какие бы то ни было аргументы. Если симлинки на 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.
Казалось бы — простая задача… Но всяких интересных нюансов оказалось довольно много. И я вполне допускаю, что можно было сделать проще и лучше. А как вы у себя используете линтеры для корпоративных проектов?
Комментарии (29)
Ronnie_Gardocki
25.02.2017 16:26+3>Третий вариант неплох, но до сих пор мы как-то обходились без таск менеджеров. Добавлять их в проекты ради такой задачи это явный overkill.
Вы уж меня простите, но вы там командой чтоли лендинги какие-то пилите? В 2017 году без таск-раннеров и линтинга со старта это жесть какая-та.
В любом нормальном проекте линтер должен прогонятся после каждого изменения кода, + обязательный прогон перед пушем (вместе с тестами), чтобы не пропустить никакую фигню. И для такого не обязательно иметь большую и серьезную команду, уже с двумя разработчиками жизнь без таких вещей превращается в ад.
jehy
25.02.2017 16:32+2Не лендинги, ровно наоборот. Микросервисы без всякого визуального обвеса. Отлично обходимся без таск раннеров.
А отсутствие линтера обусловлено банальным наследованием легаси и большой загрузкой. Вернуться в прошлое и добавить линтер, к сожалению, не представляется возможным. Новые проекты, естественно, стартуют уже с линтером.
immaculate
25.02.2017 16:48Не понял часть про PCI DSS: зачем соблюдать PCI DSS на тестовых серверах, и еще зачем для этого все шифровать? Прошу прощения за такой вопрос, мои знания о PCI DSS очень поверхностные.
jehy
25.02.2017 17:09Увы, это вопрос к нашим прокачанным по этой теме девопсам. Кажется, у них есть задача обеспечения идентичности работы тестовой и боевой среды.
immaculate
25.02.2017 17:23А шифровать для PCI DSS зачем все? Я по диагонали читал материалы о PCI DSS, но там вроде о тотальном шифровании кода и всех зависимостей вообще не говорится. Или я пропустил, это все такая занудная и плохо структурированная тема, читал скорее для общего развития, чем из необходимости.
jehy
25.02.2017 17:26Шифруется весь трафик, проходящий между серверами. В том числе код при выкатке.
staticlab
25.02.2017 21:40А если код на выкатку передавать в tar.gz? Трафика меньше — передача быстрее. Всё равно ведь однажды объём зависимостей увеличится.
jehy
25.02.2017 21:51Естественно, туда передаётся архивированная информация. Это архив со всем добром занимает 200 мегабайт. Подозреваю, что архивации используется с малой степенью компрессии. Ну тут выбор — экономия трафика или CPU. Логичнее использовать второе.
А с ростом объёма зависимостей мы успешно боремся. Со временем их становится меньше.staticlab
26.02.2017 00:28Просто у меня eslint + плагины + airbnb в исходном виде в node_modules занимают 26 Мб, так что не совсем очевидно, где тут экономия от его сокрытия.
jehy
26.02.2017 00:33По месту смогу точно сказать в первый следующий рабочий день — у меня почему-то гораздо больше вышло.
Но наши микросервисы занимают каждый не более 30 мегабайт, так что даже если так, то экономия выходит в два раза.
Luchnik22
25.02.2017 19:21Как написано в статье, хочу быть как крутые ребята, и уже поставил в задачи внедрить его в CI, поэтому не парюсь и ставлю все зависимости в devDep. Это не сильно увеличивает время сборки проекта при деплое, около ~15 секунд (это было посчитано нашими деплойками).
Правда коллеги приводят довод, что если какой аврал будет, то из — за этого линтера не получится в срочном порядке вылить изменения на прод, а я фыркаю на них…jehy
26.02.2017 00:35Ну как же так. Обязательно нужно иметь возможность в срочном порядке вылить баг на прод!
Armleo
25.02.2017 19:47+2Бывшие C#-овцы. Используем TypeScript и TSlint и горя не знаем. И вам рекомендуем. :)
А для прогона кода между серверами у нас гит с https.jehy
26.02.2017 00:38Видимо, у вас нету билд сервера. С ним всё несколько интереснее выходит — но на мой взгляд его наличие оправдано.
Armleo
26.02.2017 07:05Билд сервер — не нужен. Все собираеться за 6-8 секунд и пушиться в гит, откуда его забирает сервер. Мы билд сервер держим только для Android и Quartus, и то потому что иметь 64 Гб памяти на ноутбуках разработчиков невозможно.
3y3
26.02.2017 01:06Мне не совсем понятна ваша проблема с devDependency. Правильно ли я понимаю, что в продакшн у вас улетает пакет со всем, что лежит в dependency и devDependency? Если так, то это достаточно странно.
Если вы гоняете автотесты на престейбле, то возможно логичнее выделить тестраннер в отдельный пакет, и все его зависимости перенести в dependency этого пакета. Это должно сэкономить вам еще драгоценное место.
devDependency как мне кажется должны использоваться для другого. В рамках продакш пакета это то место, где вы можете позволить себе указывать неточные версии модулей, потому что знаете, что эти модули не утекут в продакшн.
К слову, если вы заботитесь о надежности сборки пакета, то наиболее надежным подходом будет установка через shrinkwrap из локальной директории. В этом случае вы сможете выкатить хотфикс, даже если npm сломан.jehy
26.02.2017 01:14В продакшн, конечно, devDependency не улетает. Как я писал, речь идёт о тестовых серверах.
Выделять тесты в отдельный пакет, который будет в отдельном проекте и репозитории — это нарушать целостность кода. Кроме того, если мы его ставим как devDependency, то он прилетает со всеми своими вложенными devDependency.jehy
26.02.2017 01:31Upd. Пардон, попутал, рекурсивная установка devDependencies была багом в некий момент. Но всё равно не хочется тесты в отдельный пакет выделять. А если под тест раннером вы имеете в виду не сами тесты, а окружение для из выполнения, то тут тоже всё не так просто — разные микросервисы могут иметь разный набор требуемых пакетов для тестирования. И придётся либо делать один мега-раннер, в котором собирать всё-всё-всё, либо по раннеру на каждый пакет.
3y3
26.02.2017 02:00В данном случае я не предлагал вам уносить тесты из проекта, только тестраннер.
Предположим вы используете какой-нибудь стандартный стек тестирования (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-testrunnerjehy
26.02.2017 02:30Ммм, а сам testrunner при этом где будет прописан в зависимостях? В devDependencoes его вы вроде предложили не класть.Если в dependencies, то по месту это ничего не меняет — наоборот, эта зависимость начнёт уходить на продакшн. А если он не прописан нигде и стоит на билд сервере, то это усложняет логику тестирования и приводит к неожиданным последствиям в случае разности локального и серверного testrunner'a.
Насчёт зависимостей для тестов — есть ещё одна хорошая issue, в которой предлагают ввести секцию testDependencies, что имхо правильно, но пока что документация по package.json говорит о том, что тесты должны быть в devDependencies.
renya
02.03.2017 13:40"lint": "./node_modules/eslint/bin/eslint.js app.js routes modules test App.js"
Лучше написать так:
"lint": "eslint app.js routes modules test App.js"
Focushift
Так о чем статья? как сложно устанавливать линт?
Где тогда описание использования утилиты в команде и т.п.?
jehy
Статья о том, как его интегрировать с учётом инфраструктуры и потребностей команды.
Использование в команде на текущем этапе не сильно отличается от индивидуального использования. Когда добавим его в наш CI, то, конечно, новые интересные вещи появятся. Но там всё более-менее прозрачно.
staticlab
Значит установить линт — сложно, а интегрировать его с CI — тривиально?
Обычно перед тестированием сначала идёт прогон линтера. Если же считаем, что код в репозитории по определению "чистый", то хотя бы хуки на коммит всё равно надо поставить.
jehy
Взять и установить линтер — просто. Установить с учётом особенностей окружения — сложнее. А интегрировать с CI действительно несложно. Если у вас уже есть в CI автотесты, то просто добавляется ещё один скрипт, который должен возвратить 0. На примере Jenkins или Travis — это действительно элементарно.
Естественно должны быть хуки. Но читайте внимательнее — это будет чуть дальше. На текущий момент нужно было хотя бы дать разработчиком возможность использовать сам линтер. Хотя бы чтобы было время спокойно отрефакторить код до добавления хуков — а не блокировать всю разработку тем, что внезапно код перестал уходить в репозиторий.