vuejs / rfcs

RFCs for substantial changes / feature additions to Vue core
4.86k stars 548 forks source link

Reactivity's `effectScope` API #212

Closed antfu closed 3 years ago

antfu commented 3 years ago

Rendered

CyberAP commented 3 years ago

Not strictly against this proposal, but I don't completely understand the motivation point. I have several questions if you don't mind:

  1. What are the real word applications of this? How frequently does this happen\might happen?
  2. What kind of problems do require that style of authoring reactivity?
  3. What are the challenges of collecting stop handlers? (See example below)
  4. How wrapping this in a callback scope would solve this? (maybe creating a special higher-order watcher could work in that case)
  5. How is it going to affect Composition API? In particular how it would interplay with the existing setup cleanup behaviour?

Also the code could be authored in a more simplified way, which might help with the issue:

const disposables = []

const counter = ref(0)
const doubled = computed(() => counter.value * 2)

disposables.push(() => stop(doubled.effect))

disposables.push(
  watchEffect(() => {
    console.log(`counter: ${counter.value}`
  }))
)

disposables.push(watch(doubled, () => console.log(double.value)))

disposables.forEach(f => f())
disposables = []
antfu commented 3 years ago
  1. If you like to make your own a framework. For example, @vue/lit, it does not handle effects in the component instance lifecycles, which will cause mem leakage if you try to mount and unmount the sub-component multiple times. In order to solve this, you will need to implement recordInstanceBoundEffect for your framework, which is not an easy & simple task from the outside. And also they will lose the ability to be reused interchangeably with Vue's ecosystem.

  2. Clean up side-effects, preventing mem leak.

  3. I don't see your code being "simplified". A huge amount of disposables.push( will make the logic hard to read. And if you take the following example, things will become more complex.

import { useMyLogic1, useMyLogic2 } from './utils'
import { useLogicFromLibrary } from 'xxx-compsoable'

const disposables = []

// each your custom logic should do the collecting in order to be disposed by the user side
const { state1, disposables: disposables1 } = useMyLogic1()

disposables.push(...disposables1)

const { state2, disposables: disposables2 } = useMyLogic2()

disposables.push(...disposables2)

// it does not collect the disposables, how do you dispose it? (note this is ok in setup)
const state3 = useLogicFromLibrary()
  1. Similar to what setup does.

  2. This actually more like "extracting the setup functionality, generalized and abstract into @vue/reactivity", and the goal is to be reused in setup() as well.

Thanks for the feedback, and hope I answered your concerns.

CyberAP commented 3 years ago

And also they will lose the ability to be reused interchangeably with Vue's ecosystem.

Why is that important?

This actually more like "extracting the setup functionality, generalized and abstract into @vue/reactivity", and the goal is to be reused in setup() as well.

This resembles a solution to the specific component-based framework problems, but outside of these frameworks what would be the case when you need to do a lot of effect creation and disposal? Cases when we need to do a lot of cleanup are of the most interest to me.

antfu commented 3 years ago

Why is that important?

It's not that important, but why not if you can? Like I might be able to set up an alias and make VueUse work for @vue/lit, so I don't need to reinvent another set of utils that doing the same thing for it or yet another framework. They could even contribute back to Vue's ecosystem.

what would be the case when you need to do a lot of effect creation and disposal?

Even outside of Vue's instance this could be a problem. Maybe I would like to build a state management system my own, I might need some side-effects and to be cleaned afterward. I need to collect them.

More widely, I can use it in VSCode extension (I already did), I need to watch on config changes, editor changes, file system changes... I would also need to clean them up when I no longer need some. I might use it in a server to do something like HMR which is based on local file changes. When some file gets deleted, I also need to dispose the fs watchers.

There are so many possibilities for this great reactivity system. And we just got it for only a while now.

jods4 commented 3 years ago

I think this is nice because the concept is already in Vue but not cleanly abstracted. The fact that Vue itself needs this shows that there is a use case. It's already in the framework, so instead of making a special case just for components, why not extract a reusable API?

It won't get much use when @vue/reactivity is used in UI though, as Vue already provides the very useful Component == reactive scope, but it could be handy if you're doing something that is not based on components. And it's impossible to do from outside, without heavy hacks.

