chromaui / learnstorybook-code

Code for Learn Storybook
https://learnstorybook.com
MIT License
396 stars 1.21k forks source link

Angular: Use service instead of ngxs to share state #44

Closed maiermic closed 3 years ago

maiermic commented 4 years ago

The Angular documentation suggests to use a service to communicate and share state between parent and child components. It is unnecessary to introduce a library like ngxs to the tutorial. The reader may not be familiar with ngxs, but a service is a basic building block that every reader should know.

usrrname commented 4 years ago

👋 Hi again! I too am wondering why it was included, however, maybe it is a way of making a global state object available to different components or stories? Would you like to make a PR for a separate branch including services? I'm helping trying to help update their doc to make their dependencies support 6.0.22+ in LearnStorybook. https://github.com/usrrname/learnstorybook-v6-angular

jonniebigodes commented 4 years ago

@maiermic thank you for bringing this to our attention. To give you some context, i was not the author of the tutorial and i've been maintaining it to the best of my knowledge. Based on experience with the tutorial, the ngxs library was introduced partially to maintain a measure of consistency across the frameworks implementing a redux pattern for angular. The aim of the data and screen sections is to demonstrate how you could use a package like ngxs or any other data management one in an application/project and solve some common issues that might come from implementing external providers, not going to deeply into the whole implementation and what is ngxs.

I'm just thinking out from the top of my head, but to use a service as opposed to the ngxs package for state management we would probably would have to have to mock it somehow...

As @usrrname mentioned if you can draft up a pull request on his repo i'm more than happy to go over it as well.

Hope my comment shed some insights on the issue.

Stay safe both of you.

usrrname commented 4 years ago

@maiermic you might enjoy this video https://www.youtube.com/watch?v=Azus3_CEkpw It gave me a greater understanding of why use services vs a state manager in Angular.

maiermic commented 4 years ago

@usrrname Hans Schenker points out (in his comments) that you do not need NgRx (an additional library) to apply the Redux/Flux Pattern in RxJS. The motivation of the speaker to introduce the Redux/Flux Pattern are race conditions. However, the Redux/Flux Pattern doesn't solve these by default. You still have to handle them manually. You can do the same with RxJS only.

The example of the Storybook tutorial does not handle race conditions and shouldn't IMO (too much overhead). Otherwise, the Redux/Flux Pattern would be appropriate IMO.

services vs a state manager in Angular

The Angular documentation defines

Service is a broad category encompassing any value, function, or feature that an app needs. A service is typically a class with a narrow, well-defined purpose. It should do something specific and do it well.

A state manager does not have to build upon Redux/Flux Pattern. The term state manager is way more generic. Further, a state manager should be provided as a service (like Store of ngxs). In other words, a state manager is a service or rather a specific tool to implement a service. How you manage state is an implementation detail that should be hidden by the service, even though this may only be possible to a certain degree due to the exposed interface of the service.

maiermic commented 4 years ago

Based on experience with the tutorial, the ngxs library was introduced partially to maintain a measure of consistency across the frameworks implementing a redux pattern for angular.

@jonniebigodes I see your viewpoint as a maintainer. Unfortunately, it clashes with the viewpoint of the reader, who may not be familiar with the Redux/Flux Pattern. This makes it difficult to understand for the average reader.

The aim of the data and screen sections is to demonstrate how you could use a package like ngxs or any other data management one in an application/project and solve some common issues that might come from implementing external providers, not going to deeply into the whole implementation and what is ngxs.

Isn't the main point that an injected dependency (service) of a nested (child) component can have (side) effects that you have to avoid or work around in Storybook (similar to tests)? The data section shows a work around. Split component into presenter (no dependencies) and container (has dependencies). Now you can use the presenter in Storybook. However, you still have the issue if the container is used in another component. The screen section shows how to provide dependencies in a story. This may be straight forward in ngxs (in this case), but it depends on the external provider. How do you handle non-local (state) side effects like HTTP requests? Who does these requests in case you are using ngxs? As far as I understand, they would be done in the store by calling a service (dependency of the store) that uses Angular HttpClient. That would be a dependency that you have to mock/stub in your story, wouldn't it?

