fix: replace undefined docById with native document.getElementById#4924
fix: replace undefined docById with native document.getElementById#4924walterbender merged 3 commits intosugarlabs:masterfrom
Conversation
|
✅ All Jest tests passed! This PR is ready to merge. |
|
While validating this fix on the live environment to reproduce the crash, I noticed that the server is returning 404 Not Found for several core assets (e.g., themebox.js, ast2blocklist.js) that are present on Localhost. Implication: Once this PR fixes the startup docById crash, the application will proceed further in the loading sequence and will likely hit these missing file errors next (specifically breaking the Welcome Tour). This is not caused by this PR, but fixing the startup crash will "unmask" these underlying server-side missing files. I recommend verifying the deployment build artifacts on the server. 
|
|
✅ All Jest tests passed! This PR is ready to merge. |
|
I originally tried to keep the diff small (~100 lines) by disabling auto-format, but the ESLint / Lint check failed requiring Prettier compliance. I have now applied the standard formatting to satisfy the CI build, which is why the line count has increased |
|
✅ All Jest tests passed! This PR is ready to merge. |
2 similar comments
|
✅ All Jest tests passed! This PR is ready to merge. |
|
✅ All Jest tests passed! This PR is ready to merge. |
Inuth0603
force-pushed
the
fix-startup-errors
branch
from
94ededf to
8c6fe2f
Compare
|
✅ All Jest tests passed! This PR is ready to merge. |
|
Understood! Since document is global, we don't need to declare it here. I have removed it from the list. |
Inuth0603
force-pushed
the
fix-startup-errors
branch
from
8c6fe2f to
c43843d
Compare
|
✅ All Jest tests passed! This PR is ready to merge. |
1 similar comment
|
✅ All Jest tests passed! This PR is ready to merge. |
Inuth0603
force-pushed
the
fix-startup-errors
branch
from
1db5c3b to
8c6fe2f
Compare
|
✅ All Jest tests passed! This PR is ready to merge. |
|
Hi @walterbender, I've addressed the review feedback by removing the global document from the declaration list. I am aware the ESLint / Lint check is still failing. However, I see from the discussion in #4923 that there is a repository-wide conflict between .prettierrc files causing these failures on unrelated files. I have formatted my changes locally, but I will wait for the config fix to merge before attempting any further formatting pushes to keep this PR's history clean. |
2 participants
Description: Fixes #4921
Changes Proposed:
Replaced all 111 instances of the custom helper docById with the standard document.getElementById in js/activity.js.
Reasoning: The custom docById helper has repeatedly caused regressions (see #4295) because it relies on specific dependency load orders that are fragile in the production build.
By switching to the native document.getElementById, we remove the external dependency entirely.
This permanently eliminates the "Race Condition" causing the startup crash because the native DOM method is always available.
Testing:
Localhost: Verified that the application loads and functions correctly without the ReferenceError.
Impact: This ensures the application initializes the UI logic safely without throwing errors in the console.