Skip to content

Lazy-load performanceTracker only when performance mode is enabled us…#6434

Open
swapnachoudhary43 wants to merge 4 commits intosugarlabs:masterfrom
swapnachoudhary43:fix/lazy-load-performanceTracker
Open

Lazy-load performanceTracker only when performance mode is enabled us…#6434
swapnachoudhary43 wants to merge 4 commits intosugarlabs:masterfrom
swapnachoudhary43:fix/lazy-load-performanceTracker

Conversation

@swapnachoudhary43
Copy link
Copy Markdown
Contributor

@swapnachoudhary43 swapnachoudhary43 commented Mar 29, 2026

This PR fixes issue #6418 by implementing lazy-loading of performanceTracker to improve application startup performance.

Changes included:

  • Removed the static import of performanceTracker in activity.js so it no longer loads for all users at startup.
  • Updated logo.js to dynamically load performanceTracker using RequireJS only when performance mode is enabled.
  • Introduced window.performanceTrackerInstance to store the loaded module, replacing all previous typeof performanceTracker !== "undefined" checks.
  • Updated all calls to performanceTracker (enable(), disable(), startRun(), enterBlock(), exitBlock(), endRun(), logStats()) to use the new dynamic loading method.

Testing performed:

  1. Normal mode (no ?performance=true):

    • performanceTracker.js is not loaded.
    • No console errors related to performanceTracker.
  2. Performance mode (?performance=true):

    • performanceTracker.js loads dynamically.
    • All instrumentation functions execute correctly.

Outcome:

  • Normal users experience faster startup.

  • Performance mode users retain full instrumentation functionality.

  • Clean code with no leftover typeof performanceTracker checks ensures maintainability.

  • Bug Fix

  • Performance

Note: synthutils.test.js is also failing but this is a pre-existing issue on master branch, completely unrelated to this PR. All other 123 test suites pass including logo.test.js.

@swapnachoudhary43
Copy link
Copy Markdown
Contributor Author

Implemented lazy-loading of performanceTracker (#6418).

  • Removed static import from activity.js.
  • Dynamically load in logo.js only when performance mode is enabled.
  • Updated all calls to use window.performanceTrackerInstance.
  • Tested both normal and performance modes; no errors observed.

@github-actions
Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

logo.test.js

@github-actions
Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

synthutils.test.js

@github-actions github-actions bot added bug fix Fixes a bug or incorrect behavior performance Improves performance (load time, memory, rendering) size/M Medium: 50-249 lines changed area/javascript Changes to JS source files area/tests Changes to test files labels Apr 4, 2026
Moved constants to logoconstants.js to resolve circular dependency.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

synthutils.test.js

@Ashutoshx7
Copy link
Copy Markdown
Contributor

resolve merge conflict
pr looks good

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

synthutils.test.js
logo.test.js
ActionBlocks.test.js

@Ashutoshx7
Copy link
Copy Markdown
Contributor

hy merge conflict again and jest faling

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

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior performance Improves performance (load time, memory, rendering) size/M Medium: 50-249 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants