MoOx / pjax

Easily enable fast Ajax navigation on any website (using pushState + xhr)
MIT License
1.46k stars 125 forks source link

Inline scripts can trigger `pjax:complete` event before `loadUrl` does #117

Closed robinnorth closed 6 years ago

robinnorth commented 6 years ago

After an inline external script that has been processed by lib/eval-script.js loads, it triggers a pjax:complete event which can sometimes fire before loadUrl triggers its own pjax:complete and pjax:success events.

The documentation doesn't suggest that we should be firing multiple pjax:complete events, so I think we should either change the event being fired by these injected scripts (to something like pjax:complete:external) or remove them entirely -- is there a use case for them to fire events?

BehindTheMath commented 6 years ago

Is this related to #35?

robinnorth commented 6 years ago

Any fix for #35 will almost certainly have to hook into the script onload events defined in lib/eval-script.js, but the two issues don't otherwise have the same cause or solution. Might be good to try and fix both at the same time, but this one is probably the easier of the two to resolve.

Bran99 commented 6 years ago

I'm not so sure this is closed. I've manually invoked .loadUrl and pjax:complete is firing before and after. Commenting out pjax.loadUrl(url, options) seems to still work and remove the double firing, but I'm not sure this is the best approach. The following is my invocation with pjax.loadUrl commented out:

$(document).on("click", "a[href]:not(.no-pjax)", function (e) {
  e.preventDefault()
  url = $(e.target).closest("a").attr("href")
  pjax = new Pjax({
    elements: "a[href]:not(.no-pjax)",
    selectors: ["title", "#pjax-container", "body"],
    cacheBust: false,
    switches: {
      "body": function (oldEl, newEl, options) {
        newClass = $(newEl).attr("class")
        $(oldEl).attr("id", newEl.id).attr("class", newClass)
        @onSwitch()
      }
    }

  // pjax.loadUrl(url, pjax.options)
  })
})

Like I said, it seems that everything is still running with regards to the pjax functionality. Am I missing something?

BehindTheMath commented 6 years ago

@Bran99 If you're using Pjax, you don't need click handlers; Pjax handles that for you. You also don't need to specify switch functions unless you want custom behavior.

Try replacing the above code with just the Pjax constructor:

const pjax = new Pjax({
  elements: "a[href]:not(.no-pjax)",
  selectors: ["title", "#pjax-container", "body"],
  cacheBust: false,
});