joshburgess / redux-most

Most.js based middleware for Redux. Handle async actions with monadic streams & reactive programming.
https://www.npmjs.com/package/redux-most
MIT License
139 stars 14 forks source link

Rewrite createEpicMiddleware without Subjects to simplify and shrink build (removes replaceEpic) #14

Closed joshburgess closed 7 years ago

joshburgess commented 7 years ago

Rewrite createEpicMiddleware without Subjects to simplify and shrink build (removes replaceEpic)

I talked with @davidchase about rewriting createEpicMiddleware without Subjects, and it looks like we can simplify the code a lot just by changing createEpicMiddleware to:

export const createEpicMiddleware = epic => store => next => action => {
  const actionsIn$ = just(action)
  const actionsOut$ = switchMap(x => epic(just(x), store), actionsIn$)

  observe(store.dispatch, actionsOut$)

  return next(action)
}

Pros:

Cons:

The redux-observable docs say that replaceEpic can be used for hot reloading & code splitting, but I haven't been using it. Do any of you use it? Can you provide some examples of how you're using it?

Do people think it's worth removing replaceEpic to get a smaller build size?

@jshthornton @Vlad-Zhukov @evilsoft

jshthornton commented 7 years ago

If hmr is desired then we could write in support for it only in development?

briancavalier commented 7 years ago

This looks like a really nice simplification! I think you may be able to go further with it by applying a few algebraic equivalences (e.g. switchMap(f, just(x)) ~= f(x)). The code in the OP effectively uses just to ensure that next is always called before epic. Is that a requirement? If so, I don't think the simplification I have in mind will work. If not, then it may--happy to post the idea and discuss if that's the case.

joshburgess commented 7 years ago

@briancavalier I've been talking with @davidchase some more and trying things out. This is what I've settled on so far:

const createEpicMiddleware = epic => store => {
  store.dispatch({ type: '@@INIT_EPICS' })
  return next => action => {
    observe(store.dispatch, epic(just(action), store))
    return next(action)
  }
}

The @@INIT_EPICS dispatch isn't necessary. Just added it to fit in with other redux middleware conventions so the user can see when the middleware is setup in the console or the Redux DevTools.

It's definitely a lot simpler. The only problem is trying to come up with a way to write that replaceEpic function without Subjects. I'm not sure there is a way. Can you think of anything?

Maybe, I can just have two packages, redux-most which is written entirely without Subjects and doesn't feature a replaceEpic function and redux-most-subject which is written with Subjects and allows users access to replaceEpic at the cost of more dependencies & a larger build size.

Thoughts?

joshburgess commented 7 years ago

Actually, I think it might be possible by using takeWhile or until and dispatching an action from within replaceEpic to signal to that stream to stop & then just recreating the middleware if we held a mutable reference to it earlier... not 100% sure, but I'm going to try it out when I get home later.

briancavalier commented 7 years ago

@joshburgess Cool, that's the simplification I was hinting at. Quite often when you see anotherStreamOp(..., just(x)), it can be simplified.

I'm not familiar enough with replaceEpic to offer any useful suggestions on it specifically right now. I'll take a look at it, though. If it's something that people use infrequently, then a separate package might be a good option.

joshburgess commented 7 years ago

@briancavalier After trying that rewrite out a bit, I realized that it doesn't work in the same way that redux-observable and redux-most currently do. When you call just(action) you're creating a new stream every time, and that's not what we want to happen, because we want to be able to debounce, throttle, etc. on subsequent actions of the same type.... In the current version, this works because all epics (functions that take in an action$ and an instance of the Redux store) are taking in the same pre-existing action$ that all actions are being piped through.

The middleware always ends by returns a function like:

action => {
  observe(store.dispatch, epic(just(action), store))
  return next(action) // move above observe if there are any timing issues
}

This is the function that store.dispatch uses... This means that we're actually creating a brand new stream each time we call just(action) whenever we dispatch an action, even when it's the same type. Unfortunately, because of this, I think Subjects are required... as we need to call our epic() functions on a stream that all dispatched actions travel through. At least, I can't think of a way to make that happen without Subjects... Can you?

joshburgess commented 7 years ago

I'm going to close this for now. Like with CycleJS, I think this is one of those cases where using Subjects is required to abstract away some details into a library/framework that the user doesn't see or interact with in order to facilitate the use of a specific pattern in user land.

Also, I've been using replaceEpic a bit for hot module reloading in a new app at work where I've been using redux-most, and I think it's pretty useful.

If anyone thinks of another way, feel free to comment and I'll reopen.