nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.83k stars 2.38k forks source link

Incorrect types for the fetch operator and the combination of operators if the fetch operator has an id. #8782

Closed Fafnur closed 2 years ago

Fafnur commented 2 years ago

Current Behavior

Since version 13.4.4, operator fetch together with operator concatLatestFrom (withLatestFrom) is not working correctly, provided that the fetch operator has the id property, but the id has no parameters.

Expected Behavior

Id may not contain all parameters

Steps to Reproduce

Sample

import {Injectable} from '@angular/core';
import {createEffect, Actions, ofType, concatLatestFrom} from '@ngrx/effects';
import {fetch} from '@nrwl/angular';
import {Store} from "@ngrx/store";

import {getPostError} from "@loopy/posts/state";

import * as PostActions from './post.actions';

@Injectable()
export class PostEffects {
  init$ = createEffect(() =>
    this.actions$.pipe(
      ofType(PostActions.init),
      concatLatestFrom(() => this.store.select(getPostError)),
      fetch({
        id: () => 'init',
        run: (action, error) => {
          // Your custom service 'load' logic goes here. For now just return a success action...
          return PostActions.loadPostSuccess({post: []});
        },
        onError: (action, error) => {
          console.error('Error', error);
          return PostActions.loadPostFailure({error});
        },
      })
    )
  );

  constructor(private readonly actions$: Actions, private readonly store: Store) {
  }
}

Screenshots

1 2

technbuzz commented 2 years ago

I am experiencing he same problem when no combineLatestFrom or id used with fetch

image

image

image

s0l4r commented 2 years ago

I have the same issue after generating a new effect using NX npx nx generate @nrwl/angular:ngrx --name=creators --module=lib/creator/feature/list/creators.module.ts --directory=state --facade

import { Injectable } from '@angular/core';
import { createEffect, Actions, ofType } from '@ngrx/effects';
import { fetch } from '@nrwl/angular';

import * as CreatorsActions from './creators.actions';
import * as CreatorsFeature from './creators.reducer';

@Injectable()
export class CreatorsEffects {
  init$ = createEffect(() =>
    this.actions$.pipe(
      ofType(CreatorsActions.init),
      fetch({
        run: (action) => {
          // Your custom service 'load' logic goes here. For now just return a success action...
          return CreatorsActions.loadCreatorsSuccess({ creators: [] });
        },
        onError: (action, error) => {
          console.error('Error', error);
          return CreatorsActions.loadCreatorsFailure({ error });
        },
      })
    )
  );

  constructor(private readonly actions$: Actions) {}
}

Versions:

nx report

 >  NX   Report complete - copy this into the issue template

   Node : 16.13.0
   OS   : darwin x64
   npm  : 8.1.0

   nx : 13.8.3
   @nrwl/angular : 13.8.3
   @nrwl/cli : 13.8.3
   @nrwl/cypress : 13.8.3
   @nrwl/detox : undefined
   @nrwl/devkit : 13.8.3
   @nrwl/eslint-plugin-nx : 13.8.3
   @nrwl/express : undefined
   @nrwl/jest : 13.8.3
   @nrwl/js : 13.8.3
   @nrwl/linter : 13.8.3
   @nrwl/nest : undefined
   @nrwl/next : undefined
   @nrwl/node : undefined
   @nrwl/nx-cloud : undefined
   @nrwl/react : undefined
   @nrwl/react-native : undefined
   @nrwl/schematics : undefined
   @nrwl/storybook : 13.8.3
   @nrwl/tao : 13.8.3
   @nrwl/web : 13.8.3
   @nrwl/workspace : 13.8.3
   typescript : 4.5.5
   rxjs : 7.4.0
   ---------------------------------------
   Community plugins:
     @angular/animations: 13.2.3
     @angular/common: 13.2.3
     @angular/compiler: 13.2.3
     @angular/core: 13.2.3
     @angular/forms: 13.2.3
     @angular/platform-browser: 13.2.3
     @angular/platform-browser-dynamic: 13.2.3
     @angular/router: 13.2.3
     @ngrx/component-store: 13.0.2
     @ngrx/effects: 13.0.2
     @ngrx/entity: 13.0.2
     @ngrx/router-store: 13.0.2
     @ngrx/store: 13.0.2
     @ngrx/store-devtools: 13.0.2
     @angular-devkit/build-angular: 13.2.4
     @angular/cli: 13.2.4
     @angular/compiler-cli: 13.2.3
     @angular/language-service: 13.2.3
     @ngrx/schematics: 13.0.2
     @ngrx/store-devtools: 13.0.2

Error:

Type 'Observable<unknown>' is not assignable to type 'EffectResult<Action>'. Type 'Observable<unknown>' is not assignable to type 'Observable<Action>'. Type 'unknown' is not assignable to type 'Action'.ts(2322) [effect_creator.d.ts(42, 161): ]()The expected type comes from the return type of this signature. Argument of type '(source: ActionStateStream<unknown, TypedAction<"[Creators Page] Init">>) => Observable<Action>' is not assignable to parameter of type 'OperatorFunction<ActionOrActionWithState<unknown, TypedAction<"[Creators Page] Init">>, unknown>'. Types of parameters 'source' and 'source' are incompatible. Type 'Observable<ActionOrActionWithState<unknown, TypedAction<"[Creators Page] Init">>>' is missing the following properties from type 'Observable<ActionOrActionWithState<unknown, TypedAction<"[Creators Page] Init">>>': _isScalar, _trySubscribe, _subscribets(2345)

