angular-redux / example-app

Example using @angular-redux/store, @angular-redux/router, and @angular-redux/form together with the Angular CLI
123 stars 101 forks source link

Thanks, but... #42

Open scottschafer opened 7 years ago

scottschafer commented 7 years ago

I always appreciate when people provide working examples, and I can see you've put significant time into this.

However, I feel compelled to say that if this example application illustrates the correct use of the library, it has convinced me to look elsewhere. I am very new to redux and that's certainly a factor. Yet "beginner's mind" can be valuable. If we bring new engineers onto this project, they will face the same learning curve that I did. Here was my experience.

  1. I realize you didn't invent the term, but I found "epic" to be confusing and non-descriptive. From what I can tell, it's a method to observe redux actions and act on them without modifying them. This seems a lot like an observable, except with a LOT more overhead.

  2. The code structure is really hard for me to follow. In particular, many files called "epic.ts", etc, in which their purpose is only made "clear" by seeing the path the file is in. After many hours of working with this structure I still found myself very confused trying to navigate it.

  3. The code inside epic.ts feels extremely over-engineered to me, overly clever, and it's not obvious to me how it will scale. For example:

  private createLoadAnimalEpic(animalType): Epic<AnimalAPIAction, IAppState> {
    return (action$, store) => action$
      .ofType(AnimalAPIActions.LOAD_ANIMALS)
      .filter(action => actionIsForCorrectAnimalType(animalType)(action))
      .filter(() => animalsNotAlreadyFetched(animalType, store.getState()))
      .switchMap(a => this.service.getAll(animalType)
        .map(data => this.actions.loadSucceeded(animalType, data))
        .catch(response => of(this.actions.loadFailed(animalType, {
          status: '' + response.status,
        })))
        .startWith(this.actions.loadStarted(animalType)));

Now, I understand that boilerplate is necessary here, and that in a trivial example like this the boilerplate may seem especially silly. However, this boilerplate seems to me like it will need to be repeated endlessly throughout the application with every new API. I also am not sure about performance. Does every action pass through these filters?

I am reminded a bit of this spoof, in which a trivial problem is solved using industry standard design patterns and libraries and made exponentially more complicated. https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition

This is just my opinion, and as I've said, I'm new to redux. However, I still maintain there's something wrong with the implementation of this library if this is the "correct" way to use it.

Thanks for listening.

P.S. I have switched over to investigating the architecture used in ngrx, namely: https://github.com/ngrx/example-app.

To me, this is just vastly easier to follow. Just the naming of the file "book.ts" in the models folder makes a huge difference to me. Can you make a case for the advantages of angular-redux library over ngrx?

SethDavenport commented 7 years ago

Interesting. I will look at your complaints in more detail later. File naming in particular is easy enough to change.

However it's worth noting that ngrx works almost identically. If you don't like epics, you won't like ngrx/effects, which is a bespoke reimplementation of epics. So there's that.

As for the advantages of our store over ngrx, again, they work the same. However we have chosen to maintain compatibility with Redux community middlewares whereas they choose to reimplement popular middlewares like redux-observable from scratch.

That their example app may be structured differently than this one does not mean their implementation of redux is better or worse.

If you want to use community middlewares, use us. If you want to wait for Google to produce a 'blessed' reimplementation of each middleware, use ngrx.

scottschafer commented 7 years ago

You're right. I see that Effects is equivalent to epics. And I'm struggling with that architecture a bit too.

I've seen this push to decouple everything as much as possible in the industry, and while it's great in some ways (separation of concerns), it sometimes feels to me like the goal is to obfuscate as much as possible "on x, do y".

If the goal is to have cleaner, more logical and less brittle code, then I have to wonder if this is achieving that goal. It seems like there are many points of failure here and a steep learning curve.

JMO.

SethDavenport commented 7 years ago

Although FWIW I should probably look at their example and see if we can harmonize on naming conventions, etc. This app is kind of contrived to be sure.

scottschafer commented 7 years ago

This is probably not the place for this, but I wonder if you wouldn't mind taking a look at a project I threw together. https://scottschafer.github.io//xcelsior-sample/

Obviously, this represents a huge amount of hubris on my part, but hey.

This was my attempt to answer the question of whether the core ideas of redux (one way data flow, immutable models) could be captured using fewer moving parts. This is what I came up with. I'm using the "immutable-assign" library to deduce model changes so that you express changes to the model using plain javascript.

I'd very much welcome your feedback.

SethDavenport commented 7 years ago

Hey - sure - I'll try and take a look next week.

Wykks commented 7 years ago

Just my 2 cents... I personnaly prefer the structure of this example project instead of the ngrx one. It's (mostly?) following official angular styleguide, in particular the folder-by-feature (https://angular.io/guide/styleguide#folders-by-feature-structure). Whereas ngrx example is more folder-by-concept (often used in simple react app), and trying to lazyload a feature module with this structure, is going to be messy.

doronnac commented 7 years ago

The great thing about this library is that it's compatible with standard redux middleware. redux-observable is a redux middleware which is unrelated to this library and can be removed / replaced by others that more suit your requirements. Regarding folder structure / etc. it seems that React integrates more cleanly with Redux with wrappers and mapStateToProps, while Angular users have to adapt with the more generic Angular's DI. Different setup, same result.

lvidal1 commented 7 years ago

Now, I do not feel alone by reading this thread. I love how this implementation works with redux and its middleware, however, I am also still very confused with the little parts. I notice that in order to understand how all these piece of code work together, one must have many concepts in mind first. I do not pretend to ask you more about this job, but could you please draw a roadmap or a little explanation on how data behave after been gotten by the ApiService thorugh the animals modules at least?

e-schultz commented 7 years ago

Hi,

This is something I've been thinking about lately - and how to better document / introduce ideas.

A large part of the appeal in the redux pattern is the simplicity of the ideas behind it, I also like how the redux library itself does very little - it manages state, and that's about it.

This leads to quite a bit of flexibility, and how to approach things - but for many people, it's also a little too close to the metal at times. It's sort of like express for node - the core of it does very little, it's the ecosystem around it that really brings it to life.

This comes with a trade off though - as once you start to go beyond basic use cases, it can become overwhelming to understand all the different parts, how to do things, best practices, etc.

The challenge that I've found when trying to teach / give examples / etc, is a bit of

I've been trying to come up with a happy middle-ground of a demo app / tutorial that could incrementally introduce concepts, and have logical refactoring points to pull things out into the appropriate locations.

Ideally, a rough outline in my head would be something like

Trying to learn everything all at once, especially if learning Angular, TypeScript, and RxJS at the same time as redux + the eco-system around it - it can be quite the head bender.

I'm open to ideas of an example app to build out in this way. I know I've just said I'm not the biggest fan of "TodoMVC", but I'm wondering if something like re-building Todometer would have enough additional complexity to help.