feat: add restrict-plus-operands

[leviFrosty]

https://typescript-eslint.io/rules/restrict-plus-operands/

[leviFrosty]

Will fix CI

[leviFrosty]

I do not like this change for various reasons... The only thing I can think to do is turn this into a mini mono-repo with a nested project within ./test-cases that installs the root eslint-config package, sets up its' own tsconfigs, tooling, etc. Then update CI to call this test-cases project within the workspace.

Otherwise, as it stands users of this package will get this dependencies installed automatically, which isn't ideal because some node-only projects will not want vue as a dev dep.

I think I'll probably refactor this, but just wanted to get out a working version and then see what you thought @onebytegone

[leviFrosty]

Formats to 3 spaces (like some other repos)... Not really supposed to be included in this PR but I added here anyway

[leviFrosty]

Notable changes lines 48-50

[leviFrosty]

This add restrict-plus-operands is the first eslint plugin rule that we have enabled that requires a typescript project config available. It needs access to type information in order to give more powerful linting options.

See https://typescript-eslint.io/getting-started/typed-linting/

[yokuze]

I do not like this change for various reasons... The only thing I can think to do is turn this into a mini mono-repo with a nested project within ./test-cases that installs the root eslint-config package, sets up its' own tsconfigs, tooling, etc. Then update CI to call this test-cases project within the workspace.

Otherwise, as it stands users of this package will get this dependencies installed automatically, which isn't ideal because some node-only projects will not want vue as a dev dep.

I think I'll probably refactor this, but just wanted to get out a working version and then see what you thought @onebytegone

I haven't looked at this closely enough to understand the overarching issue that you're suggesting the monorepo for, but just wanted to point out that any package that uses this package shouldn't get Vue because it's a devDependency, in case that helps.

[leviFrosty]

I do not like this change for various reasons... The only thing I can think to do is turn this into a mini mono-repo with a nested project within ./test-cases that installs the root eslint-config package, sets up its' own tsconfigs, tooling, etc. Then update CI to call this test-cases project within the workspace.
Otherwise, as it stands users of this package will get this dependencies installed automatically, which isn't ideal because some node-only projects will not want vue as a dev dep.
I think I'll probably refactor this, but just wanted to get out a working version and then see what you thought @onebytegone

I haven't looked at this closely enough to understand the overarching issue that you're suggesting the monorepo for, but just wanted to point out that any package that uses this package shouldn't get Vue because it's a devDependency, in case that helps.

Honestly kind of blanked and forgot devDeps don't get inherited. Think this PR should be OK to merge as is then?

[leviFrosty]

Consumers of this update will need to add this to their eslint config if they do not already have it.

Don't know how we'd want to document this breaking change -- but notable here.

{
    // eslint.config.js
    ...
    files: [ '**/*.ts' ],
    languageOptions: {
       parserOptions: {
          project: 'tsconfig.json', // <----- THIS LINE
       },
    },
    ...
}

[leviFrosty]

Alright, I've added the readme.md examples as well as the update to the commit message, please have a look over them.
Hopefully they're concise enough to get across the point without omitting too many details.

Should be good to merge now @yokuze

[yokuze]

It looks like the target of the ...config spread moved from the array to an object that's surrounding all of the config properties. Also, there's a syntax error: The { ...typescript block is nested within that object, but without a key.

[yokuze]

Which makes me think that with these extra levels of nesting the spread syntax approach is just a bit too onerous now.

Could you look at how we may be able to use ESLint's extend property instead? Maybe something like:

[
   {
      files: [ '**/*.ts' ],
      languageOptions: {
         parserOptions: {
            project: './tsconfig.node.json',
         },
      },
      extends: [ typescript ],
   }
]

? Not sure - you'd have to check that ESLint merges the config keys/values in the way that would make that work without blowing away the other languageOptions, for example.

[leviFrosty]

Hey, sorry about misaligned spread syntax. Resolved that issue on latest change. Also simplified the example as, like you said, it's a bit onerous.

Tested on another large repo with my forked commit hash. Tested working as presented in this example as well as a few other configuration options that are more involved.

Regarding the extends key, it's been deprecated/removed in v9, only enabled via some compat functions. Now that this example has been simplified to only highlight the necessary changes, I think we'll be fine to leave as-is for now and return to this issue when we upgrade to eslint v9.

Oops! Something went wrong! :(

ESLint: 8.57.1

A config object is using the "extends" key, which is not supported in flat config system.

Instead of "extends", you can include config objects that you'd like to extend from directly in the flat config array.

Please see the following page for more information:
https://eslint.org/docs/latest/use/configure/migration-guide#predefined-and-shareable-configs

[yokuze]

There's a "Vue Support" heading below - it seems like this belongs there, especially now that it's required config for Vue.

[leviFrosty]

Thanks for catching. I moved all of the Vue related config notes within the Vue subheading.

[yokuze]

Same issue here with the top-level { }

[leviFrosty]

Resolved this as well. Thanks

[yokuze]

Looks great, thank you!