fix(dx): resolve script loading race conditions in index.html#6530
Open
vanshika2720 wants to merge 1 commit intosugarlabs:masterfrom
Open
fix(dx): resolve script loading race conditions in index.html#6530vanshika2720 wants to merge 1 commit intosugarlabs:masterfrom
vanshika2720 wants to merge 1 commit intosugarlabs:masterfrom
Conversation
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
github-actions
bot
added
bug fix
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
2 similar comments
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
Contributor
|
This.cleanupIdleWatcher() is called in activity.js (2 places), but the method doesn’t exist. It should be this._stopIdleWatcher(). |
vanshika2720
force-pushed
the
fix/dx-script-loading-race-conditions
branch
from
6b75f71 to
cb0b90d
Compare
github-actions
bot
added
size/XXL
Contributor
|
❌ Some Jest tests failed. Please check the logs and fix the issues before merging. Failed Tests: |
vanshika2720
force-pushed
the
fix/dx-script-loading-race-conditions
branch
from
cb0b90d to
f69477f
Compare
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
vanshika2720
force-pushed
the
fix/dx-script-loading-race-conditions
branch
from
f69477f to
5fe9b88
Compare
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
Contributor
|
❌ Some Jest tests failed. Please check the logs and fix the issues before merging. Failed Tests: |
1 similar comment
Contributor
|
❌ Some Jest tests failed. Please check the logs and fix the issues before merging. Failed Tests: |
vanshika2720
force-pushed
the
fix/dx-script-loading-race-conditions
branch
from
df808c9 to
d3cf6b9
Compare
github-actions
bot
added
size/XL
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
vanshika2720
force-pushed
the
fix/dx-script-loading-race-conditions
branch
from
d3cf6b9 to
7553051
Compare
github-actions
bot
added
size/M
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
vanshika2720
force-pushed
the
fix/dx-script-loading-race-conditions
branch
from
7553051 to
eaddc28
Compare
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
Fix jQuery plugin IIFE that was never invoked: the closing `});` meant the function was defined but never called, so $.fn.fixMe and the $(document).ready handler never executed. Changed to `})(jQuery);` and wrapped in a DOMContentLoaded listener with a typeof guard so it waits for both the DOM and the deferred jQuery script. Remove broken duplicate script block (lines 1203-1285 on master) that contained: - A syntax error (`}} )`) prematurely closing the DOMContentLoaded handler after showPersistentNotification - Duplicate definitions of openFullscreen, closeFullscreen, setIcon, and handleFullscreenChange (already defined globally at lines 1132-1194) - A duplicate `#installButton.hidden` style block The working fullscreen functions and the proper play-only-mode script (at the end of the file) are preserved as-is.
vanshika2720
force-pushed
the
fix/dx-script-loading-race-conditions
branch
from
eaddc28 to
c59113a
Compare
github-actions
bot
added
area/core
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[DX] Resolve Script Loading Race Conditions in index.html
Type of Change
Description
This PR addresses frequent startup console errors (
$ is not defined,createjs is not defined,p5 is not defined) by standardizing and consolidating the script loading architecture inindex.html.Key Changes
<head>with thedeferattribute. This ensures they are loaded in parallel but executed in a deterministic order.DOMContentLoadedlisteners.setIcon,openFullscreen), and eliminated fragmented service worker registration blocks.Value
Dramatically improves the Developer Experience (DX) by providing a "clean" console on startup and making the application initialization sequence robust and easy to reason about. It also improves project reliability across different browsers (especially those with stricter execution timings like Firefox).
Testing Performed
$andcreatejsare ready whenloader.jsexecutes.index.html.