Skip to content

Conversation

MikeBellika
Copy link
Contributor

@MikeBellika MikeBellika commented Aug 19, 2025

πŸ”— Linked issue

Should resolve #517 and #1367

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR adds a scoped option to mountSuspended. When this option is enabled, state isn't shared between components. This is achieved by using an effectScope and a "global" cleanup function. This is similar to what is done in https://github.com/nuxt/test-utils/blob/main/src/runtime-utils/render.ts

The watcher cleanup tests are adapted from #1367. The useCounter tests are adapted from https://github.com/MikeBellika/component-not-unmounted-test-utils/blob/main/test/unit/repro.test.ts

- Wrap component setup and Nuxt root setup in effect scopes
- Add global cleanup mechanism to prevent state persistence between
tests
- Ensure watchers and reactive effects are properly disposed
- Improves test isolation for mountSuspended components
Copy link

pkg-pr-new bot commented Aug 19, 2025

Open in StackBlitz

npm i https://pkg.pr.new/@nuxt/test-utils@1389

commit: 236c77b

@MikeBellika MikeBellika marked this pull request as ready for review August 19, 2025 13:32
@MikeBellika MikeBellika requested a review from danielroe as a code owner August 19, 2025 13:32
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

The difference is that render only expects a single component to render, but you might mount more than one component in a test.

Additionally, some people mount a component and then have several assertions (or 'it' tests) about it.

I think it's better to require people to unmount components manually, which is also (afaik) what you have to do if you are using @vue/test-utils.

@MikeBellika
Copy link
Contributor Author

The difference is that render only expects a single component to render, but you might mount more than one component in a test.

Additionally, some people mount a component and then have several assertions (or 'it' tests) about it.

I think it's better to require people to unmount components manually, which is also (afaik) what you have to do if you are using @vue/test-utils.

I think it's unintuitive that state is shared between tests and wouldn't expect the order of calling tests to influence the result. If people do multiple assertions without mounting the component again I think they're writing their tests wrong.

@danielroe
Copy link
Member

danielroe commented Aug 19, 2025

A test library like this should be unopinionated about how people do their mounting of components.

I think we should follow the example of @vue/test-utils when it comes to mountSuspended...

Some code that would be broken:

it('implicitly unmounts comp1, unexpectedly', async () => {
  const comp1 = await mountSuspended(/* */)
  const comp2 = await mountSuspended(/* */)
})

@MikeBellika
Copy link
Contributor Author

Investigation

Just checked in vue: https://github.com/MikeBellika/vue-test-mount-repro
The tests pass but the console logs in HelloWorld.vue aren't reached. So I guess the components aren't unmounted between tests and effectScope isn't used either.

Just tried to run this test with my change:

  it('can mount twice', async () => {
    useCounterMock.mockRestore()
    const comp1 = await mountSuspended(GenericStateComponent)
    console.log('comp1 mounted')
    const comp2 = await mountSuspended(GenericStateComponent)
    console.log('comp2 mounted')
    expect(comp1.text()).toMatchInlineSnapshot('"Zero or negative count"')
    expect(comp2.text()).toMatchInlineSnapshot('"Zero or negative count"')
  })

I also updated GenericStateComponent

<script setup lang="ts">
const { isPositive } = useCounter()
const { data } = useLazyAsyncData(async () => {
  return isPositive() ? 'positive' : 'zero-or-negative'
})

onUnmounted(() => {
  // not called
  console.log('unmounted')
})

onScopeDispose(() => {
  console.log('disposed')
})
</script>

<template>
  <div v-if="data === 'positive'">
    Positive count
  </div>
  <div v-else>
    Zero or negative count
  </div>
</template>

The output is:

stdout | tests/nuxt/mount-suspended.spec.ts > composable state isolation > can mount twice
disposed

stdout | tests/nuxt/mount-suspended.spec.ts > composable state isolation > can mount twice
comp1 mounted
disposed

stdout | tests/nuxt/mount-suspended.spec.ts > composable state isolation > can mount twice
comp2 mounted

So the components aren't unmounted within the test but the scopes do seem to be disposed. I don't know what effect this will have.

My opinion

As a user of nuxt test-utils the behaviour of different invocations of mountSuspended reusing the same state is unexpected. I don't think I'm alone in this and IMO #517 and #1367 stem from this issue.

My mental model of mountSuspended is that it mounts a component in isolation, with nothing shared between other tests. From my tests it seems like this is how it works in vue but I could easily be mistaken.
If this is the wrong way to think about it, I think a section about test isolation should be added to the documentation. Maybe it could have some tips about using component.unmount() and clearNuxtData() after each test.

Alternatively what about an isolate option on mountSuspended?

@danielroe
Copy link
Member

I'd be very happy to support an option (maybe dispose?)

however, isolated is a bit misleading as there's a single nuxtApp instance that persists between tests, and things like state (via useState) doesn't reset. we already call this out IIRC but maybe we can make it clearer

@MikeBellika
Copy link
Contributor Author

What about scoped as a name for the option?


I'm not too strong on Nuxt internals but I think there are two related issues that I have gotten a bit mixed up.

  1. State like useState is shared between tests. This is mentioned in the docs but could be made more explicit
  2. this test fails. I think it's because the function in useLazyAsyncData is reused between tests. To me this feels like a foot gun.

What do you think about:

  1. Updating docs to include a section about what state is shared between tests and how to ensure test isolation
  2. Besides adding an option on mountSuspended, adding an option to create a whole new nuxtApp instance for each test.

I can do 1. but I wouldn't know where to start with 2.

Regarding 1. the docs do mention that a global nuxt instance will be used but I think it could be made more clear:

When you run your tests within the Nuxt environment, they will be running in a happy-dom or jsdom environment. Before your tests run, a global Nuxt app will be initialized (including, for example, running any plugins or code you've defined in your app.vue). This means you should take particular care not to mutate the global state in your tests (or, if you need to, to reset it afterwards).

Source: https://nuxt.com/docs/4.x/getting-started/testing#running-tests

@MikeBellika
Copy link
Contributor Author

@danielroe I've updated the PR to add a scoped option

@danielroe danielroe changed the title Improve isolation between tests feat(runtime-utils): add scoped option to automatically cleanup components Sep 6, 2025
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.

Clearing state before/after each test
2 participants