I'm just thinking out from the top of my head, but to use a service as opposed to the ngxs package for state management we would probably would have to have to mock it somehow...

Why? You only do local state management. As I pointed out above, it is the same reason, why you can inject ngxs (Store) without using a mock in the first place. It wouldn't be as easy if you do HTTP requests.

usrrname commented 4 years ago

@maiermic thanks for raising those observations. I'm going to read through the sources you provided and rethink the approach. Thanks for taking your time to share that reasoning.

usrrname commented 4 years ago

@maiermic In the case of using ngxs I'm not sure there is a need for services anymore because that logic is relocated to actions.ts. It is possible however that the fetching of external data would happen through HTTPClient which returns an observable, and that data would be sent to the actions.ts to be used in the reducer-like functions ... that of which I have not figured out how to write a test for yet. These are all great questions by the way. I love being challenged on my assumptions.

maiermic commented 4 years ago

that of which I have not figured out how to write a test for yet

@usrrname You have to provide the dependency. However, you can use any implementation of the dependency interface, i.e. you can use different implementations in e.g. production, tests and stories. You may use the same implementation in production and e2e (end-to-end) tests, but you like to avoid the side-effects (e.g. HTTP requests) in your stories and unit tests to make them independent (no server as dependency). You may use a mock, stub or a another/custom side-effect free implementation.

Who does these requests in case you are using ngxs? As far as I understand, they would be done in the store by calling a service (dependency of the store) that uses Angular HttpClient.

You like to extract the service as a dependency that builds and runs the HTTP requests to act up to the single responsibility principle. Further, you should keep the interface segregation principle in mind, while you decouple services, components, etc.

maiermic commented 4 years ago

@jonniebigodes TasksState is used like a global state object, since all (container) components (i.e. TaskListComponent and InboxScreenComponent) use the same state object (TasksState). Is a global state object intended or only the case due to the simplicity of the example (no more components that do not share state)?

maiermic commented 4 years ago

@jonniebigodes It is written in the screen section that InboxScreenComponent is

pulling a top-level error field out of our store (let's assume we'll set that field if we have some problem connecting to our server)

but it is not mentioned, who dispatches the error action.

Who does these requests in case you are using ngxs? As far as I understand, they would be done in the store by calling a service (dependency of the store) that uses Angular HttpClient.

If my assumption is correct, TasksState would dispatch AppError if the call to the service that connects to the server (missing in the example) returns an error (it is probably more precise that the error is emitted on the Observable returned by the service). In that case, TaskListComponent does not know/care that errors may occur. Only InboxScreenComponent does. But both share the same state object/manager, i.e. its interface(s) is/are shared. Hence, the interface segregation principle is violated. Besides, TaskListComponent may dispatch any action on ngxs's Store, doesn't it? There is no explicit interface type defined, which actions may be passed to store.dispatch. This makes it more difficult to get an overview of the area of responsibility of a component. Consequently, it is more difficult to comply with the interface segregation principle.

maiermic commented 4 years ago

@jonniebigodes I wouldn't call PureTaskListComponent pure as long as it sorts items :wink: The pure list should not care about the order. It should just render what it gets. There may be more features you like to add in the future (e.g. filtering) or you'd like to change the order (sort behaviour). That's one reason, why Jeremy Elbourn (member of the Angular Components team) suggests to design components with composition.

jonniebigodes commented 3 years ago

To give a bit more context on this. This repo has a working implementation with Storybook and State (service), as you may see it will work perfectly with Storybook. But applying this scenario breaks the tutorial, more specifically the section as the intended behavior is to demonstrate that you can add more context into the Story (related to the providers) and so on. Technically we want to break the stories internationally in order to learn how to solve them.

maiermic commented 3 years ago

Thank you for the linked showcase project and for maintaining the tutorial :+1: I really appreciate it :smiley:

jonniebigodes commented 3 years ago

No need to thank whatsoever. Glad that I was able to help. And sorry for not keeping the Angular branch up to date. As it required a bit of work to be up to par. I had to recreate it from scratch.