In particular how it would interplay with the existing setup cleanup behaviour?

If this is accepted, the existing cleanup should 100% be based on this new API.

This would give us an easy way to perform something that is really hacky today: escape component lifetime (some users have already asked how to do this in issues during the beta). Opening a nested scope would escape the component scope:

setup() {
  // Stopped when component is unmounted
  watch(...)
  // This one won't be stopped automatically
  effectScope(() => {
    watch(...)
  })
}

I'm wondering if this API could have more advanced uses by exposing the captured effects, maybe through an effects property on the returned stop function? To be honest I have no use case for that right now. I was thinking along the lines being able to force a refresh of everything in a scope, or maybe for tooling / debuggers, or detecting that a piece of code is effect-free (leading to static optimizations?).

privatenumber commented 3 years ago

Great proposal! I love to see support for making @vue/reactivity work more conveniently as a standalone package, especially since being used independently of Vue is advertised as a pretty big feature of Vue 3 (eg. brought up multiple times in the Vue 3 announcement keynote).

@jods4

If @vue/runtime-dom setup is refactored to use effectScope behind the scenes, I think any new effectScope instantiated inside setup will also be cleaned up as per the Nested Scopes spec.

I think this can still be worked around though, simply by instantiating effectScope outside of setup, but if users really want to create a scope unbound to the component lifespan, I think they'll always find a way.

jods4 commented 3 years ago

If @vue/runtime-dom setup is refactored to use effectScope behind the scenes, I think any new effectScope instantiated inside setup will also be cleaned up as per the Nested Scopes spec.

That's a good point. I think Components don't work like that today -- they only stop effects that were created inside them, not their children (which get stopped by unmounting the whole tree, but that's not the same mechanism).

This is incentive to change the spec slightly so that nested effectScopes are not stopped by the outer one, so that Components can be implemented on top of that spec. If there is a good use-case for the opposite behavior, then maybe this could be controlled by an optional flag passed to effectScope (if there is no use case today, such a flag can always be added later).

IMHO not implementing components on top of this spec feels bad because:

think this can still be worked around though, simply by instantiating effectScope outside of setup, but if users really want to create a scope unbound to the component lifespan, I think they'll always find a way.

It's been asked already and I know of at least two ways you can achieve this today... but both are hacky. It'd be a nice bonus if this spec allowed that in a clean way through a documented API.

antfu commented 3 years ago

Update: effectScope now returns the scope instance instead of the stop handler. Providing more flexibility and make the stop api more general.

const scope = effectScope(() => {
  const doubled = computed(() => counter.value * 2)

  watch(doubled, () => console.log(double.value))

  watchEffect(() => console.log('Count: ', double.value))
})

// to dispose all effects in the scope
stop(scope)
unbyte commented 3 years ago

I really like its concept however I have doubts about it šŸ™‹ā€ā™‚ļø

  1. in most cases computed values would be used as meaningful variables, and closures complicate its use. and it will lead to some bug codes if computed values can be exported from the closure, since stopping effectScope breaks reactivity of exported computed values that may still be used without notification.

  2. if stopping a parent scope also stops all child scopes, how to create a standalone scope inside a scope (it's useful)? and if can by certain api, how to stop this standalone scope simply since it's in a nested closure?

CyberAP commented 3 years ago

What do you think of attaching effects to the scope after scope creation? That might come handy for effects that are initiated asynchronously.

const scope = effectScope()

setTimeout(() => {
  scope(() => {
    watch(...)
  })
})

Or attach by pushing:

const scope = effectScope()

setTimeout(() => {
  scope.push(watch(...))
})

If the scope has been destroyed the effect should not be created.

antfu commented 3 years ago

@unbyte

and it will lead to some bug codes if computed values can be exported from the closure

Can't image it, can you provide some example of why to use computed outside the scope? Need some context so maybe I can think about how to change the design to fit the equipment.

how to create a standalone scope inside a scope

Nested scope should be collected by its parent scope, but can still be disposed explicitly

const scope1 = effectScope(() => {

  const scope2 = effectScope(() => {
    watchEffect(/* ... */)
  ))

  stop(scope2)
})

@CyberAP

What do you think of attaching effects to the scope after scope creation?

With the last change, effectScope() returns the scope instance instead of the stop handler. You can do that by

const scope = effectScope()

setTimeout(() => {
  scope.effects.push(watch(...))
})
unbyte commented 3 years ago

@antfu

Can't image it, can you provide some example of why to use computed outside the scope? Need some context so maybe I can think about how to change the design to fit the equipment.

I may get you... so you mean all related logic codes are inside the scope right?

Nested scope should be collected by its parent scope, but can still be disposed explicitly

šŸ˜‚ my mean is how to make the child scope have a longer life than the parent scope and to be stopped outside, for example https://github.com/vuejs/vue-next/issues/1532

mathe42 commented 3 years ago

šŸ˜‚ my mean is how to make the child scope have a longer life than the parent scope and to be stopped outside, for example vuejs/vue-next#1532

Maybe we can have a namespace so

const scope1 = effectScope('namespaceA', () => {

  const scope2 = effectScope('namespaceB', () => {
    watchEffect(/* ... */)
  ))

  const scope3 = effectScope('namespaceA', () => {
    watchEffect(/* ... */)
  ))
})
stop(scope1)

So scope2 is still running but scope3 is not. Also the namespace could be optional and when omited it is useing a default namespace.

unbyte commented 3 years ago

@mathe42 so in this way users should stop a scope by its name string. it looks easy but not so good for coding (personnally).

and the dependency of each layer in the nested scopes is not always one-way, which may cause some problem when clean up effects under certain namespace.

maybe I have the wrong idea to solve this problem

antfu commented 3 years ago

vuejs/vue-next#1532 seems like a reasonable use case, that would be great if we can solve it along with this RFC. I am thinking if we could add an option to it and make it not be collected by its parent. Something like:

let scope2

const scope1 = effectScope(() => {
  watchEffect(/* ... */)

  scope2 = effectScope(() => {
    watchEffect(/* ... */)
  ), { escaped: true })
})

stop(scope1)
// scope2 will still be alive until you dispose it
antfu commented 3 years ago

Update:

A draft PR is filed with the feature itself implemented. https://github.com/vuejs/vue-next/pull/2195

unbyte commented 3 years ago

@antfu if reuse it in runtime-core,effectScope may have to return some values for rendering

maybe it can be like this

const scope = effectScope(()=>{
    return {
        // some value
    }
})
scope.exported // visit exported value

child scopes' return value are exported by parent too. it should be a flat object.

but I still don't have a good way to solve the naming conflict

antfu commented 3 years ago

Actually, I have already made the refactor to runtime-core in the draft PR and all tests are passing ;)

effectScope may have to return some values for rendering

Did think about that once, can't find a clean design for it though. And TBH, I don't think this feature is a must-have, you can do it like

let result
const scope = effectScope(()=>{
    result = {
        // some value
    }
})

console.log(result)
unbyte commented 3 years ago

TBH I don't so like this solution šŸ˜‚ but it does't matter

I have a suggestion about escape, that can it be combined with callback?

a callback on stopping scope is useful. sometimes a scope need to know that it's going to be stopped then it can release certain resources or what, just like how callbacks in watch do.

so escape can be implemented by that call callback before cleanup, if false ( or falsy value ) is returned, terminate clean up

ryansolid commented 3 years ago

I will echo this mechanism at the core of my generic reactive renderer DOM Expressions and all my libraries and call them reactive roots. I had to artificially create this in https://github.com/ryansolid/vuerx-jsx. I think there is the question of nested versus isolated. I've opted to isolated in the past. Children should be able to outlive their parents if desired. I make heavy use of this to control reactive context independent of components.

Not sure its worth anything, but just wanted to throw in that I think this is a powerful and worthwhile feature. It is basically the basis for my reactive research and the key to creating dynamic reactive systems without constraints like VDOM/heavy component systems. I think this can only go to help Vue's extensibility.

antfu commented 3 years ago

Update Proposal: Forward the returned value to the outside of the scope

Mentioned in

https://github.com/vuejs/rfcs/pull/212#issuecomment-696459422 https://github.com/vuejs/rfcs/issues/234#issuecomment-728955621

While we still need to return the scope instance, maybe we can use the scope extending function to do this.

const scope = effectScope()

const { x, doubled } = scope(() => {
  const { x } = useMouse()
  cosnt doubled = computed(() => x.value * 2)

  // this make it a bit similar to `setup()`
  return { x, doubled }
})

console.log(doubled.value)

The limitation for this is that when directly calling effectScope can't have the same destructing ability as the extending function. One solution might be under the exported props as @unbyte mentioned.

const scope = effectScope(()=>{
  const { x } = useMouse()
  cosnt doubled = computed(() => x.value * 2)
  return { x, doubled }
})
const { x, doulble } = scope.exported

Or nested destructuring

const { stop, exported: { x, doulble } } = effectScope(()=>{
  const { x } = useMouse()
  cosnt doubled = computed(() => x.value * 2)
  return { x, doubled }
})

console.log(doubled.value)

What do you think? Feedback wanted. Thanks!

posva commented 3 years ago

I like the effectScope version, it's like a setup without the component. Not sure what a good name would be, I don't find effectScope relatable. scoped(), setup()?

kiaking commented 3 years ago

I also thought naming extended scope function setup might look really nice.

const setup = effectScope()

const { x, doubled } = setup(() => {
  const { x } = useMouse()
  cosnt doubled = computed(() => x.value * 2)

  return { x, doubled }
})

It's just a naming convention but this makes people more aware that setup and effectScope is a different thing, hence wouldn't be surprised they have different return value.

And of course in this case, I'm for stop, exported object get returned from effectScope.

const { stop, exported } = effectScope(() => { ... })
posva commented 3 years ago

Oh, I was thinking of directly having one function.

const outOfScopeState = setup(() => {
  const { x } = useMouse()
  cosnt doubled = computed(() => x.value * 2)

  return { x, doubled }
})

outOfScopeState.x.value // mouse.x
destroySetup(outOfScopeState)
// or
outOfScopeState.$destroy()
kiaking commented 3 years ago

Ah I see. You mean having totally separate function rather than getting it as return value from effectScope. That might work too šŸ‘€. That setup function can just call effectScope inside the function call so. Looks nice and I like it.

mathe42 commented 3 years ago

What happens when the function is a async function and I use await...? Does effectScope lose it's context after the first await?

posva commented 3 years ago

It's one of the drawbacks. I would say the function passed to setup should not return a promise. In the use cases I have in mind, they are all synchronous

mathe42 commented 3 years ago
const  scope = effectScope(async () => {
    console.log(0)
    const res = await wait10Sec() //resolves after 10 seconds
    console.log(1)
  ), { async: true })

const  scope = effectScope(async () => {
    console.log(2)
  ), { async: true })

I think it is posible if we add a async option and say async effectScopes don't run in parallel. So the example above would log 0 then waits 10s and log 1 then 2. And if async options is not provided (or false) and a Promise is returned we should throw.

mathe42 commented 3 years ago

I have some UseCases with webworkers in mind...

aztalbot commented 3 years ago

@posva Would it be confusing to have such a function named setup() available to use outside of components where you can't use component lifecycle hooks? Maybe it's just a matter of tweaking the usage warnings to avoid confusion. I could see provide and inject being enabled for use inside this setup()/effectScope() api for composition purposes, though, so maybe that could be a counter point bringing it closer to actual setup. scoped(), like you mentioned, or context() might also be more relatable names. I tend to think of this sort of mechanism as a free floating shareable context that can be revoked. Importing a context function from @vue/reactivity conveys pretty clearly, I think, that it would create a new context in which I can put reactivity logic and return some context object (just like setup returns a render context). effectScope has more of an "I'm a advanced concept, are you sure you want to use me" feel to it.

@antfu Sorry if I missed it somewhere, is there a cleanup hook available? How would I perform some action when the effect scope is stopped, either inside the effect scope itself, or outside, where I am using it?

When you extend a scope, are the returned values from that scope extension added to the exported fields in the original scope, or just returned from that single scope call? Is the main use case for extending a scope dealing with async/await? Iā€™m struggling to think of other times one would need to add to a scope after initialization. Iā€™m curious what those use cases are.

patak-dev commented 3 years ago

@mathe42 @antfu wrote a comment about a possible way to keep the scope in async setup functions using a helper like:

export function createInstanceScope(instance = getCurrentInstance()) {
  return (fn, options) => effectScope(()=> {
    const prev = getCurrentInstance()
    setCurrentInstance(instance)
    fn() // error handling
    setCurrentInstance(prev)
  })
}

If I understand correctly, this could be used in composables too:

async function useAsyncComposable() {
  const scope = createInstanceScope() // uses the current instance
  const data = await fetch() 
  let processed
  scope(() => {
    processed = useXXX(data)
    watch(/*...*/)
  })
  await fetch()
  scope(() => {
     useYYY() // after await but will use the same instance
   })
}

Edited:

I see that for the above pseudo code to work generically inside setup or in an effect scope, an equivalent to getCurrentInstance() and setCurrentInstance(instance) that returns and sets the current scope is needed. Maybe this is not possible? I did not see it discussed before. The idea is to be able to provide a helper like:

export function useScoped(scope = getCurrentScope()) {
  return (fn, options) => effectScope(()=> {
    const prev = getCurrentScope()
    setCurrentScope(scope)
    fn()
    setCurrentScope(prev)
  })
}

That could be used in composables or in setup and will work both from a component setup() or inside a stand alone effect scope:

async function useAsyncSomething(aRef) {
  const scoped = useScoped()
  const response = await fetch(...)
  scoped(() => {
    watch(aRef,...)
  })
}
antfu commented 3 years ago

@mathe42

async effectScopes

the instance context relying on the fact that JavaScript is a single thread and by wrapping the function calls like

    setCurrentInstance(instance)
    fn()
    setCurrentInstance(prev)

But when the fn becomes an async function, setCurrentInstance(prev) will be called before the first await calls, and the instance context gets lost after the await. This is the same limitation as async components' setup. See more discussion in https://github.com/vuejs/rfcs/issues/234

@matias-capeletto

useScoped()

This is already possible with the current design. Every scope is callable to be extended.

@aztalbot

is there a cleanup hook available?

That's a good point, I will add them to the RFC.

just returned from that single scope call

That's exactly what I was thinking. Merging back the exports feels a bit unintuitive.

add to a scope after initialization

Yes. That would help for async/await like https://github.com/vuejs/rfcs/pull/212#issuecomment-731298352 pointed out. Or the component setup() itself: https://github.com/antfu/vue-next/blob/51ee6dcd22475212374086244340edfbac7ae2e4/packages/runtime-core/src/component.ts#L570-L580

antfu commented 3 years ago

@posva I am kinda liking your idea, just forward return value could be much simpler and intuitive.

const { x, doubled, $stop, $scope } = effectScope(() => {
  const { x } = useMouse()
  const doubled = computed(() => x.value * 2)
  return { x, doubled } // force it to be an object so it can be destructurable
})

However, AFAIK, the $ prefixed style is no longer been used in the new Vue 3's Composition API, not sure if we could use them here.

Another idea:

import { stop, effectScope } from 'vue'

const scope = effectScope(() => {
  const { x } = useMouse()
  const doubled = computed(() => x.value * 2)
  return { x, doubled }
})

const { x, doubled } = scope

// extend scope
effectScope(() => {

}, scope) // pass another scope to extend it

stop(scope)
posva commented 3 years ago

I like the second idea without the $

antfu commented 3 years ago

Updates: Add onStop API and returns forwarding behavior.

Excerpt ### Forward returning values Values returned in the scope function will be forward as the return value of `effectScope` and able to be destructured. ```ts const scope = effectScope(() => { const { x } = useMouse() const doubled = computed(() => x.value * 2) return { x, doubled } }) const { x, doubled } = scope console.log(doubled.value) ``` The return value should be an object. ### onStop A `onStop` hook is passed to the scope function ```ts const scope = effectScope((onStop) => { const doubled = computed(() => counter.value * 2) watch(doubled, () => console.log(double.value)) watchEffect(() => console.log('Count: ', double.value)) onStop(()=> { console.log('cleaned!') }) }) stop(scope) // logs 'cleaned!' ``` ### Extend the Scope A scope can be extended by passing it into the `extend` option of `effectScope` calls. ```ts const myScope = effectScope() effectScope(() => { watchEffect(/* ... */) }, { extend: myScope }) effectScope(() => { watch(/* ... */) }, { extend: myScope }) // both watchEffect and watch will be disposed stop(myScope) ``` The extending function will also forward its return values ```ts const { foo } = effectScope(() => { return { foo: computed(/* ... */) } }, { extend: myScope }) ```
antfu commented 3 years ago

About the way to extend a scope, I am still not sure about the design combining with the return forwarding design. Would like have some feedback about several design proposals, thanks!

Proposal A

As the original proposed, a scope is callable to extend

const myScope = effectScope(() => {
  const bar = ref(1)

  return { bar }
})

const { bar } = myScope

const { double } = myScope(() => {
  // extend, the effect created in this function will be append to the 
  const double = computed(() => bar.value * 2)
  return { double }
})

Pros

Cons

Proposal B

Latest updated in RFC. To use effectScope to extend the scope

const myScope = effectScope(() => {
  /* ā€¦ */
})

const { double } = effectScope(() => {
  /* ā€¦ */
  return { double }
}, { extend: myScope }) // pass another scope to extend it

Pros

Cons

Proposal C

Introduce a new API entry extendScope

import { effectScope, extendScope } from 'vue'

const myScope = effectScope(() => {
  /* ā€¦ */
})

const { double } = extendScope(myScope, () => {
  /* ā€¦ */
  return { double }
})

Pros

Cons

meteorlxy commented 3 years ago

To align with vue hooks, what about onStop -> onBeforeStop or onStopped ? (in fact the hook name does not matter at all lol

kiaking commented 3 years ago

@antfu I like Proposal C. A little verbose, but very easy to understand that you're trying to "extend" the scope. The intention of the code, and the argument order makes perfect sense, looking good.

I honestly love Proposal A, but being able to call myScope and also it acts as an object, is I think too confusing in JS land. Proposal B... I think in this case I prefer proposal C.

znck commented 3 years ago

I prefer Proposal C for its explicitness. It is easier to analyze statically, which can be used to improve IDE experience if needed.

mathe42 commented 3 years ago

With Proposal C we could do (I think A and B works too but not 100% sure):

import { effectScope, extendScope } from 'vue'

const myScope = effectScope(async () => {
  /* ā€¦ */
  const data = await fetch()

  return extendScope(myScope, () => {
    /* ā€¦ use Data */
    return { fetchedData }
  })
})

And when useing it in a SFC the compiler could add this without the developer needs write this code...

Additionaly we might want to have a function like getCurrentScope...?

antfu commented 3 years ago

Thanks for the feedback, RFC updated to match Proposal C. Draft PR https://github.com/vuejs/vue-next/pull/2195 is also updated to align with the latest RFC.

antfu commented 3 years ago

Thinking about having an extra hook onEffectScopeStopped which serves a similar functionality to onUnmounted but works for the current scope. This could benefit help composable functions to cleanup their side effects along with its scope. Since setup() also creates a scope for the component, it will be equivalent to onUnmounted when there is no explicit effectScope created.

Example A:

function useMouse() {
  const x = ref(0)
  const y = ref(0)

  function handler(e) {
    x.value = e.x
    y.value = e.y
  }

  window.addEventLisenter('mousemove', handler)

  // we are doing now:
  // onUnmounted(() => {
  //  window.removeEventLisenter('mousemove', handler)
  // })

  // proposed:
  onEffectScopeStopped(() => {
    window.removeEventLisenter('mousemove', handler)
  })

  return { x, y }
}
// lazy init states

let state = undefined

function useState() {
  if (!state)
    state = effectScope(() => useMouse(), { detached: true })
  return state
}

export default {
  setup() {
    return useState()
  }
}

If we are using onUnmounted for useMouse in the example above, although the effects have been collected and detached with the component lifecycle, the removeEventLisenter would still be applied to the component's lifecycle. Which means the x and y would still lose their reactivity after the component gets unmounted. onEffectScopeStopped makes it decoupled more from the component model and being more flexible.

Example B:

export default {
  setup() {
    const enabled = ref(false)
    const mouseScope = null

    watch(enabled, () => {
      if (enabled.value) {
        mouseScope = effectScope(() => useMouse())
      } else {
        stop(mouseScope)
        mouseScope = null
      }
    }, { immediate: true })
  }
}

In the example above, we would create and dispose some scopes on the fly, onEffectScopeStopped allow useMouse to do the cleanup correctly while onUnmounted would never be called during this process.

Note: Would think this is an advanced API for library authors or complex composables.

Please let me know what you think about this proposal, thanks!

posva commented 3 years ago

would it make sense to keep onUnmounted() and adapt its behavior if it's inside an effectScope instead of adding a new onEffectScopeStopped() so useMouse() can be called inside a component and also with effectScope()?

antfu commented 3 years ago

@posva that would do as well. But not sure if it would cause some confusion as onUnmounted sound more like to bound with the component model šŸ¤”

posva commented 3 years ago

It does lack the intention of onEffectScopeStopped() but I don't see users thinking about implementing useMouse() calling onEffectScopeStopped() so it works on both copmonents and effectScope(). onEffectScopeStoppeed() could also internally be an alias to onUnmounted() to keep the intent

antfu commented 3 years ago

/cc @yyx990803, sorry for pinging, but would like to hear some feedback from you about the latest updating proposal: https://github.com/vuejs/rfcs/pull/212#issuecomment-765261704. I agree with @posva's idea of extending onUnmounted (or onBeforeUnmount as well?) instead of introducing a new API, but I am not sure if we are good to change the behavior of existing APIs.

jods4 commented 3 years ago

This is neat.

Drawbacks mention lack of async support but I don't think it's a blocker for a v1. It can be added later and the current version of Vue doesn't support effects in async setup either.

There's a long discussion that explores the pros and cons of several ideas to support async (in setup, but effectScope is the same) here: https://github.com/vuejs/rfcs/discussions/234

There's one slight difference to keep in mind between effectScope and setup, though.

effectScope is a reactivity primitive and it only cares about the current scope to stop effects. In addition, setup also cares about the current component, for stuff like calls to lifecycle mounted, unmounted, etc.

It means that the effectScope solution will not completely cover the setup requirements.

Here's one idea: if the async solution is based on a function withVueContext or similar, maybe we could have 2 different exports similar to what Vue does for watch today:

Module @vue/reactivity exports withVueContext that preserves just the effect scope, for low-level reactivity users, maybe even outside Vue.

Module vue exports a slightly improved withVueContext that preserves the current component (for unmounted and co) and calls reactivity onVueContext to also preserve the effect scope.

SeregPie commented 3 years ago

I hope this will be implemented really soon.

My suggestion would be to align effectScope/extendScope with the setup function to return proxyRefs.

let state = effectScope(() => ({
  a: ref(1),
  b: {c: ref(2)},
}));
console.log(isRef(state.a)); // => false
console.log(isRef(state.b.c)); // => true

But maybe it is a bad suggestion. I'm not sure.

mcrapts commented 3 years ago

When will this get addressed? Currently, Vuex 4.0.0 getters are not being cached, apparently because of this issue. This seems like a very biig issue to me, because at this moment is it not possible to use Vuex 4.0.0 + Vue 3 efficiently. Getters are re-evaluated everytime they are accessed where they should be cached. This is a huge showstopper for me.

basvanmeurs commented 3 years ago

Actually, I have already made the refactor to runtime-core in the draft PR and all tests are passing ;)

effectScope may have to return some values for rendering

Did think about that once, can't find a clean design for it though. And TBH, I don't think this feature is a must-have, you can do it like

let result
const scope = effectScope(()=>{
    result = {
        // some value
    }
})

console.log(result)

It should have stopped here. Imho, forwarding return values is the worst thing that happened to this otherwise awesome PR. I think that it's a useless unnecessary complication. Just use normal js variables in the desired scope as antfu suggested.