Repository navigation
Update @jupyter/eslint-plugin to v1.1, adopt new rules - #19307
Merged
jtpio merged 5 commits intoAug 12, 2026
Merged
Conversation
|
Thanks for making a pull request to jupyterlab! |
krassowski
marked this pull request as ready for review
August 12, 2026 12:03
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
This was referenced Aug 12, 2026
Merged
jtpio
pushed a commit
that referenced
this pull request
Aug 14, 2026
…sed by new linter rules (#19318) ## References - Follow-up to #19307, which adopted `jupyter/require-disposable-ownership` and `jupyter/require-disposable-transfer` as warnings and left most findings unaddressed. This fixes them and promotes both rules to errors. - Follow-up to #19168, which hoisted `ActiveCellTool` out of its field renderer, and addressed part of the memory leak ## Code changes - `CellMetadataField` and `NotebookMetadataField` are created once at plugin activation instead of once per rjsf render. - Their `render` disposes the `ObservableJSON` source it replaces. - `Licenses` disposes the widgets that own the bundle tab titles. - The LSP settings form keeps one `Debouncer` for the lifetime of the component and disposes it on unmount. ## Measurements In a scenario with 30 active-cell changes (say pressing down key in a notebook with 30 cells for a few seconds), with the Property Inspector open and both Common Tools and Advanced Tools expanded. | | 4.6.2 (before #19168) | 4.6.3 / `main` | this PR | | ----------------------- | ------------- | --------- | -------- | | `ActiveCellTool` | +29 | 0 | 0 | | `CellMetadataField` | +30 | +30 | 0 | | `NotebookMetadataField` | +30 | +30 | 0 | | `CodeMirrorEditor` | +60 | +60 | 0 | | heap growth | +11.0 MiB | +10.7 MiB | +7.1 MiB | #19168 removed 29 abandoned tools but only about 10 KiB per cell change, because that tool renders its preview as highlighted `<pre>` and carries no CodeMirror editor. The metadata fields each carry one. This scales with active-cell changes, not with notebook size and not with typing: selecting a cell, arrowing in command mode and shift+enter each rebuild the form, while 13 keystrokes inside one cell rebuild it zero times. Growth is linear, at 123 KiB per change over 30 changes and 119 KiB over 90. At 119 KiB, for a session that keeps the panel open: | how often user switches cells? | over 8 hours | reclaimed | | --------------------------- | ------------ | --------- | | every minute | 480 | ~56 MiB | | every 30 s | 960 | ~112 MiB | | every 10 s | 2,880 | ~335 MiB | One shift+enter pass over a 100 cell notebook is 100 changes, so about 12 MiB. With the Property Inspector closed the field renderers do not run and nothing is reclaimed. All of the measured reclaim comes from the two metadata fields. The other two findings are dropped ownership rather than retained memory, and are fixed for correctness: `TabBar.clearTabs()` already disconnects each title's signal before dropping it, and a `Debouncer` builds its `Poll` with `auto: false`, so one that was never invoked holds no timer. Measured with a temporary Galata probe: instance counts through `Runtime.queryObjects` after a forced GC, heap size from `Runtime.getHeapUsage`. ## User-facing changes Lower memory use in long sessions that keep the Property Inspector open, by the amounts above. The same scenario still grows about 219 KiB per change from retention elsewhere in the cell-switching path, so this removes roughly a third of it. Edits to LSP server settings are now coalesced into a single write. Previously a render between two edits put them on different debouncers, so each pending write carried the state of the render that scheduled it. A write now lands 500 ms after the last edit. ## Backwards-incompatible changes None. ## AI usage - **YES**: Some or all of the content of this PR was generated by AI. - **YES**: The human author has carefully reviewed this PR and run this code (keep this PR "draft" until the answer is YES) - AI tools and models used: Opus 5
krassowski
added a commit
that referenced
this pull request
Aug 15, 2026
## References - Builds on top of #19318 which is follow up to #19307 that introduced the signal lifetime lint rules. - Fixes #19267: the collapse-state listeners are now registered once per panel, guarded by a `WeakSet` as suggested there, and cleaned up through the panel's disposal. The fixes were found by tracing retaining paths in heap snapshots for three scenarios: deleting cells, closing a notebook, and closing one of several views of a notebook. > Note to reviewer: since this PR builds on top of #19318, until that is merged and this PR rebased you may want to focus on 6125aa1 alone ## Code changes ### 1. Moving or deleting cells Creating, moving and deleting cells (every cell move disposes the old widget and creates a new one): - The vendored y-codemirror undo plugin deletes its selection metadata from the undo manager stacks on destroy. The stack items live as long as the document and the metadata pinned the destroyed editor view, and through it the whole deleted cell. Its only purpose is restoring the selection, which has no meaning once the view is gone. - `Cell`, `CodeCell` and `InputArea` dispose the widgets that can be swapped out of their layout (input and output areas with their placeholders, and the rendered widget of a markdown cell in edit mode): `Widget.dispose()` only disposes layout children, and an undisposed output area stays connected to the output model. - `OutputArea` removes its prompt overlay click listener and `ResizeHandle` removes its DOM listeners on dispose, so DOM retained by anything else no longer keeps the widget graphs alive. ### 2. Closing a notebook Closing a notebook, console or file editor (with or without debugging ever used): - `StaticNotebook.dispose` disposes all cell widgets explicitly. In the `full` and `defer` windowing modes out-of-viewport cells are not layout children, so `Widget.dispose()` misses them and their connections to the shared model leak every one of them; in the default `contentVisibility` mode this is a safety net (verified by ablation: the probes, which run under the default mode, pass without it). - `ReactWidget` unmounts its React root on dispose. It only unmounted on detach, so any React-based item disposed while already detached (such as toolbar buttons parked in a disposed cell) kept its root and every subscription of the rendered components, including `UseSignal` connections to the `CommandRegistry`. - `DebuggerHandler` connects its per-widget handlers with the widget as receiver and releases them on widget disposal: `Signal.clearData(widget)` removes the connections and the disposal hook deletes the id-keyed map entries. Previously those maps on this application-lifetime object kept a closure over every closed widget forever whenever debugging had not been started for it. The handlers stay connected for the widget's lifetime, including through the non-debugging passes of `updateWidget` (covered by a new regression test). Also guards `update`, `updateContext`, `updateWidget` and `createHandler` against widgets disposed during awaits, and clears `activeWidget` on disposal. - `DocumentConnectionManager` sweeps disposed virtual documents out of its `documents` map: the adapter clears its `virtualDocument` reference before the disposal handler runs, so the existing `documents.delete` call never fired. `VirtualDocument.dispose` also clears its editor maps, and the `adapters` entry is removed under the document's current path so a renamed document does not survive its closure. - `CellBarExtension` drops its reference to the last tracker when its panel is disposed. ### 3. Closing a second view of a notebook Closing one of several views of a document (New View for Notebook, cloned output views): - `ExecutionIndicator` disposes its model in a `dispose` override (covering the toolbar item and the status bar item alike), the model's kernel and cleanup handlers connect with the model as receiver so disposal clears them, and the elapsed-time interval terminates itself when the model is disposed while a kernel is busy. - The notebook extension connects its restore-data, collapse-state and completer handlers with the panel as receiver, and makes the collapse-state and completer connections once per panel instead of on every activation or settings change. - The notebook output log connections in `nboutput.ts` use the panel as receiver. - `DocumentConnectionManager.registerAdapter` disconnects its `pathChanged` slot when the adapter is disposed. - `StaticNotebook.dispose` disposes the view model it creates, which disconnects it from the shared cell list. `WindowedList` does not take ownership of the caller-provided model, so its lifetime stays with the creator. ### Dependencies and configuration - `@jupyter/web-components` and `@jupyter/react-components` are bumped from 0.16.6 to 0.17.1. In 0.16.x, `Toolbar.connectedCallback` bound `fillColor` against its parent element, making fast-foundation create a permanent `DesignTokenNode` for that element (jupyterlab-contrib/jupyter-ui-toolkit#126 removed this in 0.17.1). Every cell the user activated, and every closed view, leaked its whole detached DOM tree through these nodes, re-anchoring widget graphs through any DOM listener. The 0.17.1 toolbar no longer synchronizes its fill color, which is the visual change that PR describes as aligning with Jupyter styling. - `eslint.config.mjs` adds the kernel session types to `longLivedTypes` for the signal lifetime rules (the option replaces the default list, so the twelve defaults are restated). This yields no findings on the current codebase, including on the sites fixed here, which other gates of the rules skip. The leaks in this PR were found with runtime heap snapshots, not lint; the config change guards new code connecting to session signals. ## Measurements All scenarios use a 75-cell notebook. Scenario 1: add 75 cells each printing 256 KiB, make each the active cell, run them, then delete them all. Moving a cell (keyboard or drag and drop) disposes and recreates its widget, so cell moves exercise the same path per move. | | `main` | this PR | | ------------------ | --------- | --------- | | `CodeCell` | +75 | 0 | | `OutputArea` | +75 | 0 | | `OutputAreaModel` | +75 | 0 | | heap used | +77.2 MiB | +35.9 MiB | | DOM nodes | +67,992 | +50,602 | | JS event listeners | +205,329 | +6,345 | Scenario 2, closing a notebook: open and close a saved 75 cell notebook 5 times, without interacting with it beyond letting it render and its kernel start. | | `main` | this PR | | ------------------ | --------- | -------- | | `NotebookPanel` | +5 | 0 | | `Notebook` | +5 | 0 | | `CodeCell` | +380 | 0 | | heap used | +14.3 MiB | +6.0 MiB | | DOM nodes | +9,916 | +5,716 | | JS event listeners | +63,548 | +725 | Every closed notebook used to be retained whole with all of its cells. Scenario 3: open a second view with New View for Notebook (`docmanager:clone`), let it render, close it, 5 times. | | `main` | this PR | | ------------------ | --------- | -------- | | `Notebook` | +5 | 0 | | `CodeCell` | +375 | 0 | | `OutputArea` | +375 | 0 | | `CodeMirrorEditor` | 0 | 0 | | heap used | +12.5 MiB | +3.0 MiB | | DOM nodes | +13,950 | +5,619 | | JS event listeners | +114,645 | +580 | Note the listeners row: tens of thousands of leaked listeners per scenario drop to a few hundred. > Taken with temporary Galata probes, not included here: instance counts through `Runtime.queryObjects` after a snapshot-forced GC, heap from `Runtime.getHeapUsage`, DOM and listeners from `Performance.getMetrics`. #19362 discusses making that a part of CI. ## Remaining known growth - Deleting cells keeps their content, including outputs, in the yjs document history so the deletion can be undone; that is the bulk of the remaining heap growth in scenario 1 and is intended. - Output rendering that is still in flight when a view is closed is parked forever by the async render pipeline: the pending render generator holds the output area and its host DOM. This is bounded by how much rendering was pending at close (the probes wait for rendering to finish before closing, as a user looking at the view would). Worth a follow-up issue: async output rendering should abort on dispose. - The rest of the per-cycle heap and DOM growth in scenarios 2 and 3 is session-scoped state (kernel and session bookkeeping, notifications), not widget retention. ## User-facing changes Lower memory use for everyday notebook work: creating, moving and deleting cells no longer accumulates dead cells and editors, closing notebooks no longer retains them whole, and closing extra views of a document no longer retains the whole view. The toolkit bump also changes the toolbar to no longer stand out from its parent background, aligning with Jupyter styling. ## Backwards-incompatible changes None ## AI usage - **YES**: Some or all of the content of this PR was generated by AI. - **YES**: The human author has carefully reviewed this PR and run this code (keep this PR "draft" until the answer is YES) - AI tools and models used: Fable 5
krassowski
added a commit
that referenced
this pull request
Sep 11, 2026
<!-- Thanks for contributing to JupyterLab! Please fill out the following items to submit a pull request. See the contributing guidelines for more information: https://github.com/jupyterlab/jupyterlab/blob/main/CONTRIBUTING.md --> ## References - Follow-up to #19307 ### Description Fixes the 53 `jupyter/galata-prefer-filebrowser-helper` warnings. Three spots keep the raw double-click with an eslint disable and the reason in the comment. <!-- Note issue numbers this pull request addresses (should be at least one, see contributing guidelines above). --> <!-- Note any other pull requests that address this issue and how this pull request is different. --> ## Code changes <!-- Describe the code changes and how they address the issue. --> Most of them were "double-click the folder, then double-click the file", which collapses into a single helper call: ```ts await page.notebook.openByPath('notebooks/Lorenz.ipynb'); await page.filebrowser.open('narrative/jupyterlab.md'); await page.filebrowser.openDirectory('notebooks'); ``` ## User-facing changes None ## Backwards-incompatible changes None ## AI usage - **YES**: Some or all of the content of this PR was generated by AI. - **YES**: The human author has carefully reviewed this PR and run this code (keep this PR "draft" until the answer is YES) - AI tools and models used: Claude Opus --------- Co-authored-by: ANUJ KUMAR SINGH <anujsinghhero292@gmail.com> Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
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
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.
References
@jupyter/eslint-plugintov1#18863CC @Darshan808
Code changes
no-restricted-syntaxfor centralizedjupyter/no-pageconfig-base-urlThis PR does not try to address the larger memory leaks found by the new rules - these will be addressed in a separate PR targeting 4.7.0 only; this PR targets
4.6.xto provide the quick win of translation fixes and avoid divergence between branches for easier backporting of PRs.User-facing changes
More UI will be translated :)
Backwards-incompatible changes
None
AI usage