If you try to create an updater in your store that takes either a PartialExpected 0 arguments, but got 1
https://stackblitz.com/edit/comp-store-partial-update-bug?file=src/app/app.component.ts
Be able to call an updater with a Partial
"@ngrx/component-store": "10.0.1",
"typescript": "~3.9.7",
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
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
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
I believe this is a typescript problem. For some reason, ts treats an object that has no required props as unknown, which is the type-safety version of any, although optional props and any is not the same thing.
Here's a workaround:
@Injectable()
export class ComponentStoreWrapperForPartial<T extends object> extends ComponentStore<T> {
constructor(defaultState?: T) {
super(defaultState);
}
updaterForPartial<V extends Partial<V>>(updaterFn: (state: T, payload: V) => T) {
return super.updater((state: T, payload: V) => updaterFn(state, payload)) as (t: V | Observable<V>) => Subscription
}
}
Alternatively, as long as the payload type (V) has at least one required property, unknown extends V in the updater's return signature will resolve to false, so wrap your Partials in an object when you call your updater, like so:
myUpdater = this.updater((state, payload: { partial: Partial<SomeType>) => { ...state, ...payload.partial })
And you call it like so:
myUpdater({ partial: { key: val } })
@andreisrob
While you are right about TS acting a certain way when handing types of the callbacks, there are some tricks that could be done. 馃檪
I've done an even more interesting trick with effect typing 馃檪
@alex-okrushko what is the reason behind the difference in return type for these two ways of calling the function?
@StephenCooper
Two reasons:
1) When we don't specify the second argument - the value in the callback, I don't want anyone to be passing it to the updater by accident, so I do want to lock it down. e.g.
const updater = this.updater((state) => ({...state, value: valueIsComingFromTheGlobalScope}));
updater('foo'); // Error: Expected 0 arguments, but got 1
2) When the value is passed to the updater it would return Subscription, because internally it would create an Observable out of it. updater can also take the entire Observable instead btw.
Now with that Subscription I can unsubscribe if I want to (obviously if we just passed the single value, which would be executed synchronously, there's no benefit from it, but in case of Observable we can stop it feeding the data).
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
It'll be fixed in v11, unless we do the minor release @brandonroberts
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:
ex:
readonly updateSomeObject= this.updater((state, someParamters: SomeObject) => {
const updatedObjectToUpdate: SomeObject = Object.assign(state.someObject);
return {
...state,
someObject: updatedObjectToUpdate
};
});
Then in the component that uses this pass in the Partial<SomeObject> into the someObjectToUpdate with an as any reference
ex:
const valuesToUpdate: Partial<SomeObject> = {
oneProperty: 'Some Value',
aSecondProperty: 0
};
this.store.updateSomeObject(valuesToUpdate as any)
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.
Most helpful comment
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