Download external scripts before executing inline JS? #35

New Issue

2015-01-15T23:37:39-05:00

joshkadis commented

2015-01-15 23:37:39 -05:00

(Migrated from github.com)

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?

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 commented

2015-01-16 00:59:15 -05:00

(Migrated from github.com)

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 :)

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 commented

2015-01-16 10:19:30 -05:00

(Migrated from github.com)

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

MoOx commented

2015-01-16 10:35:01 -05:00

(Migrated from github.com)

Reopening to remember to handle that.

BehindTheMath commented

2017-12-21 13:28:27 -05:00

(Migrated from github.com)

@joshkadis @MoOx
Is this fixed by 09f14fc?

@joshkadis @MoOx Is this fixed by 09f14fc?

MoOx commented

2017-12-21 14:33:47 -05:00

(Migrated from github.com)

No idea.

BehindTheMath commented

2017-12-21 15:04:15 -05:00

(Migrated from github.com)

@lacrioque ?

lacrioque commented

2018-01-24 02:36:20 -05:00

(Migrated from github.com)

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.

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.

joshkadis commented

2018-01-24 12:13:11 -05:00

(Migrated from github.com)

@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

@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 commented

2018-01-26 10:39:46 -05:00

(Migrated from github.com)

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: 09f14fc86c/lib/eval-script.js (L21)

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.

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*: https://github.com/MoOx/pjax/blob/09f14fc86cd161f3a27e988b56212322b08e2a16/lib/eval-script.js#L21 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 commented

2018-01-26 12:23:52 -05:00

(Migrated from github.com)

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.

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.

👍 1

robinnorth commented

2018-01-26 12:46:04 -05:00

(Migrated from github.com)

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

BehindTheMath commented

2018-01-29 08:43:30 -05:00

(Migrated from github.com)

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

BehindTheMath commented

2018-01-31 16:49:38 -05:00

(Migrated from github.com)

@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.

@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 commented

2019-04-21 13:49:48 -05:00

(Migrated from github.com)

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 :)

I am still having this problem if the new request fragment has something like this: ```javascript <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 in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: iLoveElysia/pjax#35