parse5
parse5 copied to clipboard
chore: cleanup eslint config
This is prep work for me turning the non-null-assertion rule on to error
.
Summary:
- eslint:recommended wasn't enabled for
*.ts
so i have enabled it - unicorn wasn't used at all so i got rid of it (and conflicted with a bunch of stuff when it was)
- disabled the
no-unused-vars
rule since we get this from the compiler anyway (noUnusedLocals
,noUnusedParameters
) - fixed the newly discovered lint errors
@fb55
From my understanding, the overrides
section extends the base lints. This is also supported by the test failure in #411, which reported several issues found with the latest version of eslint-plugin-unicorn
.
I am very curious where the new errors came from, as most of the lint settings shouldn't have changed.
it does extend the base one, except for the extends
array. that gets replaced.
ts
files didn't have the unicorn rules or the recommended rules because of that
A glob specific configuration can have an extends setting, but the root property in the extended configs is ignored
we get all we need really from eslint, typescript-eslint and prettier.
ts
files didn't have the unicorn rules or the recommended rules because of that
I am a bit confused as to why we saw errors in https://github.com/inikulin/parse5/runs/5282607937 👀
same :D
all sorts of conflicts happened when i added eslint's rules. especially since half of unicorn's rules are stylistic... we have prettier for that.
if i get chance i can have a look into it, but tbh we're not losing anything important here. i'd still get this and the other blocked pr merged
edit:
i tried reading through eslint's source to see what it really does but it gets spaghetti-like quickly. so im just trusting the docs are right here and assuming we ran into a bug or some unexpected behaviour. with this, we're at least following what is documented and have what we need
I just tried replicating this locally, and adding eslint:recommended
to the override doesn't change anything for me. The override extends the prettier
preset and therefore disables the curly
rule that is specified in the rules
section. It seems like most of the changes in this PR are due to the curly
rule, without it actually being added to the override.
I'd like to keep eslint-plugin-unicorn
, as almost none of the rules are actually covered by prettier. Eg. array.filter((e) => ...)[0]
should be replaced with a .find
, but prettier will never report this (this is a pattern that was actually a part of the codebase).
I’m also a fan of xo / unicorn and would prefer to keep it. While it can be quite annoying at times, it does a ton of things that are useful for clean code, which eslint:recommended and prettier don‘t catch.
ill put it back before we merge, will update once i've had chance
the docs are worded poorly, i think i misunderstood them. apparently an overridden extends
should also merge with the base one.
so im pretty confused as to why curly
gets enabled here.
this config is by the docs, exactly as they say it should work.
i also switched unicorn and prettier. prettier is right, we don't care for any stylistic rules from unicorn, we have a formatter for that. the other way around, there are some rules which can conflict.
so im pretty confused as to why curly gets enabled here.
curly
is disabled in eslint-config-prettier
, which is part of the override.
yup it is. im aware, i understand that.
but it isn't disabled with this config. even though prettier is extended