Desktop: Fixes #11129: Improve performance by allowing note list background timers to be cancelled #11133
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.
Summary
This pull request cancels note list
waitForElement
intervals when note list items unmount. This should fix #11129.Testing plan
With this PR
This pull request includes automated regression tests. At present, they do not verify that the background
waitForElement
interval stops. However, they do verify:useRootElement
(the hook that wrapswaitForElement
) can find an element if it already exists in the DOM.useRootElement
re-searches for an element if theelementId
it's given changes (tests the case that no result is initially found).Additionally, manual testing has been done (on Fedora 40):
Open the Electron dev tools and modify
packages/lib/dom.js
.diff
Press ctrl-s to apply the changes.
Clear the console with
console.clear()
Scroll quickly through a long note list (style: detailed)
Repeat step 4 for style: compact.
Verify that no
long!
warnings are present in the console.Comparison with before this PR
I then compared with a version of Joplin from before this change:
Open a copy of Joplin from before this change (v3.1.16).
Apply a the change from the previous testing steps to
packages/lib/dom.js
through the Electron dev tools.diff
Press ctrl-s to apply changes.
Scroll quickly through a long note list.
Verify that a large number of
long!
messages are logged and that the number of such log statements is increasing rapidly: