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

Download external scripts before executing inline JS? #35

Closed
joshkadis opened this issue Jan 16, 2015 · 14 comments
Closed

Download external scripts before executing inline JS? #35

joshkadis opened this issue Jan 16, 2015 · 14 comments
Milestone

Comments

@joshkadis
Copy link

The eval-script module is slick! But it doesn't address if the inline JS to eval is dependent on an external script that's loaded in the new DOM elements, e.g.

<script src="//somewebsite.com/coolThing.js"></script>
<script>coolThing();</script>

Would you be open to a pull request for that? Or is there a reason why you wouldn't want to do that?

@MoOx
Copy link
Owner

MoOx commented Jan 16, 2015

No specific reason. Please contribute.

Note that current master seems to be in a broken state due to a refactoring (one big untested file to several small tested pieces).
If you want to make a PR, please do that on the current master following the "in-progress" refactoring :)

@joshkadis
Copy link
Author

Sounds good! Thanks. Will check back for the refactoring.

@MoOx
Copy link
Owner

MoOx commented Jan 16, 2015

Reopening to remember to handle that.

@BehindTheMath
Copy link
Collaborator

@joshkadis @MoOx
Is this fixed by 09f14fc?

@MoOx
Copy link
Owner

MoOx commented Dec 21, 2017

No idea.

@BehindTheMath
Copy link
Collaborator

@lacrioque ?

@lacrioque
Copy link
Collaborator

I'm sorry I've just seen it a short while ago.
The problem here is, that external scripts are loaded asynchronously.
So we would need another event to give inline-scripts the possibility to react on that.
This however needs promises or multiple callbacks.
This library should be without any dependancies and thus i couldn't just add Promises.

@BehindTheMath BehindTheMath reopened this Jan 24, 2018
@joshkadis
Copy link
Author

@lacrioque It's been a while but I vaguely recall having solved this somehow. I will have some free time starting next week to remember what this is all about.

🚀 time travel back to 2015

@robinnorth
Copy link
Collaborator

Whilst looking into #117, I noticed that lib/eval-script.js claims to force inline external scripts to execute asynchronously, but actually forces them to be synchronous:

script.async = false; // force asynchronous loading of peripheral js

Does this not mean that, assuming the script tags in the new document being switched are written in the correct order (i.e. inline scripts that depend on external script dependencies come after the external dependency script tag) as per @joshkadis' original example, this should be fixed? Inline script tags are parsed in the order in which they are found in the new document, so the source ordering should remain as intended.

I haven't tested this assertion yet, but will aim to try and do so.

@BehindTheMath
Copy link
Collaborator

I did a quick test, which appeared to be like you said. The comment on that line would be wrong, but the behavior seems to be correct.

If that's true, I think we should leave it as synchronous, and just change the comment. That behavior would reflect how a new page would be parsed, which is by default synchronous.

@robinnorth
Copy link
Collaborator

I have actually just changed that comment in #118, so maybe that's all we need to do to close this 😄

@BehindTheMath
Copy link
Collaborator

@joshkadis @lacrioque Is there anything we're missing here?

@BehindTheMath BehindTheMath added this to the 0.2.5 milestone Jan 29, 2018
@BehindTheMath
Copy link
Collaborator

@joshkadis @lacrioque I'm going to assume for now that this was fixed by 09f14fc, and the comment was just backwards (and was fixed in #118).

If that's incorrect, please let us know and we can reopen this.

@lneves12
Copy link

I am still having this problem if the new request fragment has something like this:

<script src='/javascript/external.js'></script>
<script>
   externalDefinedVariable();
</script>

The script "/javascript/external.js" is still being loaded asynchronously. I will try to investigate it deeper, but some hints would be helpful :)

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

No branches or pull requests

6 participants