Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Redux Saga Example #8

Closed
wants to merge 1 commit into from

Conversation

giamir
Copy link

@giamir giamir commented Dec 1, 2016

Add a simple example to prove compatibility with the redux saga middleware.

@mpeyper
Copy link
Contributor

mpeyper commented Dec 2, 2016

Thanks, your example works and bonus marks given for matching the existing examples 👍

My only concern is that the the sagas aren't behaving correctly when using the namespacing feature. The actions being dispatched do not get the namespaced action type. I haven't used sagas myself before so I'm a bit unsure how to make this work.

@vivian-farrell can we accept this and raise an issue for the missing half of the functionality (should probably do this anyway tbh), or should it be all or nothing?

@giamir
Copy link
Author

giamir commented Dec 2, 2016

@mpeyper I'm pretty new to redux-saga as well.
My tech lead suggested me to have a read on how you scaled React and Redux at IOOF. We are currently facing similar challenges and the idea of having micro-frontends is very interesting.
I will try to understand more in depth how you are managing namespacing and try to fix the issue.

@mpeyper
Copy link
Contributor

mpeyper commented Dec 2, 2016

@giamir So namespacing is useful if your micro-frontends want to use the same type for some action types (e.g. "LOAD", "SAVE", etc.). Manually namespacing your action types is not a big deal if you only want to have a single instance of a micro-frontend in you app as you can come up with a naming convention and be done with it. From your example, sagas will work fine in that scenario. If you want to re-use a micro-frontend in a different context more than once, then it gets a little more complicated.

redux-subspace can get around it by automatically namespacing dispatched actions within a subspace and the namespaced reducers will disregard any actions for other micro-frontends. This is achieved by wrapping the dispatch function provided by the redux store to modify the type of any actions passing through it.

The challenge with sagas (as far as I can tell) is that it the the saga middleware is not expecting the namespace to be there.

To map this back to your example if we namespaced the micro-frontend with "component", the type here would be "READY_TO_CHANGE_VALUE"

export default function* sagas() {
  yield takeEvery(readyToChangeValue().type, changeValueGenerator)
}

but the type here would be "component/READY_TO_CHANGE_VALUE"

const mapDispatchToProps = dispatch => {
  return {
    readyToChangeValue: () => dispatch(readyToChangeValue())
  }
}

so changeValueGenerator would never execute.

This is further complicated by the micro-frontend having no idea (by design) of what namespace it is in. It is defined by the parent component.

My current thought is that the sagas could be wrapped in when being integrated in the parent component. Something like:

import { namespacedSagas } from 'redux-subspace' // possibly a separate library
import { sagas as componentSagas } from '../component'

export default function* sagas() {
  yield [ namespacedSagas(componentSagas, "component")() ]
}

which would somehow un-namespace the action types if they start with "component/". Of course, I have no idea how possible that would be as it does not appear as if the actions are passed through it at all.

My dilemma right now is if accepting your example looks like we are endorsing redux-saga when it wont work in every use case.

Hope this helps. Feel free to hit me up for any more details on this or on our approach to scaling react and redux with micro-frontends.

@jpeyper
Copy link
Collaborator

jpeyper commented Dec 2, 2016

I've had a bit of a think about this, and while initially I thought full support or no support, I've changed my mine.

I would like to see this merged with an big note in the readme about limited support for redux-saga and an issue raised for someone more familiar with it to come along and provide the namespacing feature.

As there are no functional changes (yet) it's not going to affect compatibility with thunks and therefore I would rather people who are using sagas at least get an indication of how they can use this in their projects, and what limitations it currently has.

@mpeyper
Copy link
Contributor

mpeyper commented Dec 2, 2016

Perhaps just a warning under the Caveats would suffice.

@jpeyper
Copy link
Collaborator

jpeyper commented Dec 2, 2016

I feel we might want something a bit more that that, like a supported middleware section.

There is already a section under nauseating for thanks that could be moved there, as well as the caveat that we were assuming the use of thunks.

@jpeyper
Copy link
Collaborator

jpeyper commented Dec 2, 2016

Can't edit comments on my phone,

Nauseating = namespacing

@mpeyper
Copy link
Contributor

mpeyper commented Dec 3, 2016

And thanks = thunks? ;)

I'm ok with your suggestion.

@vivian-farrell
Copy link
Contributor

I think with saga we should document the current limitations as a warning until we figure out exactly how to solve namespace unwrapping. I actually agree with @mpeyper in that the example may confuse people looking for saga support.

@giamir I will try to get the slides from the 'scaling react and redux' presentation up on Monday and link them to the repo. They should provide more context for this library.

@mpeyper
Copy link
Contributor

mpeyper commented Dec 20, 2016

I am closing this as we have raised #18 to attempt to address the sagas compatibility before adding an example for it.

@mpeyper mpeyper closed this Dec 20, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants