-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
@ngrx/component-store Updater does not accept a Partial object #2754
Comments
Looks like it comes down to this typing. unknown extends V
? () => void
: (t: V | Observable<V>) => Subscription Something odd is going on when I try to reproduce this. In the stackblitz the types for the partial evaluate to () => void which is what I was seeing in my code. However, when pasting the same code into TsPlayground the types are resolved to (t: V | Observable) => Subscription |
I just bumped into this issue as well. |
Creating function overloads seems to work at providing the correct type for Partials<> and interfaces where all the properties are optional. updater(updaterFn: (state: T) => T): () => void;
updater<V>(
updaterFn: (state: T, value: V) => T
): (t: V | Observable<V>) => Subscription;
updater<V>(
updaterFn: ((state: T) => T) | ((state: T, value: V) => T)
): (() => void) | ((t: V | Observable<V>) => Subscription) Is it intended that you can provide an updater function that does not take a value? It currently works and you can use it to update the state. What is the intended purpose of the current typing? Looks like the subscription is hidden from the caller when a value is not provided but maybe I am missing something. |
Running into the same thing class Test extends ComponentStore<{ optional?: string }> {
// (property) Test.updaterTest: () => void
updaterTest = this.updater((state, update: { optional?: string }) => ({ ...state, update }));
// (property) Test.updaterTest2: () => void
updaterTest2 = this.updater<{ optional?: string }>((state, update) => ({ ...state, update }));
/**
* Type '() => void' is not assignable to type '(arg: { optional?: string; } | Observable<{ optional?: string; }>) => Subscription'.
* Type 'void' is not assignable to type 'Subscription'.ts(2322)
*/
updaterTest3: (arg: { optional?: string } | Observable<{ optional?: string }>) => Subscription
= this.updater<{ optional?: string }>((state, update) => ({ ...state, update }));
/**
* Without optional it works
* (property) Test.updaterTestNoOpt: (t: {
* optional: string;
* } | Observable<{
* optional: string;
* }>) => Subscription
*/
updaterTestNoOpt = this.updater<{ optional: string }>((state, update) => ({ ...state, update }));
} |
Same issue
Alternatively, as long as the payload type (
And you call it like so:
|
@andreisrob |
@alex-okrushko what is the reason behind the difference in return type for these two ways of calling the function? |
@StephenCooper
const updater = this.updater((state) => ({...state, value: valueIsComingFromTheGlobalScope}));
updater('foo'); // Error: Expected 0 arguments, but got 1
|
0 |
@alex-okrushko What version is this fix available in? I am still running into it. My version in package.json is 10.0.1. |
@brinehart |
I'm good with doing a small minor. V11 is going to be a little ways away. We'll look at cutting one next week |
In the meantime here is a workaround that I found works fine: In the updater function on the store set the type to the full object type without Partial and merge the values with the current version of state with Object.assign:
Then in the component that uses this pass in the
That will allow you to have strict typing in the component but also allow the item to be passed into the function without it throwing errors. This is just a workaround until the minor update goes out. |
Minimal reproduction of the bug/regression with instructions:
If you try to create an updater in your store that takes either a Partial or an interface where all the properties are optional then you cannot pass an object to the updater. It assumes that there is not object and so you get a compilation error of
Expected 0 arguments, but got 1
https://stackblitz.com/edit/comp-store-partial-update-bug?file=src/app/app.component.ts
Expected behavior:
Be able to call an updater with a Partial.
Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):
"@ngrx/component-store": "10.0.1",
"typescript": "~3.9.7",
Other information:
I would be willing to submit a PR to fix this issue
I have not looked at the code yet to see what might be the issue but will do shortly. Just getting this raised first.
[X] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No
The text was updated successfully, but these errors were encountered: