feat(store): add set method to akita store

[field123]

Created a public method on the akita store that will completely replace the current state with the new provided state and not just update the value.

Closes #661

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/datorama/akita/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

What is the current behavior?

Issue Number: #661

What is the new behavior?

When the set() method is used the current state of the store will be replaced not just { ...currentState, ...newState } merged together.

See issue #661 for more details.

Does this PR introduce a breaking change?

• Yes
• No

Other information

[NetanelBasal]

I prefer using a simple example state and not copy-paste from your application.

[field123]

This was a much reduced version of the original, I though that session state was a good simple example and "real world" enough to be useful. My preference when reading documentation has always been simplified "real world" over slightly contrived.

I could simplify the example further to

type SessionState = AuthenticatedState | UnauthenticatedState;

interface AuthenticatedState {
  _tag: 'authenticated';
  uid: string;
}

interface UnauthenticatedState {
  _tag: 'unauthenticated'
}

or even do something like

type SessionState = TokenState | InitialState;

interface TokenState {
  name: string;
  token: string;
}

interface InitialState{
  name: string;
}

[NetanelBasal]

I prefer the latter

[field123]

I'll change to the second option.

[NetanelBasal]

I don't think we need all of this. The method name is straightforward.

[field123]

I was concerned that newer users may be unsure when to use each method, but it could be way to verbose.

[NetanelBasal]

The overwrite name is clear enough.

[field123]

I'll make the change.

[NetanelBasal]

• Why do we need a function?
• Why do we need to create a new function each time?

[field123]

1. Why do we need a function?

I've used a hook function so the prepareNewState method is not bound to a specific class method e.g akitaPreOverwrite or akitaPreUpdate instead a wrapping function of the relevant class method is used in update, overwrite or any future methods. This allows for the same class class overriding of the methods yet still being able to pass them as HOF.
const hookFn = (curr: S, newS: S) => this.akitaPreOverwrite(curr, newS as S)

Using the hook means the ordering of operations can be maintaned.

2. Why do we need to create a new function each time?

In an effort to make small single purpose functions I like to lexically scope util functions. It does have the possible performance hit but I think the clarity of being able to scroll to the bottom of a method/fn and know what's happening is worth it.

const newState = extractNewState(stateOrCallback, currentState);
const withHook = hookFn(currentState, newState);
return resolveFinalState(currentState, withHook);

This makes it clear I'm extracting the new state from my callback and current state -> apply my hook to the currentState and newState -> finally resolving the final state. I could makes these more descriptive but don't understand what the _producerFn is or why your checking if the state is an object.

You can also pluck out these small functions and raising their scope if you have a need for them elsewhere.

The alternative would be

let newState;
if (isFunction(stateOrCallback)) {
 newState = isFunction(this._producerFn) ? this._producerFn(currentState, stateOrCallback) : stateOrCallback(currentState);
} else {
 newState = stateOrCallback;
};
const withHook = hookFn(currentState, newState);
return isPlainObject(currentState) ? withHook : new (currentState as any).constructor(withHook);

While this can be made more clear through comments it could be tricky to understand what the fn/method is doing.

If the nest functions are a concern from performance we could put the functions in private methods, extract them out of the class or of course just remove them and have all the code blocked together.

// Option 1
// Inside the Store class

private extractNewState(x: Partial<S> | UpdateStateCallback<S>, cs: S) {
    if (isFunction(x)) {
      return isFunction(this._producerFn) ? this._producerFn(cs, x) : x(cs);
    } else {
      return x;
    }
  }

  private resolveFinalState(currentState: S, withHook: S): S {
    return isPlainObject(currentState) ? withHook : new (currentState as any).constructor(withHook);
  }

// Option 2

// Before or after the Store class def

const extractNewState = <S>(x: Partial<S> | UpdateStateCallback<S>, cs: S, _producerFn: (state: any, fn: any) => any) => {
  if (isFunction(x)) {
    return isFunction(_producerFn) ? _producerFn(cs, x) : x(cs);
  } else {
    return x;
  }
};

const resolveFinalState = <S>(currentState: S, withHook: S): S => {
  return isPlainObject(currentState) ? withHook : new (currentState as any).constructor(withHook);
};

That's my reasoning, happy to do what ever fits best with the project. 👍

[NetanelBasal]

Why do we need that constructNewState and resolveFinalState to be local functions?

Why are you not inline them?

[field123]

I find this:

const extractNewState = (x: Partial<S> | UpdateStateCallback<S>, cs: S) => {
      if (isFunction(x)) {
        return isFunction(this._producerFn) ? this._producerFn(cs, x) : x(cs);
      } else {
        return x;
      }
};

const inCaseClassResolveState= (currentState: S, withHook: S): S => {
   return isPlainObject(currentState) ? withHook : new (currentState as any).constructor(withHook);
};

const newState = extractNewState(stateOrCallback, currentState);
const withHook = hookFn(currentState, newState);
return inCaseClassResolveState(currentState, withHook);

To be more immediately understandable than:

let newState;
if (isFunction(stateOrCallback)) {
 newState = isFunction(this._producerFn) ? this._producerFn(currentState, stateOrCallback) : stateOrCallback(currentState);
} else {
 newState = stateOrCallback;
};
const withHook = hookFn(currentState, newState);
return isPlainObject(currentState) ? withHook : new (currentState as any).constructor(withHook);

If future me or another developer comes along and sees the line of code isPlainObject(currentState) ? withHook : new (currentState as any).constructor(withHook); it may not make sense without diving in and understanding it. But if the block of code is wrapped in a function called inCaseClassResolveState you get some understanding straight away of what the operation is doing.

With the added benefit of not mutating (with variable reassignment) the newState variable once declared. e.g. using const and not let.

I'm happy to just change the class method to match below if you prefer?

private prepareNewStateAlt<S>(stateOrCallback: Partial<S> | UpdateStateCallback<S>, currentState: S, hookFn: (curr: Readonly<S>, newS: Readonly<S>) => S): S {
    let newState;
    if (isFunction(stateOrCallback)) {
      newState = isFunction(this._producerFn) ? this._producerFn(currentState, stateOrCallback) :    stateOrCallback(currentState);
    } else {
      newState = stateOrCallback;
    }
    const withHook = hookFn(currentState, newState);
    return isPlainObject(currentState) ? withHook : new (currentState as any).constructor(withHook);
}

[NetanelBasal]

Add comments please, we don't need to create redundant functions.

[field123]

Changes done.

[NetanelBasal]

Same here

[tony]

@field123 Can you rebase?