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

parserOptions.project = true #1230

Closed
mightyiam opened this issue Aug 20, 2023 · 4 comments · Fixed by #1244
Closed

parserOptions.project = true #1230

mightyiam opened this issue Aug 20, 2023 · 4 comments · Fixed by #1244
Labels

Comments

@mightyiam
Copy link
Owner

Perhaps eslint-config-standard-with-typescript could specify parserOptions: { project: true } so that the config can work without requiring an additional setup?

eslint/create-config#71 (comment)

rostislav-simonik added a commit that referenced this issue Aug 27, 2023
closes #1230

Co-authored-by: Shahar "Dawn" Or <mightyiampresence@gmail.com>
standard-cd-bot bot pushed a commit that referenced this issue Aug 27, 2023
## [38.1.0](v38.0.0...v38.1.0) (2023-08-27)

### Features

* parserOptions.project=true ([73efcf8](73efcf8)), closes [#1230](#1230)

### Refactoring

* remove now unneeded types definition src/inclusion.d.ts ([f10cd95](f10cd95))
@standard-cd-bot
Copy link

🎉 This issue has been resolved in version 38.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@meyfa
Copy link

meyfa commented Aug 27, 2023

This was a breaking change, unfortunately. Previously, the following worked:

extends: standard
overrides:
  - files: ['*.ts', '*.tsx']
    extends: 'standard-with-typescript'
parserOptions:
  project: './tsconfig.lint.json'

but now we have to write:

extends: standard
overrides:
  - files: ['*.ts', '*.tsx']
    extends: 'standard-with-typescript'
    parserOptions:
      project: './tsconfig.lint.json'

Additionally, it causes major effort for us. We're including eslint-config-standard-with-typescript in an org-wide custom config, which is then added to dozens of downstream projects. Now all of these downstream projects have to specify the overrides section as well, with the exact same files list as our custom config, for them to be able to set the project.

i.e. what worked before:

extends: '@my-org/eslint-config'
parserOptions:
  project: './tsconfig.lint.json'

now has to be (in many repos):

extends: '@my-org/eslint-config'
overrides:
  - files: ['*.ts', '*.tsx']
    parserOptions:
      project: './tsconfig.lint.json'

Otherwise, we get hit with the following error:

C:\Users\meyfa\project-name\test\index.test.ts
  0:0  error  Parsing error: ESLint was configured to run on `<tsconfigRootDir>/test\index.test.ts` using `parserOptions.project`: <tsconfigRootDir>/tsconfig.json
However, that TSConfig does not include this file. Either:
- Change ESLint's list of included files to not include this file
- Change that TSConfig to include this file
- Create a new TSConfig that includes this file and include it in your parserOptions.project
See the typescript-eslint docs for more info: https://typescript-eslint.io/linting/troubleshooting#i-get-errors-telling-me-eslint-was-configured-to-run--however-that-tsconfig-does-not--none-of-those-tsconfigs-include-this-file

✖ 1 problem (1 error, 0 warnings)

@rhettjay
Copy link
Contributor

rhettjay commented Sep 2, 2023

@meyfa

  1. We've looked into this. We're not entirely sure that this is a breaking change, we are sorry if it is. At this point debating the reality of a breaking change is somewhat irrelevant because the release has been made.

  2. This usage:

    extends: standard
    overrides:
      - files: ['*.ts', '*.tsx']
        extends: 'standard-with-typescript'
    parserOptions:
      project: './tsconfig.lint.json'

    doesn't seem quite right because the parserOptions.project property applies only to the typescript-eslint parser, which the standard config doesn't use.

  3. Consider finding a way to use your tsconfig.json for eslint. You may not be able to.

  4. If you're using the same tsconfig.lint.json path in all downstream projects consider setting it in your shared eslint configuration.

  5. Another breaking change is planned. That is the migration to the eslint flat configuration format. Consider deferring your upgrade until that is released to minimize workload.

@meyfa
Copy link

meyfa commented Sep 2, 2023

@jay-bulk Thanks very much for your detailed response!

We've looked into this. We're not entirely sure that this is a breaking change, we are sorry if it is. At this point debating the reality of a breaking change is somewhat irrelevant because the release has been made.

I agree, and there was also a major release immediately afterwards. But yeah, I would consider it a breaking change.

This usage: [...] doesn't seem quite right because the parserOptions.project property applies only to the typescript-eslint parser, which the standard config doesn't use.

This is a usage that worked before and now it doesn't - hence my conclusion about it being a breaking change. Obviously, I wouldn't write it this way when starting a new project :) Our actual production usage is described further down in the original comment, and there you can see the parserOptions block at root level without any overrides section. This was the way to do it before v36.0.0 which removed the overrides from eslint-config-standard-with-typescript, forcing us to add it to our config. Luckily, this didn't mandate any downstream changes since parserOptions was able to be inherited - a change to the base config was enough. After the addition of parserOptions.project = true, though, that's not the case any more. One could say that with this change, v36 takes full effect also for downstream packages, while it was a simple base config change initially.

Consider finding a way to use your tsconfig.json for eslint. You may not be able to.

I considered that multiple times; unfortunately, this results in wrong compilation output since we also have tests and tooling configs in TypeScript. Those need to be linted but not compiled. From what I could gather, having a secondary config for linting like this is not uncommon.

Another breaking change is planned. That is the migration to the eslint flat configuration format. Consider deferring your upgrade until that is released to minimize workload.

That's a good recommendation, thanks. I'm actually quite looking forward to the flat config, as it will hopefully allow us to expose a factory function from our base config package, such that changes like this can be fully abstracted away. We'll hold off on updating until the migration 👍

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

Successfully merging a pull request may close this issue.

3 participants