☁️ Nx Cloud Report

CI ran the following commands for commit c0b4b29. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Sent with 💌 from NxCloud.

Thanks for the PR, @lonyele!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

✔️ Deploy Preview for typescript-eslint ready!

🔨 Explore the source changes: c0b4b29

🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/621e226a702a2900080846e1

😎 Browse the preview: https://deploy-preview-4582--typescript-eslint.netlify.app

Codecov Report

Merging #4582 (328c4a1) into main (fabfc2b) will increase coverage by 1.99%.
The diff coverage is 100.00%.

❗ Current head 328c4a1 differs from pull request most recent head c0b4b29. Consider uploading reports for the commit c0b4b29 to get more accurate results

@@            Coverage Diff             @@
##             main    #4582      +/-   ##
==========================================
+ Coverage   92.41%   94.41%   +1.99%     
==========================================
  Files         350      151     -199     
  Lines       12059     8229    -3830     
  Branches     3430     2623     -807     
==========================================
- Hits        11144     7769    -3375     
+ Misses        642      262     -380     
+ Partials      273      198      -75     

Flags with carried forward coverage won't be shown. Click here to find out more.

hm... it took some time after the comment because I was trying to fix the playground but failed...

Oh man, this got really noisy in our projects. We work with a lot of external APIs and data sources and some of them use all kind of characters like #, @, even spaces

So, we basically quote those properties so that we bypass naming conventions but now that stopped working. Can we add a an option to ignore quoted properties? There's a reason they are quoted to begin with ):

Oh god... I hear you. If maintainers agree with it. I'll make a PR that makes this an option

That would be awesome

@lonyele @alfaproject this becomes especially annoying with selectors when you use some css-in-js:

I don't want to add eslint-disable rule before selectors, nor do i want to specify some complex filter regex :(

Yeah that's another good use case to disable quoted properties

No need for an option - the rule is already designed to support this and it's already documented in the rule docs:
https://typescript-eslint.io/rules/naming-convention/#ignore-properties-that-require-quotes

Oh I was using filter to work around it. Didn't realise there was that modifier. Sorry for the noise, I'll give it a try

Edit: Actually, not quite sure how that works. Is this wrong? https://typescript-eslint.io/play/#ts=4.5.2&sourceType=module&code=PQKgUApgzgNglgOwC5gATtQIgAJIJ4AO0AxgE5wFIC008ywCAhgLaIDmVxA9ggG4TI4PTAC5UAbTQZp6TBFKkupTABopMjAG91GjJgBmcGEnmjUmrM0ZJiACzP7GMKBBVZSENhAAeZzOKoRADoAYmwAHXDxSIBdGMxUAF81XRkDJSskM3FMYhYIGABhRhdMGJTUvRcYCGIkJT8AEwhHAFdjTB0NZK6ZbUq9fQzrMwR2mAqBzGYuRrhDeShszA8AR1a4DygARVauEyXy3o1Matr65TEcrgAjACtzgBk4E1InAAVFIlJ8MuOMHoDfoDLCGYymMQWabWOwOJwuNwrTw+PwBYJhSLRcJxBKAkHpUiZZZ5ZgFYqlRHvEp5IolCCqLBQJgAawgAH08hSsABVd7vACiACU2YUAIIAZX5ZUmlUwNUYcwQbG5CGapCg3A8ficMC4AHdxewagB5UgAES4rRuNVU-2kpwK5waV0wtwedWerw+X3kv0R+CIny431+MTtSRl0jDIGAYDA3AQUCQqCGXFQAF5zDoAOQARmzYmzN0YpGzakSYCAA&rules=N4XyA&tsConfig=N4XyA

(the filters are my work-around for other quoted properties but I was hoping to get rid of them)

A filter will make the config higher precedence than others.
Hence it is matching before the requiresQuotes config.

I think this should have been released to a major version, it currently broke our pipelines linting stages and we had no idea what caused this.
Anyone having a version lower than ^5.14.0 might suddenly get errors.

major version

Thus we see the great dislike from language, linter, and similar teams for the illusion of semantic versioning. Pretty much any logical change to a lint rule could be seen as a "breaking" change. Covering cases that previously weren't checked -both in the case of a bugfix and a lacking feature- breaks users who upgrade. If we released a major version every time one of these changes was released and wanted users to receive those updates at any reasonable cadence, we'd have to release a new major version every month and still be shipping dozens of breaking changes per version.

broke our pipelines linting stages

General good practice & our recommendation is to lock your dependencies to a fixed version, such as with a fixed range in a package.json and/or a lockfile. Use a tool such as dependabot or renovate to send PRs for specific package updates on a cadence.

Happy to move this discussion to a new issue if you think I'm wrong about either of those things and we should reevaluate our release strategy!

Definitely agree on the lock file - you should be using the same versions in dev as you do on your CI. If the two are running different versions then you're in for a bad time!

And strong agree on the versioning. Fixing a bug which uncovers cases that previously weren't caught is not what we define as a breaking change. For us breaking changes are pretty much only for cases where config needs to change.

It's similar to why TS doesn't follow semver - because it's impossible to change a lint rule without changingint results.

As per our contributing guidelines, we prefer to not have discussions on closed PRs because they're not easily searchable.
If you'd like to discuss this further - feel free to open an issue.