arvgta / ajaxify

Ajaxify - An Ajax Plugin
https://4nf.org/
275 stars 124 forks source link

Inline scripts evaluated before src ones when heavy #64

Closed Shade- closed 9 years ago

Shade- commented 9 years ago

Hi, I've returned on an old project and I am willing to complete it using your awesome framework. However, there are some issues I have to deal with and the first one is caused by heavy scripts which are loaded after some inline scripts that require the heavy ones to be fired. The inline scripts don't find the necessary functions loaded and they fail, returning an error and breaking Ajaxify's workflow.

It would be awesome if you could find a way to load inline scripts only after the src ones have been loaded and evaluated completely.

arvgta commented 9 years ago

Hi,

thanks for using Ajaxify!

Please see delta-loading

all CSS’s and scripts with data-class="always" are loaded first, if the URL remains the same

Now, I doubt that the

 data-class="always"

will solve this issue, but something similar might be introduced, that makes a script load first.

Something like:

data-class="first"

That only makes sense, if the script src ones in your use-case are not the first ones anyway?

Shade- commented 9 years ago

The src ones are located in the head but they are very heavy, thus the inline ones evaluate before the big ones are ready - and this causes a JavaScript error to be triggered. I will try the data-class="always" property but I don't think that would do the trick, as the page is not the same. I am already using delta-loading and the issue arises because it is active.

arvgta commented 9 years ago

Are the src ones the first ones / at the very top?

Shade- commented 9 years ago

Yes. That's the strange thing. I have added some console logs in some crucial points and despite the code should already handle head scripts before inline ones the script is just too heavy and the inline ones are evaluated before.

arvgta commented 9 years ago

...and when you disable delta-loading you don't have this problem? I was hoping we would have some room for moving the src ones further up. Can you think of a solution?

The only thing I could think is an option to attach the inline-scripts to the main content div? (instead of eval)

If they fire too fast, maybe we could slow them down that way?

Shade- commented 9 years ago

Maybe. Or maybe evaluate and include inline scripts when src ones are completely loaded, if it's possible. If you are retrieving the scripts with jQuery's getScript() function, then you could start adding the inline ones in the auccess callback function which is available.

arvgta commented 9 years ago

Which solution would you prefer?

Balupton's Ajaxify implements the first variant... That might be an interesting option, which really adds value to the algorithm?

The only thing that puzzles me is:

EDIT: I get it - the main content div is obviously swapped every time, deleting the previously attached scripts...

Yes, I could try impementing that - it probably won't require a lot of code

Shade- commented 9 years ago

Thank you!

arvgta commented 9 years ago

Alright - I'll try implementing that shortly (if possible today) Thank you for pointing out this issue!

arvgta commented 9 years ago

Coded in #65

Demo + Source

Please set:

inlineappend: true

to enable

(if successfully tested - I might make it default, because it really might reflect, what the majority of users expect)

Shade- commented 9 years ago

It does not work. I have debugged the stack trace generated by the JS error, and it seems that the inline scripts are added together with divs when these are replaced: the head tag is replaced after the HTML tags within the body and this should cause the error.

arvgta commented 9 years ago

Sorry about that - back to the drawing board... I have the suspicion that the things are being performed double, e.g. the old inline scripts are not being detached properly.

However, I could not see the newly inserted scripts in superfluous div tags? (I reckon, you mean that both newly inserted script-tags along with the ones in divs are inserted simultaneously?)

Would like to do this properly and not just a "quick fix"

Strangely, http://4nf.org/ works with the new option (functionally). (I agree that the DOM looks rather corrupted)

Shade- commented 9 years ago

The _lDivs() function replaces the body tags with the new ones. However, since they might contain inline scripts, those are added too (_ld() function). This happens before calling $.scripts("a"); which should handle inline scripts, resulting in a premature load of inline scripts and a double load.

A solution might be: replace the body tags but not converting the scripts in the _ld() function, and replace (thus execute) the scripts in the $.scripts("a") function.

Assuming I understood the code correctly.

arvgta commented 9 years ago

Thanks so much!

I've made the proposed change in this test file

Replaced the following line:

//if(!($(this).attr("src"))) $(this).replaceWith('<script type="text/javascript">' + $(this).text() + '</script>');

with this:

if(!($(this).attr("src"))) $(this).replaceWith('');

http://4nf.org/ still functions

Shade- commented 9 years ago

It works :)

Shade- commented 9 years ago

However, now pages with heavy scripts freeze the browser. Weird.

arvgta commented 9 years ago

Does the problem persist? What does the console say?