ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.01k stars 1.97k forks source link

eslint plugin should use correct version of 'concatLatestFrom' in autofix for rule 'prefer-concat-latest-from' #4299

Open robinsierra-ergon opened 5 months ago

robinsierra-ergon commented 5 months ago

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

eslint-plugin

Minimal reproduction of the bug/regression with instructions

The rule 'prefer-concat-latest-from' has an autofix that fixes

withLatestFrom(store.select(...))

to

concatLatestFrom(() => store.select(...))

however, it uses the deprecated import form @ngrx/effects instead of @ngrx/operators.

Expected behavior

The function from @ngrx/operators should be used.

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

NgRx 17.2.0

Other information

No response

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

tom9744 commented 5 months ago

Hi, I'm interested in fixing this issue. Could I work on it?

brandonroberts commented 5 months ago

@tom9744 Sure

timdeschryver commented 5 months ago

Just for a sanity check, updating the import reference from effects to operators won't break a project if it uses the fix feature, right? Because operators is a dependency from effects we can safely update the import statement.

timdeschryver commented 5 months ago

Just for a sanity check, updating the import reference from effects to operators won't break a project if it uses the fix feature, right? Because operators is a dependency from effects we can safely update the import statement.

This seems to work just fine 👍

tom9744 commented 5 months ago

When I look at the prefer-concat-latest-from.ts file, it seems like the rule still belongs to the @ngrx/effect module. Even though the concat-latest-from operator has been moved to the @ngrx/operators module.

export default createRule<Options, MessageIds>({
  name: path.parse(__filename).name,
  meta: {
    type: 'problem',
    ngrxModule: 'effects',
    version: '>=12.0.0',
    ...

So, I wonder if I should append the operators: '@ngrx/operators' property to the NGRX_MODULE_PATHS object which is declared in the ngrx-modules.ts file?

export const NGRX_MODULE_PATHS = {
  ['component-store']: '@ngrx/component-store',
  effects: '@ngrx/effects',
  operators: '@ngrx/operators', // 👈 Here!
  store: '@ngrx/store',
} as const;
timdeschryver commented 4 months ago

This lost my attention, sorry @tom9744 . If you still want feel free to work on this. I agree that this can be added to @ngrx/operators, if you need help setting it up feel free to ping me.