-
Notifications
You must be signed in to change notification settings - Fork 524
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
fix(playground/runner): fix external script loading in playground #11017
fix(playground/runner): fix external script loading in playground #11017
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just two nits.
@fiji-flo Any concerns with this change?
Co-authored-by: Claas Augner <[email protected]>
Co-authored-by: Claas Augner <[email protected]>
b3b98d2
to
4099a6b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny nit, otherwise LGTM.
Co-authored-by: Claas Augner <[email protected]>
@fiji-flo , could you please find a moment to check the latest updates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pseudopilot. This looks great now.
Summary
Fixes #10273
Problem
Using external scripts in the playground HTML section leads to the errors because the external script is loaded later than we start using entities from it in JAVASCRIPT section. As a result some code examples may not work like on this page:
https://developer.mozilla.org/en-US/docs/Games/Techniques/2D_collision_detection
Solution
When building the page (iframe) we remove the external scripts from document's
body
and put them in the document'shead
to make sure they are loaded before any other user scripts start running.Alternative solution could be adding
onload="onScriptLoaded()"
to the external script and wrapping all the logic from JAVASCRIPT section into thefunction onScriptLoaded()
. But this approach has its drawbacks.First of all, everyone who writes some code snippets with the external scripts would need to know that this addition is needed and add it themselves.
Secondly, if the user wants to add few external scripts then the additional logic needs to be even more complicated.
Before
Screen.Recording.2024-04-27.at.18.30.10.mov
Screen.Recording.2024-04-28.at.10.26.21.mov
After
Screen.Recording.2024-04-27.at.18.29.21.mov
Screen.Recording.2024-04-28.at.10.23.30.mov
How did you test this change?
I couldn't test it on my
localhost
in dev mode because therunner.html
is being fetched from the server. So I used the approach with overridingrunner.html
content in DevTools with the modified version.