ssorallen / turbo-react

A JavaScript library that transitions between static HTML pages on navigation; no app server required.
https://turbo-react.herokuapp.com/
Apache License 2.0
274 stars 16 forks source link

Considerations for moving this beyond an experiment? #19

Closed tmlayton closed 8 years ago

tmlayton commented 9 years ago

@ssorallen continuation from rails/turbolinks#491

qq99 commented 9 years ago

I think the following should happen:

ssorallen commented 9 years ago
ssorallen commented 9 years ago
qq99 commented 9 years ago

TurboReact currently includes Turbolinks, but I don't think it should

agreed 100% :) also, it'd be very nice if it were available as a rails engine

ssorallen commented 9 years ago

@qq99, I'm familiar with Rails but not with writing Rails engines. I can check that out too.

ssorallen commented 9 years ago

https://github.com/ssorallen/turbo-react/blob/e0048b78d2c915d0517efff1afdf9c6cf7b6ca75/src/reactize.js#L24-L30 should be changed to call the original Turbolinks.replaceNode(...)

I took that suggestion and implemented it in commit https://github.com/ssorallen/turbo-react/commit/2ffd0311ee6c0b1f3fc3be50f8a5914e41fe562f.

nateberkopec commented 9 years ago

What would making TurboReact a Rails engine solve? Happy to take a stab at it but I don't understand the benefit.

qq99 commented 9 years ago

It let's you install it more like:

  1. gem 'turboreact' into your Gemfile
  2. #= require turboreact in your application.js manifest
  3. It's all working and good-to-go

It seems a bit strange to have to use npm to install something in a RoR project

ssorallen commented 9 years ago

:+1: @qq99, if you are familiar with Rails Engines a "turboreact-rails" Gem would be great. I agree using NPM in a Rails project feels awkward.

jbhatab commented 9 years ago

So is this suitable for production? The only thing that threw me off was the caching of react ids. Seems like it could break stuff.

ssorallen commented 9 years ago

@jbhatab TurboReact currently disables the Turbolinks cache because of the problem of caching React IDs.

The library doesn't have automated testing yet, but if it runs into any problems it falls back to letting Turbolinks work like normal. If you are comfortable with document.replaceChild being monkeypatched, feel free to try it out.