feat(angular): support multiple state slices in data persistence ops

[david-shortman]

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

Current Behavior

The data persistence operators only work on a single slice of state.

Expected Behavior

The data persistence operators work on multiple slices of state (e.g. selecting multiple slices with withLatestFrom and concatLatestFrom).

Related Issue(s)

Fixes #6830

[nx-cloud]

☁️ Nx Cloud Report

CI ran the following commands for commit 2126e1a. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 7 targets

• nx affected --target=e2e --base=5026038c3c033a121a554ce24cb5a531f9ed3dbe --head=2126e1a3b3f535976344702ebd5a69371fdb26b2 --exclude=e2e-react-native,e2e-detox,e2e-gatsby --parallel=1
• nx run-many --target=build --all --parallel=8
• nx run-many --target=build --all --parallel=8
• nx run-many --target=build --all --parallel=8
• nx affected --target=test --base=5026038c3c033a121a554ce24cb5a531f9ed3dbe --head=2126e1a3b3f535976344702ebd5a69371fdb26b2 --parallel --max-parallel=3
• nx affected --target=build --base=5026038c3c033a121a554ce24cb5a531f9ed3dbe --head=2126e1a3b3f535976344702ebd5a69371fdb26b2 --parallel --max-parallel=3
• nx build typedoc-theme

---

Sent with 💌 from NxCloud.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nrwl/nx-dev/bQnE5z5bx3rWk5KMUgsP7fQUJokx
✅ Preview: Canceled

[Coly010]

Thanks for submitting this PR 🎉 !
My initial thoughts are:

Does this still work with a single slice of state, without using concatLatestFrom and withLatestFrom?
The typing of T extends Array<unknown> doesn't feel quite right to me at first glance.

I feel like there must be a way to infer the type rather than using unknown also.

I know we have no tests right now in the Nx repo for this, but it would be super helpful if you could write some tests that covered the following use-cases:

• Previous Behavior without withLatestFrom/concatLatestFrom
• New Behavior with withLatestFrom/concatLatestFrom

[david-shortman]

@Coly010

Does this still work with a single slice of state, without using concatLatestFrom and withLatestFrom?

In the generic arguments for the various operators, T previously represented the shape of the "state". Now, it is a tuple containing each "slice" of state.

The functions are modified to be variadic, where the arguments have changed from (action, state) to (action, ...stateSlices). This means that existing usage, passing in (action, state), still works because "state" is a single slice.

The typing of T extends Array doesn't feel quite right to me at first glance.

T now represents the slices of state, and must be a tuple. This means that if one's state was previously an array, say a list of ids string[], then this will now be recognized as the slices tuple [string[]].

I feel like there must be a way to infer the type rather than using unknown also.

The type guard for the generic argument T is ensuring it is assignable to an Array of unknown elements. The infer keyword cannot be used in a type guard (although it uses the extends keyword, it's not the same as a conditional type). We don't "care" what the type of the elements of the array are because the type of the entire array is sufficient to use for the variadic slices argument.

I know we have no tests right now in the Nx repo for this, but it would be super helpful if you could write some tests that covered the following use-cases:

I'll see to it, and see if I can restore the previously commented out cases if possible.

[Coly010]

Thanks for the thorough answer!

This means that existing usage, passing in (action, state), still works because "state" is a single slice.

This is basically where my concern comes from.
If state was simply true, then T extends Array and ...slices would fall over.
Are there any scenarios where state will not be an array?

The infer keyword cannot be used in a type guard

This was a mistype on my part. I did not mean the actual infer keyword, rather the idea of inferring it from the previous function.

e.g. the return type of withLatestFrom(this.store.select(myString)) would result in Observable<[Action, string]>
I was hoping we might have been able to provide a better type than Array<unknown> in this scenario.

Does this mean that using

withLatestFrom(this.store.select(myString)),
navigation<AComponent, {
run: (action, [myString]) => 
...

will result in myString being of type unknown, or will TS correctly match the type from the withLatestFrom(this.store.select(myString)) ?

I'll see to it, and see if I can restore the previously commented out cases if possible.
Awesome 🔥

Thanks again for the PR! I just want to make sure we don't introduce breaking changes into existing codebases with this, or introduce type issues for people 🙂

[david-shortman]

e.g. the return type of withLatestFrom(this.store.select(myString)) would result in Observable<[Action, string]>
I was hoping we might have been able to provide a better type than Array in this scenario.

Correct, the inferred type is Observable<[Action, string]> in the scenario.

Does this mean that using

withLatestFrom(this.store.select(myString)),
navigation<AComponent, {
run: (action, [myString]) => 
...

will result in myString being of type unknown, or will TS correctly match the type from the withLatestFrom(this.store.select(myString)) ?

The generic argument is a tuple, but the actual function argument is variadic.

So in your example, one would probably write this instead:

withLatestFrom(this.store.select(myString)),
navigation<AComponent, {
  run: (action, myString) => // myString is inferred to be a `string`
  ...

But the typing doesn't have to be explicitly provided. The following will infer like so:

// with one "slice"
withLatestFrom(this.store.select(myString)),
navigation(AComponent, {
  run: (action, myString) => // myString is inferred to be a `string`
  //  ...
)

// with multiple "slices"
withLatestFrom(this.store.select(myString), this.store.select(myNumber)),
navigation(AComponent, {
  run: (action, myString, myNumber) => // myString is inferred to be a `string`, myNumber a `number`
  // ...
)

[david-shortman]

Thanks again for the PR! I just want to make sure we don't introduce breaking changes into existing codebases with this, or introduce type issues for people 🙂

The only breaking change intended is that explicitly provided generics will need to be updated.

It is likely that most consumers of the operators are not explicitly providing generics, but using inference.

If one previously used an operator with explicit generic arguments like so:

navigate<AComponent, State>(AComponent, { run: (action, state) => {} })

Then they would have to update to:

// Only the generic argument for "Slices" changes to be a tuple
navigate<AComponent, [State]>(AComponent, { run: (action, state) => {} })

[Coly010]

No problem. Thanks for clarifying all that!

[github-actions]

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.