-
-
Notifications
You must be signed in to change notification settings - Fork 426
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
feat: add lintfix and prettier scripts #829
feat: add lintfix and prettier scripts #829
Conversation
_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of lintfix
results thanks to prettier; I think we should integrate it to the template.
prettier: '<%= pmRun %> lint:prettier' | ||
} | ||
const lintfixScripts = { | ||
eslint: "<%= pmRun %> lint:js <%= pm === 'npm' ? '-- ' : '' %>--fix", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used EJS templates because there already was <%= pmRun %>
, but I don't see a reason not to `use ${variables} instead`
; especially as any incorrect templates in this JS cause a compilation error, e.g. if you escape \'
within "
instead of replacing '
by "
and using '
inside. Also the whitespace-trim didn't seem to be very useful here.
delete pkg.devDependencies.husky | ||
delete pkg.scripts.prepare | ||
} else { | ||
// Move prepare to make it the last script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of moving, we could also create it here.
@@ -2365,12 +2374,17 @@ Generated by [AVA](https://avajs.dev). | |||
'core-js': '^3.15.1', | |||
nuxt: '^2.15.7', | |||
}, | |||
devDependencies: {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the lint-staged was selected alone, which previously didn't install it.
I'd also suggest removing the .prettierrc and instead moving its content to
|
e981d50
to
32e7adf
Compare
32e7adf
to
6585d22
Compare
Reordered |
…-prettier-scripts
lint:prettier
script to provide users with a recommended way to enforce a unified code style in the project, which is less accessible if they have to research the correct usage, or even access it via npx. The intention should be to provide a configuration where linters don't conflict with each other. Also included this in thelint
script by default to enforce best practices (but anyone is free to remove it). I can't comprehend why new users should be forced to research this on their own; we need a reference solution as a source of truth for example if we want to setup IDE extensions and make sure that they converge on the same code style (definitely not trivial with all the options in VSCode like Vetur, Prettier, Eslint, built-in...)..prettierignore
, especially to exclude directories like.nuxt
. Due to Extend .prettierignore with an "include" syntax prettier/prettier#8288 and Proposal: Filter using ".gitignore" first, then ".prettierignore" second prettier/prettier#8506, we have to duplicate the.gitignore
content without being able to include it e.g. by referencing two ignore files. Users may require this file to differ from their.gitignore
, e.g. if they need to include results of code generators. Note that there is no analogy ofeslint-disable
(Feature proposal: prettier-ignore-next-line prettier/prettier#9310), so this file cannot be avoided. As for code re-use (DRY) in the template file, I was unable to import the content via EJS, and the SAO generator lacks a copy action: Cannot include EJS partials (second issue), and missing copy action saojs/sao#176lintfix
script to provide a quick way of fixing any issues in the entire project, especially before they make a commit. Prettier includes--list-different
to display only changed files (unified with other linters) instead of every single file in the project.prettier
to the beginning oflintfix
to avoid ESLint causing conflicts of rules like no-return-assign (without having to use Prettier via ESlint by installing eslint-plugin-prettier and using plugin:prettier/recommended), fixing fix(prettier): use recommended rules to avoid eslint conflicts, revert #797 #827create-nuxt-app
: runlintfix
at the end instead of hard-coding all of the scripts individually; just like what the user is supposed to do on a regular basis.lint-staged
inpackage.json
to include prettier by default, where it requires--ignore-unknown
Implement a new--ignore-unknown
argument for CLI prettier/prettier#8136, as you can reproduce the issue by trying to commit an unstyled file that's referenced in the.prettierignore
. Additionally use--cache
foreslint
as a recommended performance improvement, which is already assumed as our current.gitignore
includes the.eslintcache
.prepare
at the end. (Or could we move it to the top?).ts
to supportedeslint
types. I'd personally prefer keeping it there even if the project is JS-only, but for the sake of consistency I only added it for TS projects..scss
,.sass
, and.html
to supportedstylelint
types. Fixing CSS<style>
tags inside HTML definitely sounds like an expected default behavior to me. This list may not be exhaustive, so please make sure it's kept up-to-date with any popular use-cases.lintStaged
conditions to include it even withouteslint
. Previously this meant that together with husky they weren't configured if the user has only chosencommitlint
, which I believe is a bug. Now it installs even for prettier, but I also allowed it to be installed as an empty block if chosen alone, because the effects of that selection don't make any sense otherwise; who are we to judge if someone wants to lint something else on staging?