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: add ability to show rule for JSX usage #15

Merged
merged 5 commits into from
Dec 7, 2020
Merged

Conversation

kirkobyte
Copy link

Adds the ability to show a rule violation when deprecated methods are used in JSX.

I also had to extract the Identifier method into a method so it could also be passed to JSXIdentifier

fix #8

Adds the ability to show a rule violation when deprecated methods are used in JSX

fix gund#8
src/rules/deprecation.ts Outdated Show resolved Hide resolved
@kirkobyte
Copy link
Author

Hey @gund, any thoughts on how you'd approach the code complexity problem Code Climate picked up on in this PR?

Copy link
Owner

@gund gund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks a lot for your contribution!

I just have a few questions before we can merge this.

src/rules/deprecation.ts Show resolved Hide resolved
src/rules/deprecation.ts Outdated Show resolved Hide resolved
src/rules/deprecation.ts Show resolved Hide resolved
src/rules/deprecation.ts Outdated Show resolved Hide resolved
src/rules/deprecation.ts Outdated Show resolved Hide resolved
src/rules/deprecation.ts Outdated Show resolved Hide resolved
src/rules/deprecation.ts Outdated Show resolved Hide resolved
@gund
Copy link
Owner

gund commented Nov 20, 2020

Also it's important to add test cases for the JSX functionality to verify they are working correctly.
Will you be able to do it or do you need help with tests?

moves the internal rule creator method later and gives it a better name
@kirkobyte
Copy link
Author

kirkobyte commented Nov 20, 2020

I've moved that method and it looks better :)
I tried adding tests. But unfortunately I've had problems getting Jest to load the JSX syntax correctly

@kirkobyte
Copy link
Author

Changing the filename option in getValidTestCase to fixtures/file.tsx gives the following error:

A fatal parsing error occurred: Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: fixtures/file.tsx.
The file must be included in at least one of the projects provided.

I'd have thought adding "jsx": "prserve" would allow the tests to load .tsx files, but that unfortunately doesn't seem to be working. Any thoughts?

@kirkobyte
Copy link
Author

kirkobyte commented Nov 21, 2020

I've pushed a commit with my attempt at adding JSX support to the tests. Let me know if you have any thoughts on how to fix them :) (I'm very unfamiliar with ESLint rule development and testing)

@gund
Copy link
Owner

gund commented Nov 22, 2020

Unfortunately I've also hit the same error with JSX tests and have no idea how to fix them...

Maybe we could ping somebody from the ESLint team to give us a hint here.

@kirkobyte
Copy link
Author

I've tried upgrading the @typescript-eslint/* and typescript dependencies also, which hasn't done anything.
Reaching out to someone from the ESLint team sounds good. Is there someone from the team you usually communicate with?

@caspardue
Copy link

There is a dummy file called file.ts under fixtures. I tried adding another one called file.tsx and the test turned green.

@kirkobyte
Copy link
Author

Good find! Thanks! 🙏

adds jsx test capability with base component usage tests
@codeclimate
Copy link

codeclimate bot commented Dec 7, 2020

Code Climate has analyzed commit 0704bd3 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

View more on Code Climate.

Copy link
Owner

@gund gund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM

@gund gund merged commit 359b88a into gund:master Dec 7, 2020
@gund
Copy link
Owner

gund commented Dec 7, 2020

🎉 This PR is included in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gund gund added the released label Dec 7, 2020
rivy added a commit to rivy/js.os-paths that referenced this pull request Dec 24, 2020
…xample file

```text
Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: eg\show-paths.esm.mjs.
The extension for the file (.mjs) is non-standard. You should add "parserOptions.extraFileExtensions" to your config.
```

- may have happened on earlier commits (look...)
- ref: <https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/FAQ.md#i-use-a-framework-like-vue-that-requires-custom-file-extensions-and-i-get-errors-like-you-should-add-parseroptionsextrafileextensions-to-your-config>
- ref: <typescript-eslint/typescript-eslint#2850>
- ref: <gund/eslint-plugin-deprecation#15>
rivy added a commit to rivy/js.os-paths that referenced this pull request Dec 26, 2020
…xample file

```text
Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: eg\show-paths.esm.mjs.
The extension for the file (.mjs) is non-standard. You should add "parserOptions.extraFileExtensions" to your config.
```

- may have happened on earlier commits (look...)
- ref: <https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/FAQ.md#i-use-a-framework-like-vue-that-requires-custom-file-extensions-and-i-get-errors-like-you-should-add-parseroptionsextrafileextensions-to-your-config>
- ref: <typescript-eslint/typescript-eslint#2850>
- ref: <gund/eslint-plugin-deprecation#15>
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 this pull request may close these issues.

Feature request: fail on JSX usage of deprecated components
3 participants