feature/destroy

[TrevorBurgoyne]

Destroy

Description

• Memory leak fix on teardown. Bitmask annotations attach a decoded pixel Uint8Array (_mask) and a tinted stencil canvas (_mask_render) to each annotation object. These persisted after remove_listeners(), and consumers that rebuild ULabel per navigation could accumulate multi-GB retained heap. Changes:
  • New destroy() method on ULabel. Idempotent; releases per-annotation bitmask caches, empties action/undo streams, breaks toolbox back-references, clears the resize-observer array, and wipes the container DOM. Callers should prefer destroy() over remove_listeners() going forward.
  • destroy_annotation_context() now also drops _mask / _mask_render / _bitmask_box_hint on the destroyed annotation, so set_annotations() also frees mask memory when it recycles contexts.
  • New auto_destroy_on_detach config option (default true). When enabled, ULabel installs a MutationObserver on the container's root and calls destroy() automatically after the container leaves the DOM.
  • set_annotations(), get_annotations(), and redraw_all_annotations() now short-circuit with a warning if called after destroy().
  • Fixed a latent typo in set_annotations() that wrote to a non-existent undo_stack property instead of clearing the real undone_stack.
  • remove_ulabel_listeners() now scopes its .id_dialog cleanup to the instance's container, so tearing down one ULabel no longer strips id-dialog handlers from siblings on the same page.

PR Checklist

• Merged latest main
• Version number in package.json has been bumped since last release
• Version numbers match between package package.json and src/version.js
• Updated documentation if necessary (currently just in api_spec.md)
• Added changes to changelog.md

Breaking API Changes

Yes, ULabel will now auto-call its destroy() method once its container leaves the DOM. To opt out of that behavior, set auto_destroy_on_detach config option to false

[Copilot]

Pull request overview

Adds a comprehensive teardown path to prevent retained-memory growth (especially from bitmask annotation caches) in SPA-style lifecycles, and documents/tests the new behavior.

Changes:

• Introduces ULabel.destroy() (idempotent) to release per-annotation bitmask caches, clear action streams, break toolbox back-references, clear observers/timers, and wipe container DOM.
• Adds auto_destroy_on_detach (default true) to automatically call destroy() when the container is removed from the DOM, plus post-destroy guards for key methods.
• Updates docs/types/versioning and adds teardown-focused tests.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/teardown.test.js	Adds unit tests for destroy() behavior and the auto-detach teardown observer.
src/index.js	Implements destroy(), auto-detach observer installation, bitmask cache release on context destruction, post-destroy guards, and fixes undone_stack clearing.
src/listeners.ts	Scopes .id_dialog listener cleanup to the instance container (instead of global).
src/configuration.ts	Adds auto_destroy_on_detach configuration default.
index.d.ts	Exposes destroy() and is_destroyed in public TypeScript typings.
api_spec.md	Documents destroy() and auto_destroy_on_detach.
src/version.js	Bumps ULABEL_VERSION to 0.26.0.
package.json	Bumps package version to 0.26.0.
CHANGELOG.md	Adds 0.26.0 entry describing teardown/memory-leak fixes.
.github/tasks.md	Removes prior task checklist content (file left mostly empty).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

src/listeners.ts:755

• Scoping only the .id_dialog removal does not preserve sibling ULabel instances: the immediately preceding $(document).off(".ulabel") and $(window).off(".ulabel") still remove every sibling's global handlers because all instances share the same namespace. Since automatic teardown now calls this path, detaching one instance breaks keyboard, mouse, and unload behavior in the others. Bind and remove all handlers with an instance-specific namespace or stored handler references.

    // Scope id_dialog cleanup to this instance so a sibling ULabel keeps its handlers.
    const container_id = ulabel?.config?.container_id;
    if (container_id) {
        $(`#${container_id} .id_dialog`).off(ULABEL_NAMESPACE);

src/index.js:776

• The observer is installed only from after_init(), which runs after asynchronous image decoding and after the user callback. If the container is removed during initialization—or by that callback—_install_auto_destroy_observer() sees a disconnected/missing container and returns, so the default auto-destroy path is never armed. Install observation before the first asynchronous initialization gap/user callback, or explicitly cancel and destroy initialization when the container disconnects.

        // Install the opt-in auto-teardown observer once the container is in the DOM.
        if (this.config?.["auto_destroy_on_detach"]) {
            this._install_auto_destroy_observer();

package.json:4

• The package version is now 0.26.0, but package-lock.json:3 and package-lock.json:9 still identify the root package as 0.25.1. Keep the lockfile's root metadata synchronized so installs do not rewrite tracked state and package metadata remains consistent.

  "version": "0.26.0",

.github/tasks.md:3

• This change empties the task tracker and removes the still-incomplete Phase 6 item without adding any tracking for the teardown work. Preserve existing unfinished work and record this PR's tasks/status instead of leaving .github/tasks.md blank.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

src/listeners.ts:755

• The cleanup remains non-instance-safe: every ULabel registers its document/window handlers under the same .ulabel namespace (listeners.ts:415-448, 721-738), and lines 750-751 remove that entire namespace. Therefore auto-destroying one container still disables mouse, keyboard, and beforeunload behavior for all sibling ULabels even though the dialog cleanup is scoped. Use an instance-unique namespace or retain each handler and unregister that exact handler.

    // Scope id_dialog cleanup to this instance so a sibling ULabel keeps its handlers.
    const container_id = ulabel?.config?.container_id;
    if (container_id) {
        $(`#${container_id} .id_dialog`).off(ULABEL_NAMESPACE);

src/index.js:7100

• This guard only covers calls that begin after destruction. set_annotations() yields at wait_for_render(), so an auto- or manual destroy() during that wait clears the instance, after which this method resumes, repopulates annotations, and eventually dereferences the now-null toolbox. Recheck is_destroyed immediately after the await before performing teardown/reinitialization.

        if (this.is_destroyed) {
            log_message("set_annotations called on a destroyed ULabel instance", LogLevel.WARNING, true);
            return;
        }

src/index.js:783

• The observer is installed only from after_init(), but ulabel_init() invokes the user callback before after_init() (src/initializer.ts:238-240). If that callback synchronously navigates or removes the container—a normal SPA initialization pattern—this method sees a disconnected node and returns without ever observing or destroying the instance. Install the observer before invoking the callback (ideally immediately after the owned container is created) so the default auto-teardown guarantee also covers initialization and callback-driven unmounts.

        // Install the opt-in auto-teardown observer once the container is in the DOM.
        if (this.config?.["auto_destroy_on_detach"]) {
            this._install_auto_destroy_observer();

src/index.js:169

• destroy() clears only the toast timeout and overlay value, but queued animation-frame work is still live. For example, schedule_continue_move() (lines 5657-5667) can run after annotations/contexts are emptied, and AnnotationList has a queued callback that will run after its ulabel back-reference is set to null. These callbacks can mutate a destroyed instance or throw. Track/cancel pending frame IDs, or make every queued callback short-circuit when destruction begins, before clearing the referenced state.

        // 3. Any pending toast/interaction timers hold `this` in their closure.
        if (this.annotation_navigation_toast_timeout !== null) {
            clearTimeout(this.annotation_navigation_toast_timeout);
            this.annotation_navigation_toast_timeout = null;
        }
        // In-progress bitmask move snapshot canvas (skipped by end_bitmask_move on interrupt).
        if (this.state) {
            this.state["bitmask_move_overlay"] = null;
        }

src/configuration.ts:279

• The new public option is absent from ULabelConstructorArgs in index.d.ts, so TypeScript consumers cannot pass { auto_destroy_on_detach: false } to new ULabel(...) without an excess-property error even though opting out is the documented breaking-change escape hatch. Add the optional boolean to the public constructor argument type and cover it in the declaration test.

    public auto_destroy_on_detach: boolean = true;

[Copilot]

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

src/index.js:169

• This only releases the overlay; it leaves state.last_move and active subtask interaction flags intact. If schedule_continue_move() already queued its rAF, the callback sees is_in_move still true after annotations were emptied and calls continue_move(), which then operates on missing annotations and can throw; the queued closure also retains the instance until that frame runs. Track/cancel the rAF and clear the pending event plus active interaction state during teardown.

        // In-progress bitmask move snapshot canvas (skipped by end_bitmask_move on interrupt).
        if (this.state) {
            this.state["bitmask_move_overlay"] = null;
        }

src/initializer.ts:244

• The observer is armed only after first_bg_img.decode() and the rest of async initialization. If navigation removes the container while the image is still decoding, no observer sees that removal and initialization continues, registering listeners and retaining state on a detached tree. Install the observer immediately after _owned_container is captured and add an is_destroyed check after the decode await so initialization cannot resume after teardown.

    // Arm auto-teardown before the user callback so a callback that synchronously removes
    // the container still triggers destroy(). No-op when the flag is disabled.
    ulabel._install_auto_destroy_observer();

src/initializer.ts:248

• A ready callback can legitimately call the new public destroy() method. In that case destroy() sets this.toolbox = null, but the next line unconditionally calls after_init(), which iterates this.toolbox.items and throws. Skip post-initialization work when the callback destroyed the instance.

    // Call the user-provided callback
    user_callback();
    ulabel.after_init();

.github/tasks.md:3

• This removes the unfinished Phase 6 checklist and leaves the required task tracker empty, without recording the teardown work. Preserve outstanding tasks and add checkbox entries for this PR so .github/tasks.md continues to track work as required.

## Tasks

[Copilot]

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/index.js:159

• The PR description says id-dialog cleanup is now instance-scoped, but this still calls remove_ulabel_listeners(), whose cleanup remains global at src/listeners.ts:748-750 ($(document).off(...), $(window).off(...), and $(".id_dialog").off(...)). Consequently, destroying one instance still removes sibling handlers, so the stated fix is absent. Implement instance-specific namespaces/scoped dialog selection or remove that claim.

        // 2. Existing listener / observer cleanup.
        this.remove_listeners();

src/index.js:822

• This early return loses auto-teardown after a cross-root reparent. If the container moves from one connected Document/ShadowRoot to another during the grace frame, isConnected is true, but this observer remains attached to the old root; a later removal from the new root produces no callback and the instance leaks. When a connected container's getRootNode() changes, retarget the observer to that root (or explicitly define a root transfer as teardown).

            if (container.isConnected) return;
            requestAnimationFrame(() => {
                const s = weak.deref();
                if (!s || s.is_destroyed) return;
                if (container.isConnected) return;