Skip to content
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

Separate ts-for-js and ts #772

Open
matwilko opened this issue Jul 21, 2023 · 2 comments
Open

Separate ts-for-js and ts #772

matwilko opened this issue Jul 21, 2023 · 2 comments

Comments

@matwilko
Copy link

Currently, importing ts.json also imports ts-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 import ts.json and get all the ts-for-js rules along for the ride on my .ts, it makes things much messier!

Given that ts-for-js is listed separately on the README.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 the ts-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: { ... } }?

@EvgenyOrekhov
Copy link
Owner

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?

@matwilko
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants