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

Variation where components signal up via a context. #4

Merged
merged 1 commit into from
Mar 3, 2016

Conversation

fredcy
Copy link
Contributor

@fredcy fredcy commented Mar 2, 2016

This version copies Peter's and then has the components send indications upward via a new context parameter to each update function.

slorber added a commit that referenced this pull request Mar 3, 2016
Variation where components signal up via a context.
@slorber slorber merged commit a946435 into slorber:master Mar 3, 2016
@slorber
Copy link
Owner

slorber commented Mar 3, 2016

thanks a lot! I'll have to take a closer look later!

Isn't your proposal somehow close to the 2 mailboxes proposal I made here? #2

@slorber
Copy link
Owner

slorber commented Mar 3, 2016

Btw @fredcy passing that context down to NewGif component, doesn't it couple it too much to our app? What if I'd like to reuse that NewGif component inside another application?

@fredcy
Copy link
Contributor Author

fredcy commented Mar 3, 2016

@slorber I'm not comprehending the discussion in #2. It sounds similar to what I did but I'm not fluent in React/Flux/Redux and not able to relate it all.

The context does introduce some coupling but all the component sees is a Signal.Address () which serves as kind of a local outgoing port. The component does not know anything about the data types used by the parent or about how the parent uses the signal that results. I think that makes it reusable. However, any parent using the component has to provide that context parameter and a Signal.Address value to pass to it even if it doesn't care about the resulting signal, so that's a complication. I was rushing -- I might update the code to fix that.

My choice of NewGif for the new top level action label was badly done since that can be conflated with the same-named action in the component.

@fredcy
Copy link
Contributor Author

fredcy commented Mar 3, 2016

I responded to your comment (fredcy@ddb98dd#commitcomment-16469112) inline.

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

Successfully merging this pull request may close these issues.

2 participants