Skip to content

Update @jupyter/eslint-plugin to v1.1, adopt new rules - #19307

Merged
jtpio merged 5 commits into
jupyterlab:mainfrom
krassowski:update-eslint-plugin-add-new-rules
Aug 12, 2026
Merged

jtpio merged 5 commits into
jupyterlab:mainfrom
krassowski:update-eslint-plugin-add-new-rules

Conversation

@krassowski

@krassowski krassowski commented Aug 12, 2026 •

Copy link
Copy Markdown
Member

References

CC @Darshan808

Code changes

  • fixes a few strings not being correctly translated due to template literal usage
  • fixes a few minor memory leaks
  • replaces no-restricted-syntax for centralized jupyter/no-pageconfig-base-url

This 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.x to 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

  • 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 for generation, Fable 5 for review

@jupyterlab-probot

jupyterlab-probot Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

View the integration tests report in the browser: UI tests report

@krassowski
krassowski marked this pull request as ready for review August 12, 2026 12:03

@jtpio jtpio left a comment

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.

Thanks!

@jtpio
jtpio merged commit 4a570ff into jupyterlab:main Aug 12, 2026
112 checks passed
@lumberbot-app

lumberbot-app Bot commented Aug 12, 2026

Copy link
Copy Markdown

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 4.6.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 4a570ff048e69fa6e7a8f4a8e8464ea9c7143421
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #19307: Update `@jupyter/eslint-plugin` to v1.1, adopt new rules'
  1. Push to a named branch:
git push YOURFORK 4.6.x:auto-backport-of-pr-19307-on-4.6.x
  1. Create a PR against branch 4.6.x, I would have named this PR:

"Backport PR #19307 on branch 4.6.x (Update @jupyter/eslint-plugin to v1.1, adopt new rules)"

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 Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

@github-project-automation github-project-automation Bot moved this from Todo to Done in CFP '25 tracking Aug 12, 2026
@krassowski
krassowski deleted the update-eslint-plugin-add-new-rules branch August 12, 2026 13:56
krassowski added a commit that referenced this pull request Aug 13, 2026
#19315)

Backport PR #19307 on
branch 4.6.x (Update @jupyter/eslint-plugin to v1.1, adopt new rules)
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants