The future of turbolinks

Hello everyone!

Foreman uses a very old version of turbolinks (turbolinks-classic) which is not maintained and won’t be supported anymore.

Background

Turbolinks mimics a SPA behavior by body-swapping when the head element remain the same ( by observing tracked scripts and other tags on it). Turblinks adds events and listeners in order to fulfill a SPA look like

Suggestions:

1, Upgrading Turbolinks
2, Switching to an alternative solution
3. Dropping Turbolinks with no alternative

Do we really need turbolinks?

React and especially react-router don’t work well with Turbolinks, mostly because both alter the browser’shistory object, and both addressing the same set of problems but in a way different approach, we had and still have workarounds and hacks in order to make them somehow to work together.
In addition, turbolinks adds lots of events that we barely use due to moving to react, and works behind the scene, like a black box, although we do have a brand new client-routing mechanism and I prefer a small implementation to control legacy transitions together with our routing mechanism in one place - it would be more stable for the long term; maintainable and fully testable,

POC

I’ve created a small POC that mimics the body-swapping mechanism of turbolinks but controlled by our client routing mechanism.

Demonstration:

POC

Turbolinks

3 Likes

I believe we are again on the question:

When and if we get to the full SPA?

If we are going to get rid of our implementation and it will be only a bridge for us for short period of time I am for going with our own implementation as it seems to be easier.
Having our implementation around for longer time I believe we will suffer from supporting it as such solution tend to grow. Opposed to Turbolinks we suffer only once switching to it - it is pretty stable, so once we switch, I would expect it being far less overhead with supporting it.

I am afraid there is not a huge push for SPA here, so it will still take time to switch to it fully.

1 Like

Well, I don’t think in this case moving to a full SPA is important, this POC basically allows us to control much better transitions between pages in one place, no matter if it’s a react page or a legacy one.

Having our implementation around for longer time I believe we will suffer from supporting it as such solution tend to grow.

on the contrary - over time more pages will be move to react, new content should be based on react, so with an alternative solution, its use will be decreased over time

Opposed to Turbolinks we suffer only once switching to it - it is pretty stable

Not accurate, we had so many bugs that solved by workarounds and hacks

I am afraid there is not a huge push for SPA here, so it will still take time to switch to it fully.

It’s not a push for SPA, and basically this PR increase performance for legacy views transitions and keep the body-swapping mechanism, regardless of this issue, handling both approaches for a long time is the worst option.

Upgrading it would enforce a deep legacy rewriting - because of lots of breaking changes, and this very outdated library - jquery-turbolinks which we use ATM and it won’t work anymore, while I’m not sure if we will use most of its features

This is also the case for katello’s legacy angular application. There have been many problems with turbolinks over the years.

I support option 2 or option 3. To me, the benefit of turbolinks doesn’t outweigh the cost of fixing bugs around it. It is, and always has been, a hack and if we’re going to use a hack I’d rather use one that we control ourselves. I personally think option 3 is the best.

3 Likes

Experience tells us that temporary solutions always last longer than you think.

My interpretation of what @ezr-ondrej said was that he was worried there’s not a push for SPA. It means we won’t get rid of legacy. Maybe that just my perception of the current situation.

Personally I’d like to see a clear choice because trying to support both appears to introduce more issues and nobody is happy with it.

It may be good to provide some docker images with current version, no turbolinks, react poc alternative. That way people can play with it and feel the difference. Also some timing info would be good, videos are good for illustration, but may not be accurate. If any of alternative here introduces a worse user experience, I’d be concerned about accepting it with the argument “long term we’ll migrate”, for reasons @ezr-ondrej and @ekohl mentioned.

4 Likes

Exactly. It will also be more interesting when there are 100s or 1000s of hosts.

Great PR @amirfefer, tested and imo it brings in most cases a better user experience,
though there are still some edge-cases that worth to be discussed.

Along the history of Foreman there seem to be so many tweaks to get Turbolinks to work…
and sometimes it is frustrating that we can’t really control it.

speaking of history, I am facing a critical bug right now where Turbolinks seem to interpolate the window.history and window.location and when I try to access those from react-router i get the wrong location, for example on entity pages such as architectures, if I will go to an architectures's edit page, the location that I will see will be only architecture -> see the PR

and when I tested it with your POC, it looks that this problem is gone. probably because turbolinks didn’t touched it.

What I am trying to say is that we can’t have 2 different mechanism that alters the window history or location objects… it already leads to unexpected bugs since we have no control on turbolinks.

2 Likes

@ezr-ondrej, did you succeed creating a turbolinks-less production with docker?

See Fixes #28325 - fix react-router & Turbolinks warning by Ron-Lavi · Pull Request #7193 · theforeman/foreman · GitHub :grin:

I just git cloned and git pushed to my repo which auto builds - so here you go:

just change the image section in the docker-compose file to

quay.io/ohadlevy/foreman:amirfefer-drop-turbolinks
4 Likes

Thank you @ohadlevy!

I’m in for dropping Turbolinks. The JS evaluation on every request doesn’t feel that bad. I’m not using katello though. But let’s try it out.

2 Likes

I’ll add my 2 cents, lets remove it, its an easy fix to bring it back, the value we get out of turbolinks is reducing over time, and instead of trying to maintain the hack, I hope we could focus on improving the non-ideal parts instead.

4 Likes

I dont think it matters a lot for katello, as once you load katello, there arent full page reloads within the angular code base? (I think?)

1 Like

I agree. The first step of dropping turbolinks is easy to achieve and will unblock the rails 6 upgrade. After that, we can decide to either do nothing, bring in a newer version of turbolinks, roll our own front-end solution or maybe even find a different library that fits us better.
We can also see how much of an impact it will have on performance in a production env (where apache serves all the compressed static files) with the nightlies before 2.0 is out and if we have to fix it or if it isn’t as bad as we thought (or maybe improve in 2.1 if needed).

This is an important point. in many pages the cause for slowness isn’t the page load itself but rather waiting on the server to get some data from the DB or on the front-end code doing a lot of work (e.g. select2 initialization). Both of these cases don’t benefit from turbolinks and won’t be improved even if we find a different solution.

Exactly. We disable turbolinks on all of our pages.

Here’s a PR for those interested: https://github.com/theforeman/foreman/pull/7236

1 Like