ngxs / store

πŸš€ NGXS - State Management for Angular
http://ngxs.io
MIT License
3.53k stars 399 forks source link

Lazy Loading documentation improvement #868

Closed thomas-ama closed 5 years ago

thomas-ama commented 5 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[x] Documentation issue or request
[ ] Support request => https://github.com/ngxs/store/blob/master/CONTRIBUTING.md
[ ] Other... Please describe:

Current behavior

Hello,

My team and I have been using NGXS for a few days. I have read all the documentation on the website. It is rather complete and well written overall!

However, the part on lazy-loading is a little ambiguous. In particular, the following paragraph:

You probably defined a AppState interface that represents the global state graph but since we lazy loaded this we can't really include that in the definition. To handle this, let's extend the AppState and use that in our a component like:


export interface AppStateModel {
zoos: Zoo[];
}

export interface OfficesStateModel extends AppStateModel { offices: Office[]; }


What **this** and **that** refer to?
What's the benefit of extending `AppStateModel`? (It seems to work without doing it).

Then,
> You probably defined a AppState interface that represents the global state graph

What do you mean by global state?
My global state looks like this:

{ appState: {}, officesState: {} }


So AppState is not really the global state. Was I mistaken?
Should the result look more like this?

{ appState: { officesState: {} } }


Or even like that:

appState: { officesState: {} }


I don't know if this last solution is possible, but that would be great for state typing (more explanations [here](https://stackoverflow.com/questions/54856628/giving-a-type-to-the-root-state).)

Thanks a lot,
Thomas
arturovt commented 5 years ago

@markwhitfeld should we close this?

splincode commented 5 years ago

I think we need to improve docs for this

arturovt commented 5 years ago

@thomas-ama You're saying that:

The problem is that the state variable has no types: no type checks, no auto-completion, etc

It can't. You also can't create an interface for that as your feature states may not be initialized at this moment. And also I'd ask what is the motivation of using the callback function? IMHO that's much better to provide a state class as an argument or static selector. I've never used such approach as there is no need. :relieved:

You probably defined a AppState interface that represents the global state graph

This is just a semantic overview of the global graph and it doesn't force you to create an interface for the global state :wink:

But yes need to rephrase it.

thomas-ama commented 5 years ago

And also I'd ask what is the motivation of using the callback function? IMHO that's much better to provide a state class as an argument or static selector. I've never used such approach as there is no need. 😌

Indeed, over time I understood that using selectors brought many advantages, including typing!

About typing, here is a message from Mark from NGXS Slack:

We will be adding a lot of type safety in v4 because a lot of it requires an upgrade to Typescript 3. In NGXS v3 we are committed to be backward compatible to TS 2.5

splincode commented 5 years ago

Released in 3.5.0