Eslint8 support

[cezudas]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Build (npm run build) was run locally and any changes were pushed
• Unit tests (npm test) were run locally and passed

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe): Drop Eslint 7 support, Add Eslint 8 support

What is the current behavior?

ATM the peerDeps clearly states this plugin is not compatible with the latest Eslint 8 and @typescript-eslint/eslint-plugin.

bug: decorator is not properly detected by eslint #60
bug: @stencil/no-unused-watch reporting errors after eslint v8 update #59

What is the new behavior?

This plugin now works with Eslint 8.

Does this introduce a breaking change?

• Yes
• No

This PR drops support for Eslint 7.

Testing

First I updated the peerDeps and devDevs to latest and greatest (Eslint 8, latest @typescript/eslint-plugin) and I ran the unit tests -> all the tests failing, as expected.

Then I find all occurrences of 'ClassProperty ' with 'PropertyDefinition' and ran the unit tests again =>
all tests passed.

Then I finally used npm build followed by npm pack and used the tarball in a sample NX workspace here, the repository has a StencilJS library project and comes with Eslint 8 already configured. I did merely for a sanity check, all seems fine.

[cezudas]

Hi team! I was wanted to kindly ask the maintainers to have a look, if possible. I have raised the MR 15 days ago, I was hoping for some feedback :) Thank you!

[Bombo]

Any updates on this? Getting some annoying linter errors in the IDE because of #60 :)

[cezudas]

Any updates on this? Getting some annoying linter errors in the IDE because of #60 :)

There is not a single thing I can do, unfortunately :(

[Bombo]

Yeah, sorry. Wasn't aiming at you, specifically. Just wanted to bump this one for the powers that be to get notified. ;)

[theo-staizen]

Hi @cezudas, I tried out your branch and while it is working with eslint8, it seems to be causing a bug

Specifically, it seems that it incorrectly marks props/methods decorated with any decorator (@Prop, @Method, @Listen, @Watch, @Event, @Element) as needing to have the private keyword added, which was not required before and actually conflicts with other rules @stencil/props-must-be-public and @stencil/methods-must-be-public

I don't understand what's happening, because if I run the tests in your repo, all the rules are passing and I can see the tests do cover the scenario I described above...

Could this be affected by how typescript compiles?

[raphaelpor]

I think it's time to give a second try on this PR.
I created a branch where I updated all the dev dependencies and it seems to work fine.
I also created a build to test in my project and didn't see any new issues.

Do you guys need help to get this done?

[theo-staizen]

@raphaelpor can you share the branch? the current branch is definitely causing issues for me (see my comment above) so it would be good to try out your branch too

[raphaelpor]

@theo-staizen False alarm. I checked again my changes and I do get the same issues as you.
I will try to continue working on that to understand what's going on and see if I can come up with a solution.

Sorry for that.

[maldago]

Hello,

I was playing around with this and discovered that if I updated as follows:
@types/eslint: "8.4.10!
eslint: "8.31.0"
@typescript-eslint/parser: 5.48.1
@typescript-eslint/eslint-plugin: 5.48.1

that the own-props-must-be-private and props-muse-be-public tests were failing.

The changes I made to fix them are as follows and it seems from a first glance and resolving some of the aforementioned issues.

To view the changes I made : https://github.com/maldago/stencil-eslint

Let me know if I can do anything to help out.

[raphaelpor]

Hello,

I was playing around with this and discovered that if I updated as follows:
@types/eslint: "8.4.10!
eslint: "8.31.0"
@typescript-eslint/parser: 5.48.1
@typescript-eslint/eslint-plugin: 5.48.1

that the own-props-must-be-private and props-muse-be-public tests were failing.

The changes I made to fix them are as follows and it seems from a first glance and resolving some of the aforementioned issues.

To view the changes I made : https://github.com/maldago/stencil-eslint

Let me know if I can do anything to help out.

Great @maldago! I will have a look next week and see if we can use then. Thanks 🙏

[raphaelpor]

@maldago Big thanks for you comment. I checked your change and created a PR (#84) based on that.
I also tested it in a private project and everything seems to work as expected.

I will close this PR here since we can use the new one as way to move forward.

Thank you all for contributing. 🎉