Transition cache #286

Merged
merged 9 commits into from Jan 7, 2014

Conversation

Projects
None yet
3 participants
@smidwap
Contributor

smidwap commented Oct 7, 2013

This is a continuation of rails#248. I rebased master and took a completely different approach that I believe is bug free.

I changed pageCache to use the url as a key. The state object now contains a url attribute instead of a position. This solves the issues brought up by @dhh.

However, this does introduce the problem of how to constrain the cache to a certain size...I haven't solved this problem yet but I am sure it can be solved.

A few other points

  1. reflectNewUrl has been moved out of the xhr callback to make the code a bit cleaner. It was moved into the callback in rails#231.
  2. I removed the xhr.abort callback that calls rememberCurrentUrl. I did not see a regression from rails#162. I believe this is due to using url's as keys.
  3. I purposely ensured that recallScrollPosition was not called a second time if a cached copy is shown. @dhh's gist https://gist.github.com/dhh/8460cd271cc57dc361d0 called recallScrollPosition a second time, but that creates a scroll jump if the user starts scrolls after seeing a cached copy.
  4. I don't have "opt out" built-in yet. That's trivial to build in once we decide what to do with this PR

I'd appreciate a second round of testing. Thanks!

body: document.body,
title: document.title,
positionY: window.pageYOffset,
positionX: window.pageXOffset
- constrainPageCacheTo cacheSize
+ #constrainPageCacheTo cacheSize

This comment has been minimized.

Show comment Hide comment
@dhh

dhh Oct 8, 2013

Contributor

Why is this commented out?

@dhh

dhh Oct 8, 2013

Contributor

Why is this commented out?

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 8, 2013

Contributor

I'm still working on a solution to constrain the cache size. It was easy when the cache key was the position. With the change to url as the cache key it's not so simple...or maybe it is, but I haven't figured it out yet.

@smidwap

smidwap Oct 8, 2013

Contributor

I'm still working on a solution to constrain the cache size. It was easy when the cache key was the position. With the change to url as the cache key it's not so simple...or maybe it is, but I haven't figured it out yet.

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 8, 2013

Contributor

Ok, I have the constrainPageCacheTo function back in place. It's not beautiful, but it works according to my testing.

Contributor

smidwap commented Oct 8, 2013

Ok, I have the constrainPageCacheTo function back in place. It's not beautiful, but it works according to my testing.

@reed

This comment has been minimized.

Show comment Hide comment
@reed

reed Oct 14, 2013

Collaborator

Now that I've tried it out, I feel the need to reiterate my concern for this. Turbolinks offers an actual increase in speed through the reduction of network requests and asset processing. The beneficial side effect of the implementation is a smoother experience for users navigating the site. The feature being proposed has only one focus: increasing the speed of the users' perception of a changing page. Another way to put it would be: tricking the user into thinking the site is faster than it really is. This might sound fine, but not when you consider the costs:

More Processing
When you click a link for a page that's in the cache, these are the events that fire:

# fetch from cache
page:before-change
page:change
page:update
page:restore
# fetch from server
page:fetch
page:receive
page:change
page:update
page:load

Not only does this fire every event turbolinks has, it fires page:change and page:update twice. So all the JS you have bound to these events will be executed twice. That certainly doesn't help performance.

Not to mention: what happens when the handler for the first page:change isn't finished running when the page changes a second time? Or if your page:change handler sets a timeout?

Degraded User Experience
This is what I described in the last thread. It stems from this feature's assumption that a cached page (which is cached at the end of a page's lifespan) is always going to be virtually identical to the new copy being fetched from the server. Maybe this is true for Basecamp, but it's going to be a problem for many sites. The assumption ignores two factors: Time (the page returned by the server might be pretty different from a copy that may have been cached an hour ago) and DOM manipulation (in my testing, it took only 4 clicks before I came across this problem).

Simply dismissing this concern by saying it's not an issue in your apps is reckless. All conceivable use cases should be considered.

It should say something that a feature whose entire purpose is a better user experience could potentially result in a worse user experience.


In my opinion, this is Turbolinks getting too cute for it's own good. I just don't see the benefit outweighing the potential drawbacks.

I realize that my concerns will probably be ignored on this, and that's fine. Just be sure to include a global opt-out so that I can use it for all my projects.

Collaborator

reed commented Oct 14, 2013

Now that I've tried it out, I feel the need to reiterate my concern for this. Turbolinks offers an actual increase in speed through the reduction of network requests and asset processing. The beneficial side effect of the implementation is a smoother experience for users navigating the site. The feature being proposed has only one focus: increasing the speed of the users' perception of a changing page. Another way to put it would be: tricking the user into thinking the site is faster than it really is. This might sound fine, but not when you consider the costs:

More Processing
When you click a link for a page that's in the cache, these are the events that fire:

# fetch from cache
page:before-change
page:change
page:update
page:restore
# fetch from server
page:fetch
page:receive
page:change
page:update
page:load

Not only does this fire every event turbolinks has, it fires page:change and page:update twice. So all the JS you have bound to these events will be executed twice. That certainly doesn't help performance.

Not to mention: what happens when the handler for the first page:change isn't finished running when the page changes a second time? Or if your page:change handler sets a timeout?

Degraded User Experience
This is what I described in the last thread. It stems from this feature's assumption that a cached page (which is cached at the end of a page's lifespan) is always going to be virtually identical to the new copy being fetched from the server. Maybe this is true for Basecamp, but it's going to be a problem for many sites. The assumption ignores two factors: Time (the page returned by the server might be pretty different from a copy that may have been cached an hour ago) and DOM manipulation (in my testing, it took only 4 clicks before I came across this problem).

Simply dismissing this concern by saying it's not an issue in your apps is reckless. All conceivable use cases should be considered.

It should say something that a feature whose entire purpose is a better user experience could potentially result in a worse user experience.


In my opinion, this is Turbolinks getting too cute for it's own good. I just don't see the benefit outweighing the potential drawbacks.

I realize that my concerns will probably be ignored on this, and that's fine. Just be sure to include a global opt-out so that I can use it for all my projects.

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 14, 2013

Contributor

Nick, would it be possible to upload a video of the DOM manipulation problem in the context of your app? That would be helpful to understand the issue for apps other than my own (where DOM manipulation is not a big deal).

Contributor

smidwap commented Oct 14, 2013

Nick, would it be possible to upload a video of the DOM manipulation problem in the context of your app? That would be helpful to understand the issue for apps other than my own (where DOM manipulation is not a big deal).

@reed

This comment has been minimized.

Show comment Hide comment
@reed

reed Oct 14, 2013

Collaborator

It's not a public site, so I'm not going to post video of it. But the issues are generic enough to where describing them should be enough.

The first involved tabbed content. After visiting the page, you click on a second tab (other than the default) to view that tab's content. Then you click a link to go to another page. Now, if you come back to this page, you'll see the second tab active briefly before it suddenly switches back to the default tab.

Another issue involved graphs. I have a dashboard page with multiple graphs. Their initialization involves animation. If you click back to this page, you see the outdated graphs briefly before they disappear and then animate back to how they were.

These aren't catastrophic events, but they create a confusing effect.

Collaborator

reed commented Oct 14, 2013

It's not a public site, so I'm not going to post video of it. But the issues are generic enough to where describing them should be enough.

The first involved tabbed content. After visiting the page, you click on a second tab (other than the default) to view that tab's content. Then you click a link to go to another page. Now, if you come back to this page, you'll see the second tab active briefly before it suddenly switches back to the default tab.

Another issue involved graphs. I have a dashboard page with multiple graphs. Their initialization involves animation. If you click back to this page, you see the outdated graphs briefly before they disappear and then animate back to how they were.

These aren't catastrophic events, but they create a confusing effect.

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 14, 2013

Contributor

Nick, you bring up some good points, but I believe you are misguided in thinking the idea should be tossed. This feature has worked really well for my app (www.lesson.ly) and Basecamp. Where it works, it does a lot for increasing perceived site speed. It would surprise me if a hefty percentage of apps running Turbolinks wouldn't benefit greatly.

Instead, I would try to see how we can meet half way. I know @dhh needs to weigh in, but is there not room for a global opt-in/out and page-by-page opt-in/out? Is this not something worthy of releasing, getting feedback, and deciding upon once more than a few of us have weighed in?

Contributor

smidwap commented Oct 14, 2013

Nick, you bring up some good points, but I believe you are misguided in thinking the idea should be tossed. This feature has worked really well for my app (www.lesson.ly) and Basecamp. Where it works, it does a lot for increasing perceived site speed. It would surprise me if a hefty percentage of apps running Turbolinks wouldn't benefit greatly.

Instead, I would try to see how we can meet half way. I know @dhh needs to weigh in, but is there not room for a global opt-in/out and page-by-page opt-in/out? Is this not something worthy of releasing, getting feedback, and deciding upon once more than a few of us have weighed in?

@dhh

This comment has been minimized.

Show comment Hide comment
@dhh

dhh Oct 14, 2013

Contributor

I think having this on by default but with both local and global opt-outs would be great.

On Oct 14, 2013, at 20:27, Matt De Leon notifications@github.com wrote:

Nick, you bring up some good points, but I believe you are misguided in thinking the idea should be tossed. This feature has worked really well for my app (www.lesson.ly) and Basecamp. Where it works, it does a lot for increasing perceived site speed. It would surprise me if a hefty percentage of apps running Turbolinks wouldn't benefit greatly.

Instead, I would try to see how we can meet half way. I know @dhh needs to weigh in, but is there not room for a global opt-in/out and page-by-page opt-in/out? Is this not something worthy of releasing, getting feedback, and deciding upon once more than a few of us have weighed in?


Reply to this email directly or view it on GitHub.

Contributor

dhh commented Oct 14, 2013

I think having this on by default but with both local and global opt-outs would be great.

On Oct 14, 2013, at 20:27, Matt De Leon notifications@github.com wrote:

Nick, you bring up some good points, but I believe you are misguided in thinking the idea should be tossed. This feature has worked really well for my app (www.lesson.ly) and Basecamp. Where it works, it does a lot for increasing perceived site speed. It would surprise me if a hefty percentage of apps running Turbolinks wouldn't benefit greatly.

Instead, I would try to see how we can meet half way. I know @dhh needs to weigh in, but is there not room for a global opt-in/out and page-by-page opt-in/out? Is this not something worthy of releasing, getting feedback, and deciding upon once more than a few of us have weighed in?


Reply to this email directly or view it on GitHub.

@reed

This comment has been minimized.

Show comment Hide comment
@reed

reed Oct 14, 2013

Collaborator

If it's implemented like data-no-turbolink, the local and global opt-outs would be one and the same.

Collaborator

reed commented Oct 14, 2013

If it's implemented like data-no-turbolink, the local and global opt-outs would be one and the same.

@dhh

This comment has been minimized.

Show comment Hide comment
@dhh

dhh Oct 14, 2013

Contributor

👍

On Oct 15, 2013, at 7:24 AM, Nick Reed notifications@github.com wrote:

If it's implemented like data-no-turbolink, the local and global opt-outs would be one and the same.


Reply to this email directly or view it on GitHub.

Contributor

dhh commented Oct 14, 2013

👍

On Oct 15, 2013, at 7:24 AM, Nick Reed notifications@github.com wrote:

If it's implemented like data-no-turbolink, the local and global opt-outs would be one and the same.


Reply to this email directly or view it on GitHub.

@reed

This comment has been minimized.

Show comment Hide comment
@reed

reed Oct 14, 2013

Collaborator

Are we sure indexing the cache by url is a good idea?

Let's say after navigating around an app for a while, your browser history looks like this:

1. Page A # START
2. Page B
3. Page A
4. Page C
5. Page A
6. Page D # CURRENT

Now, if you start hitting the back button, each time you get to page A, you're going to see what it looked like at state 5. Shouldn't the page appear the same as it did at that point in history?

What's more, this would now allow turbolinks to span across full page loads. Consider this:

1. Page A # START
2. Page B
3. Page A
4. Page C # <- Full page load (turbolinks re-initialized)
5. Page A
6. Page D # CURRENT

Now if you go back, it will fetch from the cache at states 3 and 1 even though those states weren't cached in the same session. Even if you like that behavior, it's still a logic error, isn't it?

Thoughts?

Collaborator

reed commented Oct 14, 2013

Are we sure indexing the cache by url is a good idea?

Let's say after navigating around an app for a while, your browser history looks like this:

1. Page A # START
2. Page B
3. Page A
4. Page C
5. Page A
6. Page D # CURRENT

Now, if you start hitting the back button, each time you get to page A, you're going to see what it looked like at state 5. Shouldn't the page appear the same as it did at that point in history?

What's more, this would now allow turbolinks to span across full page loads. Consider this:

1. Page A # START
2. Page B
3. Page A
4. Page C # <- Full page load (turbolinks re-initialized)
5. Page A
6. Page D # CURRENT

Now if you go back, it will fetch from the cache at states 3 and 1 even though those states weren't cached in the same session. Even if you like that behavior, it's still a logic error, isn't it?

Thoughts?

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 21, 2013

Contributor

I added an opt-out. I think all that's left is adding documentation.

re: back/forward behavior. I had the same question when changing the cache to be by url, but I can't see why it's worse than the default browser behavior. The behavior mimics Basecamp's caching. I can't imagine a time when displaying a newer state while using the back button would be a bad idea.

Contributor

smidwap commented Oct 21, 2013

I added an opt-out. I think all that's left is adding documentation.

re: back/forward behavior. I had the same question when changing the cache to be by url, but I can't see why it's worse than the default browser behavior. The behavior mimics Basecamp's caching. I can't imagine a time when displaying a newer state while using the back button would be a bad idea.

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 21, 2013

Contributor

Btw, the opt-out is including data-no-transition-cache anywhere in the html doc. A global opt-out would mean adding that to the body tag in a layout.

Contributor

smidwap commented Oct 21, 2013

Btw, the opt-out is including data-no-transition-cache anywhere in the html doc. A global opt-out would mean adding that to the body tag in a layout.

@reed

This comment has been minimized.

Show comment Hide comment
@reed

reed Oct 21, 2013

Collaborator
  1. You haven't eased my mind about caching by url. In fact, you admitted that it will result in behavior that differs from the default browser behavior. Maintaining as much of the default browser behavior as possible is a big part of what this library tries to accomplish.
  2. Your opt-out isn't implemented like data-no-turbolink. You're tying it to the cached page instead of the clicked link.
  3. I still don't think the term transition cache makes any sense. Makes it sound like we're keeping a cache full of transitions.
Collaborator

reed commented Oct 21, 2013

  1. You haven't eased my mind about caching by url. In fact, you admitted that it will result in behavior that differs from the default browser behavior. Maintaining as much of the default browser behavior as possible is a big part of what this library tries to accomplish.
  2. Your opt-out isn't implemented like data-no-turbolink. You're tying it to the cached page instead of the clicked link.
  3. I still don't think the term transition cache makes any sense. Makes it sound like we're keeping a cache full of transitions.
@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 22, 2013

Contributor

re: opt-out. I believe attaching it to the clicked link would be a bad idea. Here's why: you want to opt-out a page, not a link. Let's say I want to opt-out my /posts index page. I would need to search for every link to the /posts page and add data-no-transition-cache.

re: caching by url. The point of Turbolinks is to change the browser's default behavior. I wouldn't consider Turbolinks' purpose to "maintain as much default behavior as possible" but rather "depart from default behavior when the benefits are greater than the costs." In this case, I think the cost of departing from default behavior is tiny (non-existant?). We've already debated the benefits.

re: transition cache name. The name seems ok (we discussed it here: rails#248 (comment)), but maybe slightly non-revealing. Lengthening the name to page transition cache might make more sense. What do you think?

Contributor

smidwap commented Oct 22, 2013

re: opt-out. I believe attaching it to the clicked link would be a bad idea. Here's why: you want to opt-out a page, not a link. Let's say I want to opt-out my /posts index page. I would need to search for every link to the /posts page and add data-no-transition-cache.

re: caching by url. The point of Turbolinks is to change the browser's default behavior. I wouldn't consider Turbolinks' purpose to "maintain as much default behavior as possible" but rather "depart from default behavior when the benefits are greater than the costs." In this case, I think the cost of departing from default behavior is tiny (non-existant?). We've already debated the benefits.

re: transition cache name. The name seems ok (we discussed it here: rails#248 (comment)), but maybe slightly non-revealing. Lengthening the name to page transition cache might make more sense. What do you think?

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Oct 23, 2013

Contributor

What about just naming it "cache" or "page cache"? The code already makes
use of the term caching for the purposes of back/forward navigation, but
I'm not sure Rails developers currently see turbolinks as caching anything.

Then we can make the opt out "data-no-turbolinks-cache" or maybe
"data-no-page-cache"

Maybe this is a bit confusing because all pages still get cached for
history nav, but I think it would make sense for people using turbolinks
On Oct 22, 2013 5:59 AM, "Nick Reed" notifications@github.com wrote:

You haven't eased my mind about caching by url. In fact, you admitted
that it will result in behavior that differs from the default browser
behavior. Maintaining as much of the default browser behavior as possible
is a big part of what this library tries to accomplish.
2.

Your opt-out isn't implemented like data-no-turbolink. You're tying it
to the cached page instead of the clicked link.
3.

I still don't think the term transition cache makes any sense. Makes
it sound like we're keeping a cache full of transitions.


Reply to this email directly or view it on GitHubhttps://github.com/rails/turbolinks/pull/286#issuecomment-26764343
.

Contributor

smidwap commented Oct 23, 2013

What about just naming it "cache" or "page cache"? The code already makes
use of the term caching for the purposes of back/forward navigation, but
I'm not sure Rails developers currently see turbolinks as caching anything.

Then we can make the opt out "data-no-turbolinks-cache" or maybe
"data-no-page-cache"

Maybe this is a bit confusing because all pages still get cached for
history nav, but I think it would make sense for people using turbolinks
On Oct 22, 2013 5:59 AM, "Nick Reed" notifications@github.com wrote:

You haven't eased my mind about caching by url. In fact, you admitted
that it will result in behavior that differs from the default browser
behavior. Maintaining as much of the default browser behavior as possible
is a big part of what this library tries to accomplish.
2.

Your opt-out isn't implemented like data-no-turbolink. You're tying it
to the cached page instead of the clicked link.
3.

I still don't think the term transition cache makes any sense. Makes
it sound like we're keeping a cache full of transitions.


Reply to this email directly or view it on GitHubhttps://github.com/rails/turbolinks/pull/286#issuecomment-26764343
.

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Nov 4, 2013

Contributor

@dhh @reed any follow up? I'd like to make sure the opt-out is ok before writing any documentation.

Contributor

smidwap commented Nov 4, 2013

@dhh @reed any follow up? I'd like to make sure the opt-out is ok before writing any documentation.

@reed

This comment has been minimized.

Show comment Hide comment
@reed

reed Nov 4, 2013

Collaborator

You've dismissed all of my concerns, so I'm just going to refrain from expressing any more opinions on this. I'll leave it to you and DHH to hash out the rest.

Collaborator

reed commented Nov 4, 2013

You've dismissed all of my concerns, so I'm just going to refrain from expressing any more opinions on this. I'll leave it to you and DHH to hash out the rest.

@dhh

This comment has been minimized.

Show comment Hide comment
@dhh

dhh Nov 4, 2013

Contributor

Apologize for not having time to review this yet. Hope to soon get a moment.

On Nov 4, 2013, at 12:15, Nick Reed notifications@github.com wrote:

You've dismissed all of my concerns, so I'm just going to refrain from expressing any more opinions on this. I'll leave it to you and DHH to hash out the rest.


Reply to this email directly or view it on GitHub.

Contributor

dhh commented Nov 4, 2013

Apologize for not having time to review this yet. Hope to soon get a moment.

On Nov 4, 2013, at 12:15, Nick Reed notifications@github.com wrote:

You've dismissed all of my concerns, so I'm just going to refrain from expressing any more opinions on this. I'll leave it to you and DHH to hash out the rest.


Reply to this email directly or view it on GitHub.

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Dec 17, 2013

Contributor

@dhh do you still want to review this?

Contributor

smidwap commented Dec 17, 2013

@dhh do you still want to review this?

@dhh

This comment has been minimized.

Show comment Hide comment
@dhh

dhh Dec 22, 2013

Contributor

I continue to think this would make a good default. But we need to make sure that things like the limit on the cache remains, as this is a concern on memory-constrained mobile devices. Do you have a solution for that?

Contributor

dhh commented Dec 22, 2013

I continue to think this would make a good default. But we need to make sure that things like the limit on the cache remains, as this is a concern on memory-constrained mobile devices. Do you have a solution for that?

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Dec 23, 2013

Contributor

Yes, see commit smidwap/turbolinks@9fb3425. Basically, if the cache is full, then the oldest cache is deleted first.

Here's a quick list of what remains to get done:

  1. Decide if "transition cache" is an appropriate name within the code and for exposing the opt-out data-no-transition-cache
  2. Write documentation for the feature and opt-out.

Besides those things, anything else holding us back?

Contributor

smidwap commented Dec 23, 2013

Yes, see commit smidwap/turbolinks@9fb3425. Basically, if the cache is full, then the oldest cache is deleted first.

Here's a quick list of what remains to get done:

  1. Decide if "transition cache" is an appropriate name within the code and for exposing the opt-out data-no-transition-cache
  2. Write documentation for the feature and opt-out.

Besides those things, anything else holding us back?

@dhh

This comment has been minimized.

Show comment Hide comment
@dhh

dhh Dec 30, 2013

Contributor

Ah, I see. Makes sense.

Here’s what I’d actually like to do. I’d like to introduce this feature as off by default first, then shake out any issues in production systems, and then later turn it on by default.

So maybe we can start by having a data-transition-cache=true setting?

On Dec 22, 2013, at 6:03 PM, Matt De Leon notifications@github.com wrote:

Yes, see commit smidwap@9fb3425. Basically, if the cache is full, then the oldest cache is deleted first.

Here's a quick list of what remains to get done:

• Decide if "transition cache" is an appropriate name within the code and for exposing the opt-out data-no-transition-cache
• Write documentation for the feature and opt-out.
Besides those things, anything else holding us back?


Reply to this email directly or view it on GitHub.

Contributor

dhh commented Dec 30, 2013

Ah, I see. Makes sense.

Here’s what I’d actually like to do. I’d like to introduce this feature as off by default first, then shake out any issues in production systems, and then later turn it on by default.

So maybe we can start by having a data-transition-cache=true setting?

On Dec 22, 2013, at 6:03 PM, Matt De Leon notifications@github.com wrote:

Yes, see commit smidwap@9fb3425. Basically, if the cache is full, then the oldest cache is deleted first.

Here's a quick list of what remains to get done:

• Decide if "transition cache" is an appropriate name within the code and for exposing the opt-out data-no-transition-cache
• Write documentation for the feature and opt-out.
Besides those things, anything else holding us back?


Reply to this email directly or view it on GitHub.

@smidwap

This comment has been minimized.

Show comment Hide comment
@smidwap

smidwap Jan 7, 2014

Contributor

Ok, I added documentation and turned off transition cache by default. To enable:

Turbolinks.enableTransitionCache()

I decided to make the opt-in via javascript, not data attributes. Two reasons:

  1. There still needs to be an opt-out via data attributes for individual pages. We can't have both an opt-in and opt-out via data attributes, at least not without some wonky code and confusing documentation.
  2. We want a global opt-in, not a page-by-page opt-in, so it doesn't make sense to tie opt-in to the DOM.

@dhh how do you feel about the name "transition cache"? I agree with @reed that's it not the best name, but I honestly cannot think of another one.

Excited to get this out the door.

Contributor

smidwap commented Jan 7, 2014

Ok, I added documentation and turned off transition cache by default. To enable:

Turbolinks.enableTransitionCache()

I decided to make the opt-in via javascript, not data attributes. Two reasons:

  1. There still needs to be an opt-out via data attributes for individual pages. We can't have both an opt-in and opt-out via data attributes, at least not without some wonky code and confusing documentation.
  2. We want a global opt-in, not a page-by-page opt-in, so it doesn't make sense to tie opt-in to the DOM.

@dhh how do you feel about the name "transition cache"? I agree with @reed that's it not the best name, but I honestly cannot think of another one.

Excited to get this out the door.

@dhh

This comment has been minimized.

Show comment Hide comment
@dhh

dhh Jan 7, 2014

Contributor

I think "transition cache" makes sense as in "use the cache during page transitions". This is otherwise looking good to me. Agree with the global opt-in like that!

Contributor

dhh commented Jan 7, 2014

I think "transition cache" makes sense as in "use the cache during page transitions". This is otherwise looking good to me. Agree with the global opt-in like that!

dhh added a commit that referenced this pull request Jan 7, 2014

@dhh dhh merged commit f57ac9c into turbolinks:master Jan 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment