Skip to content

fix : replace manual index.html patching with config-based plugin (#114)#135

Open
Dbansal06 wants to merge 6 commits intosoft-eng-practicum:developfrom
Dbansal06:fix/114-jupyter-lite-config-cleanup
Open

fix : replace manual index.html patching with config-based plugin (#114)#135
Dbansal06 wants to merge 6 commits intosoft-eng-practicum:developfrom
Dbansal06:fix/114-jupyter-lite-config-cleanup

Conversation

@Dbansal06
Copy link
Copy Markdown

@Dbansal06 Dbansal06 commented Mar 17, 2026

Note: PR #120 by @mohab-elshamy addresses the same core issue using a PyPI-published extension. This PR takes a complementary approach — shipping the plugin locally in the repo — and additionally fixes three other problems not covered by #120: consolidating the duplicate JupyterLiteStorageService, fixing a window event listener memory leak in ProjectNotebookItemDisplayComponent, and adding the jupyter_lite_config.json build config so the output path is explicit.

Closes #114

What was wrong

The postMessage('jupyterlite-load') signal that tells AnalySim when
JupyterLite is ready was injected by patching a manually copied
index.html file. Every jupyter lite build regenerates that file,
silently wiping out the patch. This is why upgrading JupyterLite has
historically required manual steps after every version bump.

What I changed

analysim_notifier/ plugin
The postMessage logic now lives in a proper JupyterLite plugin that uses
app.started — the stable API for knowing when the environment is ready.
It gets picked up automatically at build time and survives version upgrades
without any manual intervention.

Config files

  • jupyter_lite_config.json — explicit build config, output always goes
    to dist/, no guessing required
  • jupyter-lite.json — runtime config for app name and disabled extensions,
    previously absent
  • overrides.json — UI theme defaults in one declarative place

Consolidated JupyterLiteStorageService
The IndexedDB service existed in two near-identical copies — one in the
admin folder (forageIndexDb.ts) and one in the project folder
(localforageIndexdb.ts). They used slightly different storage names
meaning bug fixes had to be applied twice. Moved to app/shared/services/
with the old files re-exporting from the new location for backwards
compatibility.

Fixed event listener memory leak
ProjectNotebookItemDisplayComponent added a window message listener
in ngOnInit but never removed it in ngOnDestroy. Opening and closing
a notebook repeatedly would accumulate stale listeners. Fixed by storing
a reference to the bound handler and removing it properly on destroy.

How to test

cd src/Analysim.Web/ClientApp/src/assets/jupyter
pip install -r requirements.txt
jupyter lite build

Run the app and open any notebook — it should load exactly as before,
but no manual file copying is needed after building.

Notes

The dist/ output path is unchanged so nothing else in the codebase
needs updating. The index.html that was manually maintained is deleted
— its only custom logic (the postMessage) now lives in the plugin.

…ugin (soft-eng-practicum#114)

The postMessage signal AnalySim relies on to detect when JupyterLite is
ready was injected by patching a manually copied index.html file. Every
jupyter lite build regenerates that file, silently wiping the patch out.
This is why upgrading JupyterLite has historically required manual work.

Changes in this commit:

- analysim_notifier plugin: the postMessage logic now lives in a proper
  JupyterLite plugin using app.started, which is the stable API for this.
  The plugin survives version upgrades without any manual steps.

- jupyter_lite_config.json: explicit build config so output directory is
  always dist/ and no manual steps are needed after building.

- jupyter-lite.json: runtime config for app name and extension settings,
  previously absent entirely.

- overrides.json: UI defaults (theme) in one declarative place.

- Consolidated JupyterLiteStorageService: the service existed in two
  near-identical copies in admin and project folders. Moved to
  app/shared/services/ with old files re-exporting for compatibility.

- Fixed event listener leak in ProjectNotebookItemDisplayComponent:
  the window message listener added in ngOnInit was never removed in
  ngOnDestroy, causing listeners to accumulate on repeated opens.

Fixes soft-eng-practicum#114
@Dbansal06 Dbansal06 force-pushed the fix/114-jupyter-lite-config-cleanup branch from 79bc27a to ace5af6 Compare March 17, 2026 21:04
Copy link
Copy Markdown
Member

@cengique cengique left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dbansal06 I actually like this solution compared to #120 as it's simpler. However, I am not able to run this version because it gives the "it took to long" error and not staying in the JupyterLite interface. I did follow the instructions in the PR to build JupyterLite and I don't see any illuminating errors in the console. Except these:

GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  2ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  7ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  7ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  7ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  6ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  11ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  11ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  11ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  8ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  7ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  7ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  3ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json
[HTTP/2 404  11ms]

XHR
GET
https://localhost:5001/assets/jupyter/dist/api/contents/all.json

@Dbansal06 Dbansal06 requested a review from cengique March 28, 2026 13:27
@Dbansal06
Copy link
Copy Markdown
Author

@cengique
Thanks for testing! The dist/ folder was missing from the repo which caused the "took too long" error — the plugin couldn't load because there were no built files to serve. I've now committed the built output directly so jupyter lite build is no longer needed after cloning. Please pull the latest and try again — it should work now.

Copy link
Copy Markdown
Member

@cengique cengique left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still doesn't work for me. Please add debug statements to troubleshoot.

.DS_Store


.jupyterlite.doit.db
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove these



.jupyterlite.doit.db
package-lock.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

package-lock.json

.jupyterlite.doit.db
package-lock.json

src/Analysim.Web/ClientApp/src/assets/jupyter/dist/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already in separate .gitignore

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Less hacky way of integrating Jupyter Lite

2 participants