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

feat(linter): add parserOptions.emitDecoratorMetadata and parserOptions.experimentalDecorators for config file #3645

Conversation

mysteryven
Copy link
Contributor

@mysteryven mysteryven commented Jun 12, 2024

These two options are needed in consistent-type-import.

At first, I think parserOptions is not useful for us, and want to hard-code experimentalDecorators and emitDecoratorMetadata to false directly. But I changed the thought after reading:

  1. https://typescript-eslint.io/blog/changes-to-consistent-type-imports-with-decorators
  2. feat(eslint-plugin): [consistent-type-imports] ignore files with decorators, experimentalDecorators, and emitDecoratorMetadata typescript-eslint/typescript-eslint#8335

We don't have type-aware linting for now, and require these two options to decide how we perform: if we have both experimentalDecorators: true and emitDecoratorMetadata: true, then it will not report any errors within any files that contain decorators.

https://github.com/typescript-eslint/typescript-eslint/blob/e408b93e48e5199d83a8d99d1e27126d2dd8bc8f/packages/eslint-plugin/src/rules/consistent-type-imports.ts#L303-L336

@github-actions github-actions bot added the A-linter Area - Linter label Jun 12, 2024
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mysteryven and the rest of your teammates on Graphite Graphite

Copy link

graphite-app bot commented Jun 12, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link

codspeed-hq bot commented Jun 12, 2024

CodSpeed Performance Report

Merging #3645 will not alter performance

Comparing 06-11-feat_linter_add_parserOptions.emitDecoratorMetadata_and_parserOptions.experimentalDecorators_for_config_file (fd30f6f) with main (95e0571)

Summary

✅ 22 untouched benchmarks

…tions.experimentalDecorators` for config file
@mysteryven mysteryven force-pushed the 06-11-feat_linter_add_parserOptions.emitDecoratorMetadata_and_parserOptions.experimentalDecorators_for_config_file branch from 12f1ad9 to fd30f6f Compare June 12, 2024 14:13
@Boshen Boshen self-assigned this Jun 12, 2024
Boshen pushed a commit that referenced this pull request Jun 30, 2024
…rts` (#3895)

This PR only contains the part about report error, adding the fixer part will make the whole PR difficult to review at one time.

There are also some commented cases. One kind of them is `decorator`, as it blocked by #3645, another kind of them is type reference, need to solve #3799 first. I added TODO flags for them.
@DonIsaac
Copy link
Contributor

@mysteryven it looks like we've gotten consistent-type-imports shipped, do we still need this PR?

@mysteryven
Copy link
Contributor Author

mysteryven commented Jul 31, 2024

it looks like we've gotten consistent-type-imports shipped, do we still need this PR?

Unfortunately, this issue still exists: https://typescript-eslint.io/blog/changes-to-consistent-type-imports-with-decorators/#consistent-type-imports-caused-runtime-breakage, there are some cases are commented out temporarily.

Add parserOptions.* only for this case is not really worthy IMO, it's looks strange. It's OK for me to close this.

Maybe the FrameworkFlags and read something in tsconfig.json is better and more ergonomic?

@mysteryven mysteryven closed this Jul 31, 2024
@mysteryven
Copy link
Contributor Author

Need a better way.

@Boshen Boshen deleted the 06-11-feat_linter_add_parserOptions.emitDecoratorMetadata_and_parserOptions.experimentalDecorators_for_config_file branch September 11, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants