Skip to content

feat: onSlideEnter/Leave after mounted + pass idx#2540

Merged
antfu merged 3 commits into
slidevjs:mainfrom
maxkurze1:main
Apr 28, 2026
Merged

feat: onSlideEnter/Leave after mounted + pass idx#2540
antfu merged 3 commits into
slidevjs:mainfrom
maxkurze1:main

Conversation

@maxkurze1

Copy link
Copy Markdown
Contributor

This PR refactors the onSlideEnter and onSlideLeave functions providing 3 contributions

1. replace watchEffect with watch

Using watchEffect for the definition of onSlideEnter would cause unintended invocations triggered by arbitrary references used inside the callback. I would consider this a bug in the previous implementation, which was fixed by replacing it with a plain watch which only triggers based on the explicitly provided dependencies.

To retain the initial invocation on slide load I configured { immediate: true }, though I think it is not necessary anymore since the hook should always be set up before mounting and thus will be triggered by ismouted.value = true anyhow.

2. make sure onSlideEnter is (always) called after onMounted

AFAICT this guarantee only provides benefits. I would argue that most of the use cases of onSlideEnter/onSlideLeave want to do some setup/DOM configurations and are thus expecting this ordering already, even though it is currently not guaranteed.

The new version of onSlideEnter uses a dependency on ismounted to get as close as possible to the desired ordering. However, I am still not entirely sure whether the ordering is actually guaranteed now, as the Vue documentation is not as explicit as I would like it to be.

Maybe one should consider adding nextTick? But I'd argue that it is already good enough.

3. passing the current / previous slide number to onSlideEnter/onSlideLeave

In my personal developments I found these arguments to be quite handy, so I decided to include them here as well. Sure, one could argue that one of the parameters is useless as it just mirrors $page.value, but that didn't really matter to me.

@netlify

netlify Bot commented Apr 14, 2026

Copy link
Copy Markdown

Deploy Preview for slidev ready!

Name Link
🔨 Latest commit 56a705f
🔍 Latest deploy log https://app.netlify.com/projects/slidev/deploys/69e08c75da901c000817566f
😎 Deploy Preview https://deploy-preview-2540--slidev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@pkg-pr-new

pkg-pr-new Bot commented Apr 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

@slidev/client

npm i https://pkg.pr.new/@slidev/client@2540

create-slidev

npm i https://pkg.pr.new/create-slidev@2540

create-slidev-theme

npm i https://pkg.pr.new/create-slidev-theme@2540

@slidev/parser

npm i https://pkg.pr.new/@slidev/parser@2540

@slidev/cli

npm i https://pkg.pr.new/@slidev/cli@2540

@slidev/types

npm i https://pkg.pr.new/@slidev/types@2540

commit: 56a705f

@antfu antfu changed the title refactor: onSlideEnter/Leave after mounted + pass idx feat: onSlideEnter/Leave after mounted + pass idx Apr 28, 2026
@antfu
antfu merged commit 9c171ba into slidevjs:main Apr 28, 2026
20 checks passed
// Note: using `ismounted` to make sure `onSliderEnter` is only called after `onMounted`
watch(() => [$nav.value.currentSlideNo, ismounted.value], ([to, mounted], from) =>
$page.value === to && mounted && cb(to, from ? from[0] as number : undefined), { immediate: true, flush: 'post' })
tryOnMounted(() => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using on tryOnMounted or even the normal onMounted seems more clean to me, I also considered this. The problem being that if you have a slide with both onMounted and onSlideEnter on it, their order isn't guaranteed. In this case, as onSlideEnter basically just desugars to onMounted, their ordering in the file determines their invocation order.

E.g. the following code invokes onSlideEnter first and only after it onMounted:

<script setup lang="ts">
import { onMounted } from "vue"
import { onSlideEnter, onSlideLeave } from "@slidev/client";

onSlideEnter(() => {
  console.log("enter")
})
onMounted(() => {
  console.log("mount")
})
</script>

I've instead tried, the best I could, to make sure that onSliderEnter is always called after onMounted. In my implementation that was caused by the ismounted value which was updated during the mount but for some Vue internal reason it seemed to trigger the watch hook only after all mounts executed.

Unfortunately, I couldn't find a cleaner / more reliable way to force this ordering. I also didn't have the time yet to look into Vue and verify if this approach actually works out all the time. My intuition was that it seemed reasonable that the watch hook is only checked/triggered after another "internal tick" or something like this and during testing I never observed a different behavior.

ryan-williams added a commit to Open-Athena/slidev that referenced this pull request May 8, 2026
Notable upstream features pulled in: laser pointer (slidevjs#2510), build
`--router-mode` flag (slidevjs#2548), `onSlideEnter`/`onSlideLeave` lifecycle
hooks (slidevjs#2540), markdown-it-github-alerts (slidevjs#2547), bluesky embed
component, named v-click animation presets (slidevjs#2501), comark 0.3.2 →
0.3.3, deps update.

Conflict resolutions:

- `packages/client/composables/useDragElements.ts`: dropped the
  upstream's markdown-rewrite path that we deliberately removed in
  Stage 3a — persistence now flows through the SQLite event log →
  `commit-yaml` button. Removed unused `RE_NEWLINE` / `RE_POS_ATTR`
  too.
- `packages/client/builtin/VDrag.vue`: kept our richer body-drag /
  threshold / pinch-guard implementation; theirs was a single line.
- `packages/client/internals/NavControls.vue`: merged our
  `showHistoryDrawer` import with their `cursorStyle` / `togglePresenterCursor`.
- `packages/client/pages/play.vue`: kept both the file-drop overlay
  and the new `<LaserPointer />`.
- `packages/parser/src/config.ts`: kept both `draggableImages: false`
  and `clickAnimation: ''`; added `publish: {}` (now required by the
  HeadmatterConfig→SlidevConfig promotion).
- `cypress/fixtures/basic/slides.md`: appended the new animation-
  preset pages 19–21 after our drag-test pages 14–18.
- Upstream flattened `packages/slidev/node/syntax/markdown-it/*.ts` →
  `packages/slidev/node/syntax/*.ts`. Moved our
  `markdown-it-draggable-image.ts` to `syntax/draggable-image.ts` and
  rewired the `md.use(...)` registration in the new `syntax/index.ts`.
- `packages/client/internals/DragControl.vue`: added `as const` to a
  few `pointerEvents: 'auto'/'none'` literals that Vite 8 / Vue 3.6
  type-narrows more strictly.
- `packages/slidev/node/vite/{state,upload}.ts`: `Connect.ServerResponse`
  was removed in Vite 8; switched to `import type { ServerResponse }
  from 'node:http'`.

Tests: 14/15 suites pass; the lone failure is `integration.test.ts`'s
PlantUml-code snapshot, which fails identically on upstream `v52.15.0`
itself (platform-sensitive zlib output) — pre-existing, not our
regression.

Co-Authored-By: Claude Opus 4.7 (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.

3 participants