kossnocorp/jquery.turbolinks

Issue with rails_ujs

glebm opened this issue · 10 comments

glebm commented

Delete link (data-method=delete data-confirm="Yes?") + rails ujs + turbolink'd page (after page change) = fail
Likely related to how rails_ujs binds events

kossnocorp commented

Look like I need to hack $.fn.on method to prevent double handlers on document.

glebm commented

Are you thinking of preventing double handlers by checking if a function has already been binded?
Something like this? (not tested)

diff --git a/src/jquery.turbolinks.coffee b/src/jquery.turbolinks.coffee
index 405e608..0def112 100644
--- a/src/jquery.turbolinks.coffee
+++ b/src/jquery.turbolinks.coffee
@@ -24,7 +24,6 @@ turbolinksReady = ->

 # Fetch event handler
 fetch = ->
-  $(document).off(undefined, '**')
   $.isReady = false

 # Bind `ready` to DOM ready event
@@ -47,6 +46,14 @@ $.setFetchEvent = (event) ->
     .off('.turbolinks-fetch')
     .on(event + '.turbolinks-fetch', fetch)

+originalOn = $.fn.on
+$.fn.on = (types, selector, data, fn) ->
+  if selector == document && fn
+    for type in types when fn
+      handlers = $._data(@, 'handlers')[type] || []
+      return @ for h in handlers when h == fn
+  originalOn.apply(@, arguments)
+
 # Bind `ready` to Tubolinks page:load
 $.setReadyEvent('page:load')
glebm commented

Whoever wants to test if this prevents double submits, you can use:

gem 'jquery-turbolinks', github: 'glebm/jquery.turbolinks', branch: 'refs-12'
rstacruz commented

-1 on this... changing jQuery behavior (supressing double-binding) seems like a very bad idea.

If events are double-fired, they are most likely bound twice. If they are bound twice, they're most likely executed twice because you have your script tags in <body>, not <head>.

Try moving all your scripts to <head>, and ensuring that your load order is jquery.js -> jquery.turbolinks.js -> other scripts -> turbolinks.js.

glebm commented

Double-firing in the referred case happens because it's binded in jQuery(->) which gets re-evaluated every page:change

rstacruz commented

Rails-ujs doesn't work inside a jQuery(->) block. It hooks events via delegate ($(document).delegate('[data-method]', function() {...} )), not binding to the elements directly.

I suspect that you only need change your load order to this:

<html>
<head>
  <script src='jquery.js'></script>
  <script src='jquery.turbolinks.js'></script>
  <script src='jquery_ujs.js'></script>
  <!-- other scripts here -->
  <script src='turbolinks.js'></script>
</head>
<body>
  ...
</body>
</html>

Note that it's inside <head> and not inside the body.

glebm commented

I was referring to #8. That issue is the reason jquery.turbolinks started clearing the document events on fetch:

$(document).off(undefined, '**')

Which is why your code above will stop working after the first fetch.

My patch removes the off, but adds a duplication check on on

rstacruz commented

Why is the duplicate check of on needed?

If you bind your delegates (eg, $(document).on('click', 'button', ..)) outside a $(document).ready wrapper, double-binding shouldn't be a problem.

rstacruz commented

I think this can be closed now with 1.0.0-rc2. :)