Skip to content
This repository has been archived by the owner on Jul 28, 2018. It is now read-only.

Track assets named 'data-turbolinks-track' #133

Merged
merged 1 commit into from
Dec 4, 2012

Conversation

yasuoza
Copy link
Contributor

@yasuoza yasuoza commented Dec 4, 2012

Implemented we've discussed in #132.

Use document#reload ONLY when data-turbolinks-track assets are changed, otherwise use Turbolinks#visit.

This commit changes turbolinks' behavior.
Track "data-turbolinks-track" assets in current page,
and trigger document.reload() ONLY when clicked link page's
"data-turbolinks-track"(s) differ from those of current page.
Otherwise browser always uses Turbolinks#visit.
dhh added a commit that referenced this pull request Dec 4, 2012
Track assets named 'data-turbolinks-track'
@dhh dhh merged commit 233e393 into turbolinks:master Dec 4, 2012
@rahearn
Copy link

rahearn commented Dec 4, 2012

This still doesn't work in development if you use = javascript_include_tag "application", data: {'turbolinks-track' => ''} and don't have

//= require_self
//= require turbolinks

As the last two requires in application.js. That seems like a lot of setup to have to remember to do to get this configured properly.

@dhh
Copy link
Contributor

dhh commented Dec 4, 2012

Ryan, seems to work well here. What problem are you seeing? This change makes the order of the requires irrelevant.

On Dec 4, 2012, at 4:01 PM, Ryan Ahearn wrote:

This still doesn't work in development if you use = javascript_include_tag "application", data: {'turbolinks-track' => ''}
and don't have

//= require_self
//= require turbolinks
As the last two requires in application.js. That seems like a lot of setup to have to remember to do to get this configured properly.


Reply to this email directly or view it on GitHub.

@dhh
Copy link
Contributor

dhh commented Dec 4, 2012

I currently have this order in my application.css:

#= require_self
#= require turbolinks
#= require_tree .

And it works as expected.

@rahearn
Copy link

rahearn commented Dec 4, 2012

I'm still seeing the same problem as when I originally opened #118. Using your order of requires with #= require_tree . below #= require turbolinks I end up with

<script data-turbolinks-track="" src="/assets/jquery.js?body=1" type="text/javascript"></script>
<script data-turbolinks-track="" src="/assets/jquery_ujs.js?body=1" type="text/javascript"></script>
<script data-turbolinks-track="" src="/assets/tinynav.js?body=1" type="text/javascript"></script>
<script data-turbolinks-track="" src="/assets/application.js?body=1" type="text/javascript"></script>
<script data-turbolinks-track="" src="/assets/turbolinks.js?body=1" type="text/javascript"></script>
<script data-turbolinks-track="" src="/assets/courses.js?body=1" type="text/javascript"></script>
<script data-turbolinks-track="" src="/assets/navigation.js?body=1" type="text/javascript"></script>
<script data-turbolinks-track="" src="/assets/scorecards.js?body=1" type="text/javascript"></script>

in the page source. When rememberCurrentTrackingAssets is called courses.js, navigation.js, and scorecards.js are not yet in the DOM and thus are not added to the trackingAssets array. When Turbolinks loads the next page, it passes the entire page source to assetsChanged and extractedTrackAssets is now different than trackingAssets thus triggering a full page reload.

@dhh
Copy link
Contributor

dhh commented Dec 4, 2012

Ah, I'm sorry. I misunderstood. Never use config.assets.debug. Seems like we should delay the rememberCurrentTrackingAssets until the DOM is entirely loaded. Want to have a look at that?

On Dec 4, 2012, at 4:20 PM, Ryan Ahearn wrote:

I'm still seeing the same problem as when I originally opened #118. Using your order of requires with #= require_tree . below #= require turbolinks I end up with

<script data-turbolinks-track="" src="/assets/jquery.js?body=1" type="text/javascript"></script> <script data-turbolinks-track="" src="/assets/jquery_ujs.js?body=1" type="text/javascript"></script> <script data-turbolinks-track="" src="/assets/tinynav.js?body=1" type="text/javascript"></script> <script data-turbolinks-track="" src="/assets/application.js?body=1" type="text/javascript"></script> <script data-turbolinks-track="" src="/assets/turbolinks.js?body=1" type="text/javascript"></script> <script data-turbolinks-track="" src="/assets/courses.js?body=1" type="text/javascript"></script> <script data-turbolinks-track="" src="/assets/navigation.js?body=1" type="text/javascript"></script> <script data-turbolinks-track="" src="/assets/scorecards.js?body=1" type="text/javascript"></script>

in the page source. When rememberCurrentTrackingAssets is called courses.js, navigation.js, and scorecards.js are not yet in the DOM and thus are not added to the trackingAssets array. When Turbolinks loads the next page, it passes the entire page source to assetsChanged and extractedTrackAssets is now different than trackingAssets thus triggering a full page reload.


Reply to this email directly or view it on GitHub.

@reed
Copy link
Collaborator

reed commented Dec 4, 2012

Since we're now explicitly declaring which assets to remember and thus don't have to worry about dynamically added scripts, couldn't we just wait to extract the current assets until a link is clicked? Like this?

currentAssets = null

assetsChanged = (doc) ->
    currentAssets ||= extractTrackAssets document
    newAssets = extractTrackAssets doc
    newAssets.length isnt currentAssets.length or intersection(newAssets, currentAssets).length != currentAssets.length

@rahearn
Copy link

rahearn commented Dec 4, 2012

That seems like a much better solution than trying to reliably do on DOM load without jQuery.

@dhh
Copy link
Contributor

dhh commented Dec 4, 2012

Clever. Yes, indeed that's what we can do. Can you make a pull request for it?

On Dec 4, 2012, at 5:13 PM, Nick Reed wrote:

Since we're now explicitly declaring which assets to remember and thus don't have to worry about dynamically added scripts, couldn't we just wait to extract the current assets until a link is clicked? Like this?

currentAssets = null

assetsChanged = (doc) ->
currentAssets ||= extractTrackAssets document
newAssets = extractTrackAssets doc
newAssets.length isnt currentAssets.length or intersection(newAssets, currentAssets).length != currentAssets.length

Reply to this email directly or view it on GitHub.

@reed
Copy link
Collaborator

reed commented Dec 4, 2012

Yeah, I'll submit it in a few minutes.

@swrobel
Copy link

swrobel commented Apr 6, 2013

@dhh:

Never use config.assets.debug

Why?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants