Open matwilko opened 1 year ago
hardcore/ts
is intended to be a superset of hardcore/ts-for-js
, i. e. hardcore/ts
is intended to include all rules from hardcore/ts-for-js
, plus TypeScript-specific rules.
Could you explain how it makes things much messier?
Sure, from my perspective, when I explicitly pulled in ts.json
, my expectation was that I was enabling just Typescript support from hardcore
. The fact that JS stuff came along for the ride was surprising when there's a separate config to enable that, and my expectation was that JS-specific stuff wouldn't be enabled unless I pulled in that config as well because the README states "additional config for linting JavaScript".
The messiness comes from the fact that ts-for-js
is globally scoped (i.e. applies to all files), and and so when I saw that it was being included, I had to spend some time looking through to see what it was applying to Typescript files as well as JS files, and I quickly gave up trying to reconcile ts-for-js
and ts
, and patched ts.json
to remove the ts-for-js.json
dependency, thinking it was only touching rules intended for JS files, and safe to simply exclude.
If ts
is intended as a superset of ts-for-js
, then I think this either needs making explicit in the README, or the rules in ts-for-js
need to be merged into ts
to properly separate the two configs, and make sure that ts-for-js
is actually an additional config just for JS files.
In any case, I believe ts-for-js
absolutely needs to be modified to use an override rather than being global. The README already recommends using an override scoping its use to *.js
files.
This is because the lack of scoping is going to start causing bugs when the config is updated to the new Flat Config system - because ts-for-js
is globally scoped, it will override ts
if it is included in the flat config after ts
, that is:
// Speculative names from future version supporting flat config
import { ts, tsForJs } from 'eslint-config-hardcore';
export default [
ts,
tsForJs // a user might do this because they assume that ts-for-js is a separate config they need to include for JS files!
];
Would result in ts-for-js
overriding ts
's different configuration for .ts
files for any rules that they share in common. And it's not unreasonable for a user to think the above is a valid configuration, given the documented usage.
I think the fix for this is to completely separate the two configs so that they can be included in any order in a flat config (because they have different files: [...]
specifications).
So, overall, I think the ask here is that either any interdependencies/relationships between the configs are made explicit in the README so users know what they're getting, and/or make the configs truly independent and live up to their documented use, so that when I include one, I'm getting just what I intended to include, and the configs are all additive and non-conflicting.
Currently, importing
ts.json
also importsts-for-js.json
. This changes a lot of things at a global level, assuming that the user has kept the ESLint default of linting all.js
files.However, in my project, I made
.ts
files the default, and explicitly ban.js
files, so when I importts.json
and get all thets-for-js
rules along for the ride on my.ts
, it makes things much messier!Given that
ts-for-js
is listed separately on theREADME.md
, this behaviour was unexpected, as my expectation was that if I had both.ts
and.js
files, I would need to import both configs myself.I'd like to suggest that
ts.json
remove thets-for-js.json
import, and make it clear to users that they will need to import both if they have both.ts
and.js
files they want to lint with TypeScript.I know that's a breaking change, so as a non-breaking, but potentially more compatible alternative, could
ts-for-js.json
be updated to use{ overrides: { files: ["**/*.js"], rules: { ... } }
?