Skip to content

feat(readme): update prettier-eslint config #84

Merged
merged 4 commits into from
Aug 26, 2020

Conversation

etroynov
Copy link
Contributor

На данный момент при запуске команды format мы получаем ошибку:

prettier-eslint [ERROR]: There was trouble creating the ESLint CLIEngine.
prettier-eslint-cli [ERROR]: There was an error formatting "src/index.ts": 
    AssertionError [ERR_ASSERTION]: 'basePath' should be an absolute path.

Данная проблема описана тут:
prettier/prettier-eslint-cli#208

Решение:

        "format": "prettier-eslint --write \"./{config,src}/**/*.{ts,tsx,js,jsx,json,css}\""

заменяем на:

        "format": "prettier-eslint --write \"$PWD/{config,src}/**/*.{ts,tsx,js,jsx,json,css}\""

@etroynov etroynov self-assigned this Aug 14, 2020
@etroynov etroynov added bug Something isn't working dependencies Pull requests that update a dependency file enhancement New feature or request and removed dependencies Pull requests that update a dependency file labels Aug 14, 2020
@voronin-ivan
Copy link
Contributor

Блин, казалось, что это решалось фиксом какой-то определенной версии пакета :(

README.md Outdated
@@ -66,7 +66,7 @@ npm info "arui-presets-lint@latest" peerDependencies
"lint:css": "stylelint ./src/**/*.css",
"lint:scripts": "eslint \"**/*.{js,jsx,ts,tsx}\" --ext .js,.jsx,.ts,.tsx",
"lint": "yarn lint:css && yarn lint:scripts",
"format": "prettier-eslint --write \"./{config,src}/**/*.{ts,tsx,js,jsx,json,css}\""
"format": "prettier-eslint --write \"$PWD/{config,src}/**/*.{ts,tsx,js,jsx,json,css}\""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это не будет работать на винде. Давай затащим cross-env и пропишем как cross-env prettier-eslint --write $INIT_CWD\"/{config,src}/**/*.{ts,tsx,js,jsx,json,css}\""

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier/prettier-eslint-cli#208 (comment) так а вот так не будет работать?

Copy link
Contributor Author

@etroynov etroynov Aug 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

у нас есть пользователи винды? Насколько я знаю в альфе вся разработка ведется на маке и это стандарт, под виндой есть WSL/WSL 2.

Кстати у нас вроде есть пара библиотек в некоторых проектах которые на чистой винде не будут работать ни при каком раскладе, как быть с ними тогда?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так же хотел бы указать что тот вариант который указан в README не работает на маке, таким образом решение которое указано как целевое не должно работать у подавляющего количество сотрудников, а не только у пользователей винды.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

у нас есть пользователи винды?

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

Кстати у нас вроде есть пара библиотек в некоторых проектах которые на чистой винде не будут работать ни при каком раскладе, как быть с ними тогда?

Не сталкивался) Будет круто, если знаешь, какие именно либы, можно тогда будет issue оформить.

Так же хотел бы указать что тот вариант который указан в README не работает на маке, таким образом решение которое указано как целевое не должно работать у подавляющего количество сотрудников, а не только у пользователей винды.

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

Copy link
Contributor Author

@etroynov etroynov Aug 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

заменил на решение от @stepancar. На винде не тестил т.к у меня её нет.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это не будет работать на винде. Давай затащим cross-env и пропишем как cross-env prettier-eslint --write $INIT_CWD"/{config,src}/**/*.{ts,tsx,js,jsx,json,css}""

Кстати это будет работать в yarn 2 без cross-env, т.к у него есть встроенный мини шелл.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2020-08-26 at 15 37 07

Решение чувака из issue выше не работает, если нет ошибок, это не значит, что команда вообще отрабатывает. $INIT_CWD работает норм, но не на винде

@voronin-ivan voronin-ivan force-pushed the fix/prettier-eslint-readme branch from fcfbcea to ac10146 Compare August 26, 2020 15:36
@voronin-ivan
Copy link
Contributor

С аппрува @etroynov внес правки:

  • поменял саму команду на работающую
  • запустил yarn format и закоммитил измененный файл
  • выпилил json из проверок, т.к. поддержки сейчас нет

@voronin-ivan voronin-ivan merged commit e9770c5 into master Aug 26, 2020
@voronin-ivan voronin-ivan deleted the fix/prettier-eslint-readme branch August 26, 2020 15:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants