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(eslint-plugin): create prefer-inject-decorator #2688

Merged
merged 2 commits into from
Sep 14, 2022
Merged

feat(eslint-plugin): create prefer-inject-decorator #2688

merged 2 commits into from
Sep 14, 2022

Conversation

ddubrava
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows Conventional Commits
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Refactoring
  • Code style update
  • Build or CI related changes
  • Documentation content changes

What is the current behavior?

Closes #2662

What is the new behavior?

ESLint forces to use Inject() for Dependency Injection.

Does this PR introduce a breaking change?

  • Yes
  • No

@ddubrava
Copy link
Contributor Author

ddubrava commented Sep 13, 2022

Hey! I tried not to hardcode a lot but in some cases we can't escape this.

Anyway, there are some questions:

  1. Can we use typescript? I looked at some articles, and I don't think it's too complicated to implement it. The problem is that .? stuff comes not from typings but from real experience and AST explorer. It's unreliable. I believe typings over AST should cover such cases.
  2. Can we implement testing? It's out of the box in TypeScript ESLint. I think this rule is a little bit complicated and testing can help us.
  3. There is TypeScript ESLint Utils package, can we install it? I need an AST_NODE_TYPES enum not to hardcode types.

@nsbarsukov
Copy link
Member

@ddubrava thanks for your contribution! 🚀
Let's discuss your questions.


Can we use typescript?

I really want to use Typescript too.
But this new eslint-plugin is not only for publication but also is used for local usage.
Is there a convenient way to use this plugin locally without compiling Typescript to Javascript (and always keeping dist-folder) ?
I don't know this way that is why I use JS. May be you know a good practice for mentioned problem.


Can we implement testing?

Of course, we love testing ! And we will really appreciate if you add them!


There are TypeScript ESLint Utils, can we install it?

Yep, you can install it.
@splincode could you help with advice ?
Should it be as peer-dependency of package @taiga-ui/eslint-plugin ?

@splincode
Copy link
Member

Can we use typescript?

We need use tsc in later

There are [TypeScript ESLint Utils](https://www.npmjs.com/package/@typescript-eslint/utils), can we install it?

Yep, you can install it.
@splincode could you help with advice ?

As dependency

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #2688 (4a4b2b8) into main (7dfc227) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2688   +/-   ##
=======================================
  Coverage   56.68%   56.68%           
=======================================
  Files         955      955           
  Lines        9233     9233           
  Branches     1894     1894           
=======================================
  Hits         5234     5234           
  Misses       3588     3588           
  Partials      411      411           
Flag Coverage Δ
addon-charts 73.64% <ø> (ø)
addon-doc 19.12% <ø> (ø)
addon-editor 28.82% <ø> (ø)
addon-mobile ∅ <ø> (∅)
addon-table 83.78% <ø> (ø)
addon-tablebars ∅ <ø> (∅)
cdk 62.87% <ø> (ø)
core 62.86% <ø> (ø)
kit 61.54% <ø> (ø)
summary 56.68% <ø> (ø)

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

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@splincode
Copy link
Member

@ddubrava --fix
image

@nsbarsukov nsbarsukov merged commit 3105204 into taiga-family:main Sep 14, 2022
@well-done-bot
Copy link

well-done-bot bot commented Sep 14, 2022

'Well done'

@tinkoff-bot tinkoff-bot mentioned this pull request Sep 16, 2022
evantrimboli pushed a commit to evantrimboli/taiga-ui that referenced this pull request Sep 20, 2022
…#2688)

* feat(eslint-plugin): create prefer-inject-decorator

* fix(): fix prefer-inject-decorator errors
@ddubrava ddubrava deleted the prefer-inject-decorator branch October 16, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

🤖 - ESLint rule prefer-inject-decorator
3 participants