Platform: @ngrx/component-store Updater does not accept a Partial object

Created on 21 Oct 2020  路  13Comments  路  Source: ngrx/platform

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

Component Store

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

All 13 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
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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

oxiumio picture oxiumio  路  3Comments

axmad22 picture axmad22  路  3Comments

shyamal890 picture shyamal890  路  3Comments

alvipeo picture alvipeo  路  3Comments

brandonroberts picture brandonroberts  路  3Comments