Skip to content

fix(scroller): don't throw from destroy() before an element is set - #318

Merged
dadajam4 merged 1 commit into
mainfrom
fix/scroller-destroy-without-element
Oct 15, 2026
Merged

dadajam4 merged 1 commit into
mainfrom
fix/scroller-destroy-without-element

Conversation

@dadajam4

Copy link
Copy Markdown
Owner

Summary

Two teardown bugs in @fastkit/scroller, both reachable from @fastkit/vue-scroller.

destroy() threw missing element when no element had been set

useScrollerControl({ el: 'self' }) creates the Scroller without an element and only sets it in onMounted, but always calls destroy() in onBeforeUnmount. When the component sits inside a <Suspense> boundary that is still pending, navigating away unmounts it before it ever mounted, and destroy() → _scrollEnable() → _checkElement() threw. In an app that routes errors raised during navigation to an error page, this showed an error page.

destroy() now only re-enables scrolling when an element exists.

start() attached listeners after destroy()

Setting an element kicks off start(), which awaits ready() before attaching listeners. Destroying the instance in between made start() resume on a destroyed instance and fail with an unhandled rejection (Cannot read properties of undefined (reading 'addEventListener')). start() now returns if the instance was destroyed while waiting.

Tests

  • packages/scroller/src/__tests__/destroy.client.spec.ts: destroy() without an element; destroy() right after setting an element (captures unhandled rejections).
  • packages/vue-scroller/src/composables/__tests__/scroller.client.spec.ts: unmounting a useScrollerControl({ el: 'self' }) component while its <Suspense> boundary is pending.

All three fail before the fix and pass after it.

Release

Changeset: @fastkit/scroller patch (@fastkit/vue-scroller follows via internal dependency update).

🤖 Generated with Claude Code

`Scroller.destroy()` called `_scrollEnable()` unconditionally, which threw
`missing element` when no element had been set. `useScrollerControl({ el:
'self' })` only sets the element in `onMounted`, so unmounting it while a
`<Suspense>` boundary is still pending (e.g. navigating away during the
initial data load) threw from `onBeforeUnmount`.

Also stop `start()` from attaching listeners after the instance was
destroyed while it awaited `ready()`, which surfaced as an unhandled
rejection when destroying right after setting an element.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6c047e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@fastkit/scroller Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dadajam4
dadajam4 merged commit 2784c73 into main Oct 15, 2026
1 check passed
@dadajam4
dadajam4 deleted the fix/scroller-destroy-without-element branch October 15, 2026 04:58
@github-actions github-actions Bot mentioned this pull request Oct 15, 2026
dadajam4 added a commit that referenced this pull request Oct 16, 2026
…322)

Releasing @fastkit/scroller@1.0.1 (#318) did not release
@fastkit/vue-scroller, since its `^1.0.0` range already admitted the new
version. Add a patch changeset so its published range becomes `^1.0.1` and
updating @fastkit/vue-scroller brings the fix along.

Document when a fix in a dependency needs a changeset for its dependents
too.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.

1 participant