I have a HTML5 application which is using jquery 3.2.1.
In part of the application - a search feature - I make an ajax request. The response from the ajax request is HTML, and this includes a <script> tag which links to a js file which is hosted on the same server as the application.
So the ajax code looks like this - for making the ajax request and writing the response to a div with the ID #ajaxContent:
$.ajax({ url: $('#searchRegulations').attr('action'), type: 'post', cache: false, data: $('#searchRegulations').serialize() }).done(function (response, status, xhr) { if (response) { $('main .content').hide(); $('#ajaxContent').html(response).show(); return false; } } });
If I inspect #ajaxContent I can see that the <script> tag is included in the ajax response:
I have also checked my Network tab to make sure /js/search_regulations.js is being loaded correctly, and it's giving a 200 response:
Inside search_regulations.js there is some jquery which toggles some filters that are present in #ajaxContent.
The problem is that this code only seems to be working about 50% of the time. When it works it will toggle the state of some filter buttons by adding/removing a class .active to elements inside .browse-ctp__filters-data and then writing them to a hidden form with the ID #tmpFilters.
To ensure the script was "firing" I put in the line console.log('search_regulations.js firing'); and sure enough this is shown in the Console every time irrespective of whether the script functions or not.
What's stranger is that if I cut/paste the code into my Console after the ajax response has been written to the page, it always works as expected.
Is this some issue with the way the script is being brought into the page?
I've pasted the script below, but I don't think it's an issue with the code in it, rather the way the browser/ajax response is being handled:
$(function() { console.log('search_regulations.js firing'); /* toggle the active (applied) state on browse by filters */ /* @link https://stackoverflow.com/questions/48662677/switch-active-class-between-groups-of-include-exclude-buttons */ $(document).on('click', '.browse-ctp__filters-data .include, .exclude', function(){ var $this = $(this); // Split name into array (e.g. "find_355" == ["find", "355"]) var arr = $this.attr('name').split('_'); // Toggle active class $this.toggleClass("active"); if ($this.siblings().hasClass("active")) { $this.siblings().removeClass("active") } // Remove any existing instances of the filter from hidden form $('#tmpFilters input[value="exclude_'+arr[1]+'"]').remove(); $('#tmpFilters input[value="find_'+arr[1]+'"]').remove(); // If a filter has been applied then add it to hidden form if ($this.hasClass('active')) { $('#tmpFilters').append('<input type="hidden" name="tmpFilter[]" value="'+$this.attr('name')+'">'); } }); });
Notes about the Bounty offered:
I've offered a bounty because it's not a trivial problem to solve - demonstrated by the fact nobody has given a workable answer. I expect the correct answer to:
- Be demonstrable with jsfiddle or equivalent.
- Explain how/why it works.
- Understand that the ajax response is HTML and js. The js acts on HTML elements in the response. Therefore both the HTML and js need to be included in the response - as opposed to saying "just add the js to a global file" (I don't want the js to be global, because it's specific to the HTML response given, and can vary in different parts of the application).
- Should not use a timeout (
setTimeout or whatever). If the user interacts with the UI elements - such as buttons - returned in the HTML response before the timeout and therefore js is fired... that just leads to the same problem we have now. So that isn't a valid solution as far as I'm concerned. - If this problem is impossible to solve in HTML5/jquery explain why and suggest any alternative ways to handle it.
jsfiddle showing the HTML and script tag returned via ajax:
Several people have asked for a fiddle or demo. No 2 people would get the same outcome - which is very much the point of the question - so I didn't make one originally. The HTML returned that gets written to #ajaxContent is shown here: http://jsfiddle.net/v4t9j32g/1/ - this is what dev tools in the browser shows following the ajax response. Please note that the length of the content returned can vary, since it's the response to a keyword search facility that brings back a load of filter buttons. Also note that this HTML response contains the line <script src="/js/search_regulations.js"></script>. That is where the problematic js is located and the full contents of that are shown above in this question - the bit that includes console.log('search_regulations.js firing')
8 Answers
Answers 1
One of the problems that I see is that you're binding the onclick to the same elements multiple times...
Since the js can be loaded multiply times via ajax requests, it is important to first detach, before attaching again events.
The other problem, is that you're running the code in $(document).ready event (that is when HTML-Document is loaded and DOM is ready), however you'd probably be better off to run the code in the $(window).load event (which executes a bit latter, when complete page is fully loaded, including all frames, objects and images)
Example:
$(window).load(function() { console.log('search_regulations.js firing'); //off: detach the events //on: attach the events $('.browse-ctp__filters-data .include, .exclude').off('click').on('click', function(){ ... } });
Answers 2
In addition to what everybody were talking in the comments, I'll try to put it into an answer.
Event driven
JS is event driven, that doesn't mean you have to load scripts on the fly to make them execute functions when an event is fired. A better approach would be to load everything you need on the page load, and add event listeners, this will case much better user experience(less http requests) and much better code matinability.
TL;DR
In general I would do something like that in you html file:
<script src="/js/main.js"> <script src="/js/search_regulations.js" async> <!-- bonus tip! google "async script tag" -->
This will load all the js you need.
Keep in mind that search_regulations.js should add the event listener you want with jQuery on method, but since the html didn't exist when the script added the event listener, you might want to check this.
Why this is better?
- Now you had pretty easy case where one file loads another. In the future you might want to add more features and more complex code. This will be painful to debug when you have chains of scripts that load each other.
- The user has to wait until the request for the
search_regulations.js file to be loaded, if they're on the a mobile network/the file will get bigger in size that might be a poor user experience. - You can still reuse your code over the application with
search_regulations.js
Answers 3
You need to strip the < script > tag out of the HTML and create a script tag manually to add to the DOM. Essentially, you can convert the Html to JS via:
var $html = $(data.Html)
Then, pull out all the script tags like this:
var scripts = []; $html.each(function(i, item) { if (item instanceof HTMLScriptElement) { scripts.push(item); } });
Then you need to remove all the < script > tags from the html before appending it to the DOM.
$html.find("script").remove(); $("placeToAddHtml").append($html);
Once the HTML, stripped of the tags is added, you can then manually add each script to the DOM at the end:
for (var i = 0; i < scripts.length; i++) { var script = document.createElement('script'); $(scripts[i]).each(function() { $.each(this.attributes, function(j, attrib) { var name = attrib.name; var value = attrib.value; script[name] = value; }); }); script.text = scripts[i].innerHTML; document.body.appendChild(script); }
Hope this works how you intend it!
Edit: You may need to build up the script tag here in a way that appends all of the pulled out scripts at the same time if they depend on each other. That way, you only add one giant script tag that has all the other tags internally at once.
Answers 4
To avoid the possibility of a race condition without implementing a flat timer, you will need to return an ajax response that separates the script from the html, such as a json object that returns two elements (the script or an array of scripts to support cases where you need multiple, and a second element that contains the stringified html markup).
Sample json response from ajax:
{ "script": "/yourscript.js", "html": "<p>Your markup...</p>" }
This (parsed) json will be referred to as xhrResponse below for brevity.
Before applying either to the dom, append a preload tag for your script(s) prior to loading the dom element, so they will start being resolved in the background without loading, which should minimize the overall loading timeline, and help alleviate much of the possibility of ongoing race conditions:
document.getElementsByTagName('head')[0].appendChild( '<link rel="preload" href="' + xhrResponse.script + '" as="script">' );
This will begin loading your script in the background without executing it, so it can be applied immediately when you apply the script tag to the dom later.
You will then want to append the dom elements next, and then apply the scripts after the new dom content has resolved. You may alternately pass the markup directly in the json response, or alternately return a link to a template file and serve that in conjunction with the above mentioned preload method for the template itself. If you are more concerned with the possibility of escaping issues on content, do the latter. If you are more concerned with bandwidth and latency, do the former. There are some minor caveats to both, depending on your use case.
document.getElementsByTagName('head')[0].appendChild( xhrResponse.html );
You will then want to check that the applied html content has resolved within the dom. An arbitrary wait timer is not a very good way to do this, as it has a high likelihood of making the user wait longer than is necessary, and occasional network bottlenecks may also cause your script to occasionally choke if the content does not resolve prior to your arbitrary timer length (more of an issue if you are serving a template from a link instead of stringified html markup), or if the end user has low memory currently available (old machines, bzillion tabs open, or very large html payloads even when served directly stringified).
Please see this answer for a pretty robust solution to check if dynamically added dom content has resolved correctly.
Once this has resolved, then apply the script elements afterward with something like this:
var script = document.createElement('script'); script.src = xhrResponse.script; script.async = true; // use false here if you want synchronous loading for some reason document.head.appendChild(script);
The earlier mentioned preload tag should insure that your script has already been localized by the browser by this point, or is very nearly done doing so already. You can then check if it has completed with a ready state event listener if it is still giving you issues:
script.onreadystatechange = function() { if (script.readyState === "complete") { // debug stuff, or call an init method for your js if you really want to be sure it fired after everything else is done. } }
It should be noted that in the above ready state listener, if the preload resolves prior to this portion of your logic, the ready state will not change (because it is already done), and this portion of logic will not execute. Depending on the weight of your DOM content, this may or may not apply to you.
This approach will insure loading in the correct order, and also decouple your script and markup, so either can be recycled independently of each other elsewhere if applicable now or at any point in the future either.
Answers 5
The problems i think could be: 1. Include and exclude triggers the same function. Error in logic inside to handle both. (since you told it works 50% of the time). 2.Try updating the DOM after you have appended the HTML.
componentHandler.upgradeDom();
just after
$('#ajaxContent').html(response).show();
Hope it helps !!!
Answers 6
NOTE:
- @bastos.sergios already answered your question — see his first point.
- Credit should also be given to @yts for "strengthening" Bastos's answer.
So the following should fix the problem with your search_regulations.js script:
$(document) // Detach the event handler to prevent multiple/repeat triggers on the target elements. .off('click.toggleActive', '.browse-ctp__filters-data .include, .exclude') // Attach/re-attach the handler. .on('click.toggleActive', '.browse-ctp__filters-data .include, .exclude', function(){ ... put your code here ... });
But my primary intention of writing this answer, is to let you see the problem with your script.
And if you're curious, here's script I used with that demo, which slightly modified:
https://codepen.io/anon/pen/RBjPjw.js
Answers 7
About the requirements
The solution to the problem is available using either HTML5/jQuery or a combination of a previous HTML version and native Javascript. The only reason to use a library like jQuery is to bring older browser to the same level of support or to fix bugs and to have a standard framework shared between groups of developers.
Also, in my opinion, it doesn't matter where you place the few lines of code needed to solve the problem, they can be put in the head section of the page, at the bottom of the page or as for the requirements of the bounty inside the Ajax response payload that is reloaded frequently.
About the issue
The real trick lies in the method you use to achieve the firing of click events, your choice of doing that inside the script present in the repeated Ajax responses is what makes your cade fail at some point.
Each time an Ajax request happens the script in the response adds new event listeners. These will continuously sum up until the browsers fails for some reason (not enough memory, exhausted event stack, or click events race conditions).
About the solution
So, the solution to your issue would be to avoid adding events each time the Ajax request/response happens. This is a task which can be easily solved by a simple "capturing phase event delegation" (not frequently used unfortunately).
An event listener is added only once on one of the top nodes which can be the "document" node itself or the "html" element or the "body" element, thus on one element that is always present in the DOM after the initial page load.
This element will be in charge of capturing all the click on the page, currently present or loaded at a later time (by your Ajax requests), and this element will act that way for the complete life span of the page, no need to setup/destroy events as I see some suggested. Well that will probably work as well but will generate more workload on the browser engine, consume more memory, require more code and overall be slower and unnecessarily tricky to say the least.
Code samples
I have setup two CodePen example for you:
Delegation1 has event handling code in the HEAD section, it is my preferred way to handle these kind of issues if no restrictions or other unknown quirks exists. The benefits with this version is shorter/faster and more maintainable code.
Delegation2 has event handling code in the Ajax response and should meet the requirements about HTML/JS being loaded continuously. There is a check to only install the listener once and avoid multiple events racing & overlapping.
The Javascript code in the examples contains relevant comments and should be enough to explain the implemented strategy to make this as simple as possible and reusable in other similar cases.
These examples were written with older browsers in mind, newer browser expose more powerful APIs like element.matches(selector) very useful with event delegation. I purposely avoided to use it for broader support.
Answers 8
Thinked that if you add this to your ajax params call all will be ok:
async: false
