Akita: InitialState inside the @StoreConfig parameter instead of super()

[GrandSchtroumpf]

I'm submitting a...

[X] Feature request

Current behavior

use super to provide the initial state in a @StoreConfig decorated class.

Expected behavior

Provide the initial state inside the @StoreConfig parameter :

@StoreConfig({
    name: 'session',
    initial: createInitialState()
})
export class SessionStore extends Store<SessionState> {}

What is the motivation / use case for changing the behavior?

This is not a common pattern in Angular to ask the dev to use super(). And as initialState() is not a dependency injection, I think it shouldn't be in the constructor of an injectable class.

[NetanelBasal]

This is not a common pattern in Angular to ask the dev to use super()

Akita isn't coupled to Angular, and I don't think this is a good argument. But I see how it could be a more convenient option, and I will check what can be done.

[NetanelBasal]

Unfortunately, it can't be done now without introducing a breaking change. The store expects to get the initial value in the constructor, and if you don't call super with it, Angular throws because it doesn't know what this value is.

If you know a workaround to get it done and don't introduce a breaking change, I would love to hear. Nice suggestion btw!

[GrandSchtroumpf]

Ok, I'll check that today. Thanks for trying =)

[GrandSchtroumpf]

I think I would change the store-config.ts with a decorated class that extends the Store and I would add the super(initialState) inside.
Here is a gist :
https://gist.github.com/GrandSchtroumpf/6d2d6b1be3f43170a6439371aea5bf0a

Then you can do :

@StoreConfig<SessionState>({
    name: 'session',
    initial: createInitialState()
})
export class SessionStore extends Store<SessionState> {}

Note: The <SessionState> after @StoreConfig is optional

[NetanelBasal]

But what about an existing code? You'll run it for each store in the application.

[GrandSchtroumpf]

Well, I don't see why it wouldn't work with the super() inside the SessionStore.
I'll clone the repo and check that.

You'll run it for each store in the application.

Sorry, I'm not sure what you mean...

[GrandSchtroumpf]

With this gist, and running npm test, I got "only" one error that I cannot fix :
Gist :
https://gist.github.com/GrandSchtroumpf/5557aae7c023244a4aa328c6e39274f4
Error :

 FAIL  akita/__tests__/dirty-check.spec.ts (5.207s)
  ● DirtyCheck › Watch specific property › should only reset the watched property

    expect(received).toBe(expected) // Object.is equality

    Expected: "akita"
    Received: undefined

Maybe you'll know better what's wrong.

[NetanelBasal]

Thanks for the implementation, but I can see four disadvantages here:

1. It adds more code to the bundle as each store now needs the following code:

var DecoratedStoreConfig = /** @class */ (function (_super) {
    __extends(DecoratedStoreConfig, _super);
    function DecoratedStoreConfig() {
        return _super.call(this, metadata.initialState) || this;
    }
    return DecoratedStoreConfig;
}(constructor));

1. It adds one more prototype lookup.
2. Because you set the initial value as a static property, the reference to the initial value will be in memory until the store is destroyed.
3. When you log the store, you will get an instance of DecoratedStoreConfig instead of the "real" store. (which can be annoying when debugging)

I don't think it worth it, I appreciate your time and I will probably add this to v2 when we introduce a breaking change.

Thanks!

[GrandSchtroumpf]

Indeed, it make sense. Thanks for taking the time to review this issue anyway !

[NetanelBasal]

No problem. We are open to anything that could help. Keep up the good work.

[NetanelBasal]

Added to the roadmap.