Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add native support beforeSend and fix for asynchronous requests #423

Closed
wants to merge 2 commits into from
Closed

Add native support beforeSend and fix for asynchronous requests #423

wants to merge 2 commits into from

Conversation

fourteenmeister
Copy link

Two commits:

  1. Native support beforeSend
    Example:
$.pjax({
     beforeSend: function(e, object) {
            // some scripts
            return true;
     },
     container: "[data-pjax-container]",
});
  1. adding a script tag, depending on the type of request
    Example:
$.pjax({
     async: true, // by default
     container: "[data-pjax-container]",
});

will append script tag asynchronously

$.pjax({
     async: false,
     container: "[data-pjax-container]",
});

will append script tag synchronously

@mislav
Copy link
Collaborator

mislav commented Aug 14, 2014

Could you please explain the rationale behind this change?

@fourteenmeister
Copy link
Author

first commit - Of course I know that there is an event 'pjax:beforeSend'. But it is hung worldwide! What if I want to go to some particular query or queries to execute the something script before send query?

@fourteenmeister
Copy link
Author

second commit - there are situations when the script does not have time to load (due to the fact that it is loaded asynchronously). Some scripts are already starting to work immediately after the request and an error occurs (the script did not have time to load). This commit fixes the problem, of course if the async attribute set to false

@fourteenmeister fourteenmeister changed the title Add native support beforeSend Add native support beforeSend and fix for asynchronous requests Aug 14, 2014
@mislav
Copy link
Collaborator

mislav commented Mar 3, 2015

Sorry it took me long to review this pull request. I had to give it a lot of thought.

I don't see it worth going forward with these changes, though. First of all, this PR deals with 2 separate features, and would be better if it was submitted as separate pull requests. Second, I'm not in favor of both of these solutions.

  1. You manually call options.beforeSend() before starting the ajax request, but jQuery's beforeSend callback is supposed to be called with 2 arguments: xhr object and ajax settings hash.
  2. We don't want to support async: false ajax scenarios because it's not in our interest.
  3. We're aware that using pjax can mess up sequential loading of per-page external script resources. This is an unfortunate technical limitation due to the fact we can't rely on eval External scripts are evaluated async #331 (comment)

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

Successfully merging this pull request may close these issues.

2 participants