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

concatLatestFrom doesnt work with navigate when selecting more than one selector #6830

Closed
HarryPi opened this issue Aug 24, 2021 · 15 comments · Fixed by #8216
Closed

concatLatestFrom doesnt work with navigate when selecting more than one selector #6830

HarryPi opened this issue Aug 24, 2021 · 15 comments · Fixed by #8216

Comments

@HarryPi
Copy link

HarryPi commented Aug 24, 2021

Current Behavior

When passing 2 selectors as an array to concatLatestFrom there is a type mismatch error

Expected Behavior

Expected this to work as in NgRx repository this works and there is even a test for it to verify it works see https://github.com/ngrx/platform/blob/0bb49409413b784b1cc55c2b59e67ed760041aa5/modules/effects/spec/concat_latest_from.spec.ts#L87

Steps to Reproduce

I have an open issue on stackoverflow with a code example found here https://stackoverflow.com/q/68894745/6603342
But the ghist of it is

 loadAllCases$ = createEffect(() => this._actions$.pipe(
    concatLatestFrom(() => [
      this.store.pipe(select(CaseSelectors.getAllCases)),
      this.store.pipe(select(CaseSelectors.getCasesLoaded))
    ]),
    navigation(this.caseLandingPage, {
      run: (snap, [loaded, cases]) => {
        if (loaded) {
          return CaseActions.loadAllSuccess();
        } else {
          return this._caseService.getAll().pipe(
            map(cases => CaseActions.loadAllSuccess(cases))
          );
        }
      },
      onError: (action, error) => {
        return CaseActions.loadAllFailed(error);
      }
    })
  ));

This will work fine if i use one selector or create a selector dedicated for combining the state i wish to extract, but not when passed as an array.
Note that this will also work just fine if i swap navigate for map, switchmap etc. or zip my two selectors.

Failure Logs

The error this produces is

TS2345: Argument of type 'OperatorFunction<Action, [Action, Calendar[], boolean]>' is not assignable to parameter of type 'OperatorFunction<Action, ActionOrActionWithState<unknown, Action>>'.   Type '[Action, Calendar[], boolean]' is not assignable to type 'ActionOrActionWithState<unknown, Action>'.     Type '[Action, Calendar[], boolean]' is not assignable to type '[Action, unknown]'.       Source has 3 element(s) but target allows only 2.

Environment

"rxjs": "~6.6.0",
"@ngrx/effects": "~12.2.0",
"@ngrx/entity": "~12.2.0",
"@ngrx/router-store": "~12.2.0",
"@ngrx/store": "~12.2.0",
@HarryPi
Copy link
Author

HarryPi commented Aug 24, 2021

After further investigation this appears to be a compile time error but the whole thing works fine if i explicitly tell the ts compiler to ignore the type error using ts-ignore but thats something that should not be intended behaviour

@hadrien-toma
Copy link

hadrien-toma commented Aug 25, 2021

The NgRx repository is an Nx repository so if it works on their side, this means this issue is not linked to Nx.

@HarryPi
Copy link
Author

HarryPi commented Aug 25, 2021

The above example would work fine if i would swap navigation for NgRx map or switchMap etc.. the issue comes when using Nx data persistent type such as navigation. This will cause the application to fail to compile (if there is more than one selector) due to type incompatibility.

@leosvelperez leosvelperez added the scope: angular Issues related to Angular support in Nx label Aug 27, 2021
@Coly010
Copy link
Contributor

Coly010 commented Oct 13, 2021

@HarryPi Hi! Do you have a repo you could share that contains this code or this error? It's difficult to reproduce without being able to investigate your selectors and your this.caseLandingPage.

@github-actions
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Oct 28, 2021
@hadrien-toma
Copy link

I still would be interested in this feature 😬

@github-actions github-actions bot removed the stale label Oct 29, 2021
@HarryPi
Copy link
Author

HarryPi commented Nov 1, 2021

Hi @Coly010, apologies for the very late response.
Unfortunately i do not have time to create a repo for this and as this is enterprise code i cant link you directly to it. However, all that this.caseLandingPage is, is a component that is injected within the effects constructor

 constructor(
    private readonly _actions$: Actions,
    private readonly _caseService: CaseService,
    private readonly _caseFacade: CaseFacade,
    private readonly _router: Router,
    @Inject(CASE_DIALOG_PAGE) private readonly _caseDialogPage: Type<CaseModal>,
    @Inject(CASE_VIEW_PAGE) private readonly _caseViewPage: Type<Component>
  ) {
  }

If i find some free time i will create a repo, but as there are plenty of workarounds and in general more issues with the navigation than expected, i have taken a different aproach.

@github-actions
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Nov 16, 2021
@david-shortman
Copy link
Contributor

Commenting so I remember to look into this

@github-actions github-actions bot removed the stale label Nov 27, 2021
@github-actions
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Dec 11, 2021
@hadrien-toma
Copy link

I still have the issue

@github-actions github-actions bot removed the stale label Dec 12, 2021
@david-shortman
Copy link
Contributor

I was able to reproduce locally. Will open a PR eventually.

@david-shortman
Copy link
Contributor

david-shortman commented Dec 17, 2021

Looks like I get the exact same error with withLatestFrom operator as the concatLatestFrom operator. The DataPersistence operators expect an input stream of ActionOrActionWithState, meaning a stream carrying either an Action, or a tuple with an action and only one other piece of data (named "state") [Action, State].

const testWithLatestFrom$ = createEffect(() => {
  return (of() as Observable<Action>).pipe(
    withLatestFrom((of() as Observable<number>), (of() as Observable<string>)),
/*
  👇 Receive error below
   
  TS2345: Argument of type '(source: ActionStateStream<unknown, Action>) => Observable<Action>' is not assignable to parameter of type 'OperatorFunction<[Action, number, string], Action>'.
   Types of parameters 'source' and 'source' are incompatible.
     Type 'Observable<[Action, number, string]>' is not assignable to type 'ActionStateStream<unknown, Action>'.
       Type '[Action, number, string]' is not assignable to type 'ActionOrActionWithState<unknown, Action>'.
         Type '[Action, number, string]' is not assignable to type '[Action, unknown]'.
           Source has 3 element(s) but target allows only 2.
*/
    navigation(AComponent, {

const testConcatLatestFrom$ = createEffect(() => {
  return (of() as Observable<Action>).pipe(
    concatLatestFrom(() => [(of() as Observable<number>), (of() as Observable<string>)]),
/*
  👇 Receive error below
   
  TS2345: Argument of type '(source: ActionStateStream<unknown, Action>) => Observable<Action>' is not assignable to parameter of type 'OperatorFunction<[Action, number, string], Action>'.
   Types of parameters 'source' and 'source' are incompatible.
     Type 'Observable<[Action, number, string]>' is not assignable to type 'ActionStateStream<unknown, Action>'.
       Type '[Action, number, string]' is not assignable to type 'ActionOrActionWithState<unknown, Action>'.
         Type '[Action, number, string]' is not assignable to type '[Action, unknown]'.
           Source has 3 element(s) but target allows only 2.
*/
    navigation(AComponent, {

david-shortman added a commit to david-shortman/nx that referenced this issue Dec 18, 2021
A breaking change that enables data persistence operators to be used in streams with more than one slice of data (e.g., with `withLatestFrom` and `concatLatestFrom`).

Closes nrwl#6830
@github-actions
Copy link

github-actions bot commented Jan 1, 2022

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Jan 1, 2022
Coly010 pushed a commit that referenced this issue Jan 4, 2022
…8216)

A breaking change that enables data persistence operators to be used in streams with more than one slice of data (e.g., with `withLatestFrom` and `concatLatestFrom`).

Closes #6830
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants