Skip to content

Better support for Flux architecture #143

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

Closed
willcosgrove opened this issue Jan 6, 2015 · 31 comments
Closed

Better support for Flux architecture #143

willcosgrove opened this issue Jan 6, 2015 · 31 comments

Comments

@willcosgrove
Copy link

I've been working on a Rails app using React and Flux, and I've run into a few walls. I thought I would check with the react-rails community before I put my pioneer hat on.

So let me run through really quickly how I think I would like things to work, and maybe someone can let me know if I'm way off.

Serialization

This is not really a concern of react-rails, but it's important to my overall plan

I want all of my records for any given page serialized like so

{
  posts: [
    {
      id: 1
      title: "There's always money in the banana stand!",
      body: "...",
      author_id: 2,
      comment_ids: [1]
    }
  ],
  comments: [
    {
      id: 1
      author_id: 1,
      body: "@gob you did mail that insurance letter, right?"
    }
  ],
  users: [
    {
      id: 1,
      name: "Michael Bluth"
    }, {
      id: 2,
      name: "George Bluth Sr."
    }
  ]
}

In other words: I want each model serialized in an array on the root object. This will facilitate loading the models into their respective Flux stores.

Stores

I want all of my data loaded into stores, and then I want my react components to be passed those stores through properties. So rather than passing data directly into the component's properties, I want to tell react-rails which stores each component will need to access. My first thought was to abandon the react-rails UJS and just initialize my components the old fashioned way, but I realized it would be a pretty trivial thing to add into the UJS. This is what sparked the writing of this issue: I didn't know if this sounded like a good idea to anyone else, as I would be more than happy to submit a PR once it's all ready.

Server Side Rendering

I'm not sure at the moment how to get this all working with server side rendering. It's not a huge concern to me, but I realize that if this plan were to move forward at all with react-rails, there would need to be a way to get it to work with server side rendering.

My plan for getting the data into the stores was just going to be to pass them into gon and then have a script grab everything out of gon, and throw them into the stores. But gon wouldn't be available for server rendering, so I would need to work around that. But I don't see that as being too difficult of an issue.

Conclusion

If this sounds dumb, it's probably because it is. I haven't actually tried to do any of the following yet, but it's my tentative action plan. I would love to hear if anyone else has been successfully using the Flux architecture with react-rails, and what approach was taken. I would also be happy to begin working on a better integration for Flux and react-rails if anyone is interested in this approach (or a better one 😄).

@csm123
Copy link

csm123 commented Jan 6, 2015

React-rails worked well for me with Fluxxor, one of the Flux-inspired libraries out there. While not perfect, here is a play by play of the implementation.

Some Flux-based libraries like Reflux are tested for server-side implementation so they don't leak. Fluxxor has an issue stating that it may or may not be ready for server-side. Personally, I pre-load the components with a server-generated JSON string, but don't actually server-render them yet.

With no set-in-stone Flux standard outside of Facebook's example/recommendation, it's difficult to see react-rails hardcoding one. I agree, though, that it should provide a way to launch JS on the server that can pass a Flux store (initiated however you want to initiate it) to the component.

@willcosgrove
Copy link
Author

Yeah I agree, it probably isn't going to fly; jamming a particular flavor of flux down everyone's throats. But I don't think this necessarily needs react-rails to know which flux implementation you're using. It just needs to pass a store into a component. It doesn't care what API the store has, just that one exists.

But, if we were to pick a particular implementation of Flux (or roll our own), then react-rails could do a lot more. It could be in charge of taking the serialized data in the controller and loading them into the stores (a bit of a stretch, but I can dream), then passing those stores to the components.

But I think most people will agree that keeping react-rails as simple as possible is the right route. And to that end, I think just allowing you to pass a javascript object into a component as a property would be a great first step.

@rmosolgo
Copy link
Member

rmosolgo commented Jan 7, 2015

I'm definitely in the keep-it-simple boat. It seems like everybody has his or her own preferred spin on Flux!

@uberllama
Copy link
Contributor

I've also been pre-populating with server-generated JSON rather than actually rendering on the server. My primary motivation is removing the front end from Rails in possible anticipation of full extraction into a SPA. So far so good. And I use the rails-assets workflow for now to bring Reflux into global.

@vegetabill
Copy link
Contributor

Does anyone out there have a sensible way to populate stores with JSON data during a server-side render? Right now I'm passing in a prop to my component which during its componentWillMount calls something like:

store.initializeWith(JSON.parse(this.props.myJsonDataStringFromRails))

But this is backwards according to flux (and common sense). The component shouldn't send data into the stores. Any ideas?

@willcosgrove
Copy link
Author

@RearAdmiral I'm with you. Right now, without server rendering, I'm sending my JSON through gon. Then I have this

# initialize_stores.coffee
Actions.initialDataLoad(gon.records)

This fires an initialDataLoad action, which my stores listen for:

# user_store.coffee
class UserStore
  onInitialDataLoad: (data) ->
    if data.users?
      @records = data.users

I feel like there should be some way to seed some kind of initial data into the server rendering runtime, then you could have a store_initializer file that loads right after your flux/stores/actions which calls that initialDataLoad action.

I don't know how I would handle this if I was maintaining react-rails. You could either buddy up with Gon, as it does a pretty good job at the simple task it performs. And if the react-rails gem detects Gon, it will pass through the gon variable into the server rendering runtime. OR, you could reimplement the same basic functionality as Gon, and use that.

I'm not crazy about either approach though. Like I said above, I think the goal is to try and keep this as simple as possible. This task might be better suited to a currently nonexistent flux-rails gem... Let's build it! We can do it in two weeks!
3wo2iwh

@csm123
Copy link

csm123 commented Jan 17, 2015

@RearAdmiral Where does your store live and how do you ensure it's loaded on the server along with your component?

@bowd
Copy link
Contributor

bowd commented Jan 18, 2015

I've been also pondering something related to this. Right now I actually have duplicate date to keep things sane, i.e.

  • In a json script tag I dump all the store state on page load, this gets injected in the stores as initial state when my flux "boots".
  • Segments of data from the above dump is also passed as props to the toplevel components (server side rendered).

We have a convention that top level components use props as initial state (that would normally come from one or more stores). There's basically no way for top level components to receive new props anyway so they are in fact a sort of "initial state".

Unrelated but we also enforce that only top level component get data from stores and pass down snippets to lower level components as props.

Now this works great, but I would like to get rid of that data duplication. The only way I can see that happening is to be able to boot up my stores in the server side javascript context. The way I imagine this happening is making sure a request gets one JS context, and you have a way of executing some pseudo arbitrary JS in it, to setup your store states. After that all component renders execute in that JS context.

I dunno if that makes sense but an API for that would make it easy to incorporate any flux paradigm you want.

@csm123
Copy link

csm123 commented Jan 18, 2015

^ Completely agreed that such an API would solve this issue and make
isomorphic with flux much easier

On Sunday, January 18, 2015, bogdan-dumitru [email protected]
wrote:

I've been also pondering something related to this. Right now I actually
have duplicate date to keep things sane, i.e.

  • In a json script tag I dump all the store state on page load, this
    gets injected in the stores as initial state when my flux "boots".
  • Segments of data from the above dump is also passed as props to the
    toplevel components (server side rendered).

We have a convention that top level components use props as initial state
(that would normally come from one or more stores). There's basically no
way for top level components to receive new props anyway so they are in
fact a sort of "initial state".

Unrelated but we also enforce that only top level component get data from
stores and pass down snippets to lower level components as props.

Now this works great, but I would like to get rid of that data
duplication. The only way I can see that happening is to be able to boot up
my stores in the server side javascript context. The way I imagine this
happening is making sure a request gets one JS context, and you have a way
of executing some pseudo arbitrary JS in it, to setup your store states.
After that all component renders execute in that JS context.

I dunno if that makes sense but an API for that would make it easy to
incorporate any flux paradigm you want.


Reply to this email directly or view it on GitHub
#143 (comment).

@xionon
Copy link
Contributor

xionon commented Jan 19, 2015

I am skeptical that anything that includes some sort of Flux library and runs arbitrary code per-request will have decent performance, given the current JS-from-Rails ecosystem. If it could be proven that response time wasn't significantly affected by this, I'd definitely change my mind.

IMO, react-rails should focus on providing up-to-date versions of React for the asset pipeline, prerendering based on a given set of properties, and Rails-y helpers (methods, generators, and installers). Anything beyond that should be provided by external libraries.

@vegetabill
Copy link
Contributor

@xionon I understand your concern, but even thought we're talking about Flux, this actually applies more generally. Right now the only way to hand off data from Rails is using a component properties. I think many people, including me, are doing what @bogdan-dumitru mentions. There is nothing terribly wrong with this, but it's pretty limiting and, as the OP brought this up, it doesn't align with the officially recommended React app architecture, Flux.

If the react_component helper took in an optional block that let you feed in page-specific js needed for prerendering, I think that would satisfy the need In the docs it could be put into context as a risk for perf bottlenecks but the pre-seeding the stores we're talking about here would not be.

It might be able to be a plugin for this gem, but unless I misunderstand how prerendering is implemented, it could not be a standalone gem.

@bowd
Copy link
Contributor

bowd commented Jan 19, 2015

@RearAdmiral just to make this clear, adding extra options to render_component is a good first step... I dunno about you guys, but I personally render a few (more than 1 less than 8) top-level components per request, and that means I would initialise the stores when rendering each component. I still think the ability to isolate get a single JS context per request is crucial.

And @xionon, the perf argument for me seams a bit weird, all of us are including a big components.js or similar which should contain ReactClass definitions, but that's on good faith_. It's not like something's stoping me for having arbitrary javascript there, I'm only stopped from controlling that on a per request basis. Which means that in sense it's just limiting under the guise of "protecting devs from themselves".

Anyway if we get no resolution on this until 2/1 (magic release date), I will play around with this in a fork and see what happens. Do you have any benchmarking scenario to test against for baseline performance indication internally?

@xionon
Copy link
Contributor

xionon commented Jan 20, 2015

I really want to help you solve this problem, it seems clear that it would help at least some people. I'm anxious, because once an API is accepted, it's very hard to get it out or change it, so I really think it's important to try to get it as close to "right" as possible.

I still think the ability to isolate get a single JS context per request is crucial.

My gut feeling is that this will be a pretty complex undertaking. I'd love to see code that proves me wrong, though. The renderer code definitely needs some love!

Do you have any benchmarking scenario to test against for baseline performance indication internally?

I do not have anything automated, I will try to build one.

What hooks into the renderer would be necessary to make this a plugin, rather than a core feature?

@bowd
Copy link
Contributor

bowd commented Jan 21, 2015

@xionon great to have you on board, I also believe this may be a pretty big thing and it's important to get it right and not to opinionated. I might have sounded a bit harsh because it felt like you were dismissing the whole thing.

I'm actually playing around through the code getting a better feel of how this would look like, I'll get back to you guys with my findings, but most probably post weekend.

@uberllama
Copy link
Contributor

My biggest concerns, as others have voiced, is implementation. From my initial projects, Reflux feels right. But some people may want to use Fluxxor, and others may want a full blown Flux pattern. Whereas React is a baseline lib, none of the Flux implementations are. So how do you create a generic enough pipeline in this gem that can service all of those.

@bowd
Copy link
Contributor

bowd commented Jan 21, 2015

@uberllama well like I mentioned at some point above it feels to me that what fits most people's needs is the ability to:

  • Have a unique JS context per request
  • Have the ability to execute some "setup" JS in that context in before starting to render components. That setup code will be based on per-request data. I'm imagining a user defined lambda in the configs + a method on React::Renderer that calls the lambda (with some params passed into the function) and evals the resulting JS in the context that gets checked out from the pool on this request.

This would be used basically for initialising your stores with data so that the components can use them. And in general should be a simple function. In my case something like Flux.startForServerSide(<json dump of hash>) (example name, wouldn't wire up dispatchers and stuff just inject initial data in the stores).

  • All further component rendering happens in the one checked out context.
  • There's also some teardown javascript (same lambda config), that gets execute after the request in order to checkin the renderer instance back into the pool.

This could be something like Flux.flushStores()


This feels right to me personally, and generic enough to fit any react need, but like you said it might not, so I'm open to figuring out what would work, but I feel like this is the right direction.

@goatslacker
Copy link

@willcosgrove sorry to bring this issue back from the dead, did alt solve what you were trying to do originally?

@vegetabill
Copy link
Contributor

On the contrary, I'm still very interested in this issue. +1 to @bogdan-dumitru 's suggested implementation. Is this something that the maintainers want to implement or would they prefer a pull request?

@xionon
Copy link
Contributor

xionon commented Feb 25, 2015

I think the best way to proceed is to make the default renderer a
configuration option, so that this could be implemented as a separate gem,
e.g. config.react.renderer = React::DefaultRenderer or
config.react.renderer = FluxExperimentalRenderer

On Wed, Feb 25, 2015 at 3:12 PM, Bill DePhillips [email protected]
wrote:

On the contrary, I'm still very interested in this issue. +1 to
@bogdan-dumitru https://github.com/bogdan-dumitru 's suggested
implementation. Is this something that the maintainers want to implement or
would they prefer a pull request?


Reply to this email directly or view it on GitHub
#143 (comment).

@vegetabill
Copy link
Contributor

👍 to the extension point approach

@vegetabill
Copy link
Contributor

Has anyone spent any time on this? If not, I might try to work on something simple this weekend to get y'all's feedback on.

@dmitry
Copy link

dmitry commented Mar 14, 2015

@kanfet
Copy link

kanfet commented Mar 14, 2015

Have you read this article: http://aspiringwebdev.com/react-js-and-flux-in-rails-a-complete-example/?

There are nothing about server-side rendering.

@bowd
Copy link
Contributor

bowd commented Mar 14, 2015

@RearAdmiral I keep trying to find the time but proper work has been a bit killer. That being said if you put anything out there I'll be sure to give feedback for what it's worth. Maybe it might even push me to find the time to contribute :)

@dmitry
Copy link

dmitry commented Mar 14, 2015

In one of the projects I've used https://github.com/mroderick/PubSubJS with react-rails. PubSub can be an alternative to Flux for a small projects.

@uberllama
Copy link
Contributor

Yep, and we're using custom stores with backbone events. It would definitely be a mistake IMO to couple this lib specifically with flux. That doesn't mean there couldn't be a complimentary add on lib.

@sgwilym
Copy link

sgwilym commented Mar 20, 2015

+1 for this. Been wrangling with react-rails, Reflux and Turbolinks in particular the last two days and finally have something that (awkwardly) works. I think @bogdan-dumitru's idea of a generic implementation would be a great fit if you don't want to marry this project to any specific way of doing flux.

@vegetabill
Copy link
Contributor

I played around with a simple implementation last weekend. I hope to post a really rough PR just for feedback purposes this weekend. It's not tied to flux, but more just provides access to a react-rails js context.

@vegetabill
Copy link
Contributor

I made something very simple that fits my use case. Please check it out and see if it would work with your application as well.

vegetabill@201ec43

This was referenced Apr 28, 2015
@rmosolgo
Copy link
Member

If you're still in the market for new ways of server side rendering, I'd love your review of #253!

@rmosolgo
Copy link
Member

There are a lot of new APIs for customizing JS, I hope some of them are useful for you:

If they aren't, please open an issue to describe what's missing!

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