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

ESLint plugin does not work correctly when the inject function is used #3834

Closed
2 tasks
Armenvardanyan95 opened this issue Apr 7, 2023 · 3 comments · Fixed by #3936
Closed
2 tasks

ESLint plugin does not work correctly when the inject function is used #3834

Armenvardanyan95 opened this issue Apr 7, 2023 · 3 comments · Fixed by #3936

Comments

@Armenvardanyan95
Copy link
Contributor

Which @ngrx/* package(s) are the source of the bug?

eslint-plugin

Minimal reproduction of the bug/regression with instructions

For example, we want the linter to catch issues like subscribing to the store directly in components. It works correctly when the store is injected via a constructor, but fails to catch issues when the inject function is used

Minimal reproduction of the bug/regression with instructions

Here the plugin will correctly catch a listing a issue:

@Components({
  // metadata
})
export class MyComponent implements OnInit {
  constructor(private store: Store) {}

  ngOnInit() {
    this.store.select(selectSomething).subscribe(() => {
      // do something
    });
  }
}

But will fail to do so in the following case:

@Components({
  // metadata
})
export class MyComponent implements OnInit {
  private store = inject(Store);

  ngOnInit() {
    this.store.select(selectSomething).subscribe(() => {
      // do something
    });
  }
}

It is possible the issue exists in the case of other plugin rules, but I have managed to find this one to fail 100% of the time.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

NgRx: 15.4.0
Angular: 15.1.0
Node: 18.10.0
Browsers: N/A
Operating systems: Windows 11

Other information

No response

I would be willing to submit a PR to fix this issue

  • Yes
  • No
@timdeschryver
Copy link
Member

Good catch @Armenvardanyan95 .
Do do you want to help with this?

@Armenvardanyan95
Copy link
Contributor Author

Armenvardanyan95 commented Apr 15, 2023

@timdeschryver I honestly want to, but I'm still quite far away from understanding how linting works under the hood. I actually dived into the source code, understood a bit, but that's it thus far. If you don't think this is super critical and wanna wait a week or so, I'm would love to try out, but obviously can't promise I will succeed

@DMezhenskyi
Copy link
Contributor

DMezhenskyi commented May 8, 2023

I have never written eslint rules as well but I am very curious and would like to learn that. So I could try to solve this issue if you guys don't mind :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants