feat(app): support app level effect scope

[Mini-ghost]

close #8787

[antfu]

In general this feature makes sense to me.

[posva]

I remember we intentionally stuck to only inject and provide.

That being said I think this is really useful and did propose it too but wanted to avoid adding too much right away to runWithContext() 🤪. The router and pinia could benefit from this too.

Could you add a test?
This could probably be moved to createApp() too

[posva]

This would potentially also enable an app.onUnmount() with just one line

app.onUnmount = (fn) => app.runWithContext(() => onScopeDispose(fn))

• feat(runtime-core): app.onUnmount() for registering cleanup functions (close: #4516) #4619
• [plugin]How can a plugin know app mount/unmount ? #4516

[Mini-ghost]

Thank you for your feedback. I have a couple of questions I would like to confirm:

1. In feat(runtime-core): app.onUnmount() for registering cleanup functions (close: #4516) #4619, the implementation of adding app.onUnmount functionality is located in runtime-core/src/apiCreateApp.ts instead of runtime-dom/src/index.ts. I'm not sure if this PR should follow this approach.

2. Currently, Vue internally utilizes the new EffectScope(true) approach. Would it be advisable for this PR to adopt a class-based approach instead of using effectScope(true)?

Thank you! Your invaluable guidance means a lot to me.

[posva]

1. I think yes, I don't see why this should be in runtime-dom instead of core
2. I'm not sure but I don't think it matters

[Mini-ghost]

I have moved the implementation to runtime-core and added a test case. Could you please review?

[posva]

This looks good to me. It would be nice to have feedback from @yyx990803 to know if he can think of any reason not to have an effect like this.

One thing work noting is that any user doing this:

const myEffect = effectScope()

myEffect.run(() => app.runWithContext(fn))

Will have to rewrite it to

app.runWithContext(() => myEffect.run(fn))

Which is technically a breaking change. I haven't checked the usage of this in the wild though but I will have to update pinia before this comes out as I currently have the code shown above 😆

[posva]

This makes me wonder if the effectScope should be in app context 🤔

[Mini-ghost]

@posva In the latest commit (a7141d4), I added scope to the app. Currently, I have handled it as a private property. Should it be made public for user access?

[skirtles-code]

I'm wondering about whether this could cause memory leaks, and whether that matters.

I've put together an example using Vue Router:

• Playground - main
• Playground - this PR

Steps to reproduce:

1. Navigate between the two routes a few times by clicking the links.
2. Take a Heap Snapshot (make sure you select the main VM, not the workers).
3. Filter the snapshot by the class VTrackLeak.
4. On main you'll just find a single instance, but with this PR you'll find an extra instance per navigation.

The key parts of the code are:

router.beforeEach(() => {
  const { doubled } = useBlah()

  if (doubled.value) {
    // ...
  }
})

with:

function useBlah() {
  const raw = ref(1)
  const doubled = computed(() => raw.value * 2)

  raw.marker = new VTrackLeak()

  return { raw, doubled }
}

The callback for beforeEach is wrapped in app.runWithContext by Vue Router. That will capture the effect for the computed in useBlah, preventing it from being GCed until the application is unmounted, which may never happen.

[posva]

@skirtles-code I'm surprised they don't both leak. I would consider that usage wrong, but it does show a distinction between being able to inject and being able to use reactive effects is important. I think it makes more sense to keep things separate and adding an effectScope to the app would allow to have app.scope.run() to track reactive effects, keeping both APIs separate and forcing libraries like the router to use any they feel are relevant.

[Mini-ghost]

@skirtles-code

I studied the issue and found that it is likely caused by the ReactiveEffect inside computed. In the example, if computed is not used, the issue does not occur.

I am unsure if the usage in the example is intended. Should we discourage users from using ref this way? Also, the example causes memory leaks in Nuxt too.

I am not sure how to fix this issue. I would greatly appreciate any guidance or suggestions.