s0l4r commented 2 years ago

In my case I had to add the type manually, and I also switched to dataPersistence:

import { Observable } from 'rxjs';
import { Action } from '@ngrx/store';

...
  init$ = createEffect((): Observable<Action> =>
    this.dataPersistence.fetch(CreatorsActions.init, {
      run: (
        action: ReturnType<typeof CreatorsActions.init>,
        state: CreatorsFeature.CreatorsPartialState
      ) => {
        // Your custom service 'load' logic goes here. For now just return a success action...
        return CreatorsActions.loadCreatorsSuccess({ creators: [] });
      },
      onError: (action: ReturnType<typeof CreatorsActions.init>, error) => {
        console.error('Error', error);
        return CreatorsActions.loadCreatorsFailure({ error });
      },
    })
  );

When I first tried it, I got another error which I solved by downgrading rxjs from rxjs@7.4.0 to rxjs@6.6.7:


'import(“./node_modules/@nrwl/angular/node_modules/rxjs/internal/Observable").Observable<any>' is not assignable to type 'import("./node_modules/rxjs/dist/types/internal/Observable").Observable<any>'.

The types of 'operator.call' are incompatible between these types.
    Type '(subscriber: import("./node_modules/@nrwl/angular/node_modules/rxjs/internal/Subscriber").Subscriber<any>, source: any) => import("./node_modules/@nrwl/angular/node_modules/rxjs/internal/types").TeardownLogic' is not assignable to type '(subscriber: import("./node_modules/rxjs/dist/types/internal/Subscriber").Subscriber<any>, source: any) => import("./node_modules/rxjs/dist/types/internal/types").TeardownLogic'.
      Types of parameters 'subscriber' and 'subscriber' are incompatible.
        Type 'Subscriber<any>' is missing the following properties from type 'Subscriber<any>': syncErrorValue, syncErrorThrown, syncErrorThrowable, _unsubscribeAndRecycle, and 2 more.ts(2322)```
Coly010 commented 2 years ago

Hey! :)

Thanks for reporting this.

I'm having some difficulty reproducing this issue, could any of you provide a minimal reproduction or a link to a repo I could clone that contains the error? That would be very helpful!!

Fafnur commented 2 years ago

@Coly010 repo: https://github.com/Fafnur/nx-issues-8782

You can see a problem (example and hack) here: https://github.com/Fafnur/nx-issues-8782/blob/main/libs/feature/state/src/lib/%2Bstate/feature.effects.ts

Fafnur commented 2 years ago

@Coly010 You can try build.

If you run the build, you should get this:

> nx run demo:build:production

✔ Browser application bundle generation complete.

Error: libs/feature/state/src/lib/+state/feature.effects.ts:34:7 - error TS2345: Argument of type 'OperatorFunction<TypedAction<"[Feature Page] Init">, [TypedAction<"[Feature Page] Init">, Dictionary<FeatureEntity>]>' is not assignable to parameter of type 'OperatorFunction<TypedAction<"[Feature Page] Init">, ActionOrActionWithStates<[], TypedAction<"[Feature Page] Init">>>'.
  Type '[TypedAction<"[Feature Page] Init">, Dictionary<FeatureEntity>]' is not assignable to type 'ActionOrActionWithStates<[], TypedAction<"[Feature Page] Init">>'.
    Type '[TypedAction<"[Feature Page] Init">, Dictionary<FeatureEntity>]' is not assignable to type '[TypedAction<"[Feature Page] Init">]'.
      Source has 2 element(s) but target allows only 1.

34       concatLatestFrom(() => this.store.select(getFeatureEntities)),
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Error: libs/feature/state/src/lib/+state/feature.effects.ts:37:9 - error TS2322: Type '(action: TypedAction<"[Feature Page] Init">, features: [...T][0]) => { feature: FeatureEntity[]; } & TypedAction<"[Feature/API] Load Feature Success">' is not assignable to type '(a: TypedAction<"[Feature Page] Init">) => void | Action | Observable<Action>'.

37         run: (action, features) => {
           ~~~

  node_modules/@nrwl/angular/src/runtime/nx/data-persistence.d.ts:26:5
    26     run(a: A, ...slices: [...T]): Observable<Action> | Action | void;
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    The expected type comes from property 'run' which is declared here on type 'FetchOpts<[], TypedAction<"[Feature Page] Init">>'

 >  NX   Ran target build for project demo (3s)

    ✖    1/1 failed
    ✔    0/1 succeeded [0 read from cache]
Coly010 commented 2 years ago

Thank you for the repo! It's been super helpful!!

Coly010 commented 2 years ago

The correct solution to this issue will involve a large breaking change to the API exposed by those operators.

However, there is a workaround right now.

In your id and run declarations, you always need to define the correct number of parameters to match the action and correct number of slices of state you're using.

Examples

No slices of state:

this.actions$.pipe(
      ofType(FeatureActions.init),
      fetch({
        id: () => 'my-id',
        run: (action) => {
          return FeatureActions.loadFeatureSuccess({ feature: [] });
        },
        onError: (action, error) => {
          console.error('Error', error);
          return FeatureActions.loadFeatureFailure({ error });
        },
      })
    )

1 Slice of State

this.actions$.pipe(
      ofType(FeatureActions.init),
      withLatestFrom(
        this.store.select(getFeatureEntities)
      ),
      fetch({
        id: (action, features) => 'my-id',
        run: (action, features) => {
          return FeatureActions.loadFeatureSuccess({ feature: [] });
        },
        onError: (action, error) => {
          console.error('Error', error);
          return FeatureActions.loadFeatureFailure({ error });
        },
      })
    )

For more than 1 slice of state

this.actions$.pipe(
      ofType(FeatureActions.init),
      withLatestFrom(
        this.store.select(getFeatureEntities),
        this.store.select(getSelected)
      ),
      fetch({
        id: (action, features, selcted) => 'my-id',
        run: (action, features, selected) => {
          return FeatureActions.loadFeatureSuccess({ feature: [] });
        },
        onError: (action, error) => {
          console.error('Error', error);
          return FeatureActions.loadFeatureFailure({ error });
        },
      })
    )
technbuzz commented 2 years ago

I am not well versed with newer function like fetch, or persistence but I created feature store with NX and it creates basic files some boiler plate code. Can somebody provide a full working repo or stackblitz example based on that addressing this problem?.

Fafnur commented 2 years ago

@technbuzz You can see here: https://github.com/Fafnur/nx-issues-8782/blob/main/libs/feature/state/src/lib/%2Bstate/feature.effects.ts

The main idea of solution it's always duplicate parameters in fetch:

For example:

withLatestFrom(observable$),
fetch({
  id: (action, value) => ...,
  run: (action, value) => ...,
})

...

withLatestFrom(observable$, observable2$),
fetch({
  id: (action, value, secondValue) => ...,
  run: (action, value, secondValue) => ...,
})

...

withLatestFrom(observable$, observable2$, observable3$),
fetch({
  id: (action, value, secondValue, thirdValue) => ...,
  run: (action, value, secondValue, thirdValue) => ...,
})
alexey-chuprina commented 2 years ago

After migrating to v14 another error with the fetch. Hi, @Fafnur, maybe you know a solution? PS: try to migrate your repo "banshop" to v14 and you will see errors. :(

Fafnur commented 2 years ago

@alexey-chuprina I have updated two projects - banx and banshop. Everything works as before.

Try to remove node_modules and lock files(package-lock.json, yarn.lock) and install packages.

Coly010 commented 2 years ago

This issue results from a TS constraint which results from the API of those operators.

In the future, we may revisit the API to allow more flexibility. Still, for now, this isn't an issue as it's simply a constraint coming from TypeScript itself with how it resolves the Generic Types and spreads them, forcing the signature of both functions in the operator to match.

Workarounds and examples listed in this issue are great resources for using this operator correctly.

As such, I'm going to close this issue. Thanks, everyone for contributing to it!

stijnvn commented 2 years ago

The workaround with the parameters does not seems to work (nx 14.4.2, rxjs 7.5.6).

This is a blocker for us to upgrade to nx 14. Any idea how we can solve this?

ZNick1982 commented 2 years ago

Try to make the version of rxjs in your package.js matches with the version of rxjs in the @nrwl/angular library: https://github.com/nrwl/nx/blob/master/package.json#L220, once I've made it the same version it's started to work for me

jonesjBSV commented 2 years ago

Using rxjs fixed it for me:

import { Injectable } from '@angular/core';
import { createEffect, Actions, ofType, concatLatestFrom } from '@ngrx/effects';
import { Store } from '@ngrx/store';

import * as TodosActions from './todos.actions';
import * as TodosFeature from './todos.reducer';
import { exhaustMap, of } from 'rxjs';

@Injectable()
export class TodosEffects {

  init$ = createEffect((): any => {
    return this.actions$.pipe(
      ofType(TodosActions.initTodos),
      exhaustMap( (action) => {
          // Your custom service 'load' logic goes here. For now just return a success action...
          return of(TodosActions.loadTodosSuccess({ todos: [] }));
        }),
        exhaustMap((action, error) => {
          console.error('Error', error);
          return of(TodosActions.loadTodosFailure({ error }));
        })
    );
  });

  constructor(private readonly actions$: Actions, private readonly store: Store) {}
}
SirBr4him commented 2 years ago

I've resolved this issue by ovveriding the rxjs version in package.json like this:

"overrides": { "rxjs": "7.4.0" }

and it have to be the same verssion in stalled in your dependecies.

github-actions[bot] commented 1 year ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.