Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thanks, but... #42

Open
scottschafer opened this issue Jun 6, 2017 · 9 comments
Open

Thanks, but... #42

scottschafer opened this issue Jun 6, 2017 · 9 comments

Comments

@scottschafer
Copy link

scottschafer commented Jun 6, 2017

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
Copy link
Member

SethDavenport commented Jun 6, 2017

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

scottschafer commented Jun 8, 2017

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
Copy link
Member

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

@Wykks
Copy link

Wykks commented Jun 13, 2017

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
Copy link

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
Copy link

lvidal1 commented Jun 23, 2017

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
Copy link
Member

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

  • TodoMVC is too simple of an app generally - and putting redux + redux-observable + .... on-top of it feels like an over-engineered approach. Even if the intent is to demonstrate concepts, people get hung up on "I could do this without redux, and it'd be easier, and less code" (which is likely true)
  • Trying to find a non-contrived/non-trivial example where people don't get too tripped up on domain issues can be hard, or leads to needing build out a whole lot for it to be useful.

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

  • simple state management with a reducer in a component - no "redux" at all yet
  • refactoring that out into a reducer / action creator
  • setting up basic redux with just the logger
  • refactor component to use that
  • add in an API call, deal with it in the component for now
  • then add in something like redux-observable, and refactor towards that

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants