Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[reactivity][type] createReactiveEffect() and effect() have difference between static type and runtime #3647

Closed
hyf0 opened this issue Apr 22, 2021 · 5 comments

Comments

@hyf0
Copy link
Contributor

hyf0 commented Apr 22, 2021

Version

3.0.11

Reproduction link

https://codesandbox.io/s/elegant-chandrasekhar-dlmc4?file=/src/main.js

Steps to reproduce

function createReactiveEffect<T = any>(
  fn: () => T,
  options: ReactiveEffectOptions
): ReactiveEffect<T> {
  const effect = function reactiveEffect(): unknown {
    if (!effect.active) {
      return options.scheduler ? undefined : fn() // reactiveEffect may return undefined
    }
    if (!effectStack.includes(effect)) {
      cleanup(effect)
      try {
        enableTracking()
        effectStack.push(effect)
        activeEffect = effect
        return fn()
      } finally {
        effectStack.pop()
        resetTracking()
        activeEffect = effectStack[effectStack.length - 1]
      }
    }
  } as ReactiveEffect
  effect.id = uid  
  effect.allowRecurse = !!options.allowRecurse
  effect._isEffect = true
  effect.active = true
  effect.raw = fn
  effect.deps = []
  effect.options = options
  return effect
}

for

const re = createReactiveEffect(() => 1)
re // => ReactiveEffect<number> ✅

but for

const re2 = createReactiveEffect(() => 1, {
  scheduler: setImmediate
})

What is expected?

re2 // => ReactiveEffect<number | undefined> 

What is actually happening?

re2 // => ReactiveEffect<number> 

Please correct me if I am wrong

@HcySunYang
Copy link
Member

Please double-check the reproduction you provided, it cannot reproduce the problem you described.

@hyf0
Copy link
Contributor Author

hyf0 commented Apr 22, 2021

Please double-check the reproduction you provided, it cannot reproduce the problem you described.

@vue/reactivity does't export createReactiveEffect , but effect(exported) return result of createReactiveEffect finally, so they have same problem cause by createReactiveEffect.
so Reproduction link just show example of effect

export function effect<T = any>(
  fn: () => T,
  options: ReactiveEffectOptions = EMPTY_OBJ
): ReactiveEffect<T> {
  if (isEffect(fn)) {
    fn = fn.raw
  }
  const effect = createReactiveEffect(fn, options)
  if (!options.lazy) {
    effect()
  }
  return effect
}

example for effect

const re2 = effect(() => 1, {
  scheduler: window.setImmediate
})

What is expected?

re2 // => ReactiveEffect<number | undefined> 

What is actually happening?

re2 // => ReactiveEffect<number> 

@hyf0
Copy link
Contributor Author

hyf0 commented Apr 22, 2021

some type programing maybe helpful. I guess this need tests and .d.ts tests, which I don't know how to do it and can't send PR to fix it.

export function effect<T = any, O extends ReactiveEffectOptions = typeof EMPTY_OBJ>(
  fn: () => T,
  options: O = (EMPTY_OBJ as O)
): O extends { scheduler?: undefined } ? ReactiveEffect<T> : ReactiveEffect<T | undefined> {
  type Returned = O extends { scheduler?: undefined } ? ReactiveEffect<T> : ReactiveEffect<T | undefined>
  if (isEffect(fn)) {
    fn = fn.raw
  }
  const effect = createReactiveEffect(fn, options)
  if (!options.lazy) {
    effect()
  }
  return effect as Returned
}


const a = effect(() => 1) // => ReactiveEffect<number >

const a2 = effect(() => 1, { scheduler: setImmediate }) //  => ReactiveEffect<number | undefined>

@HcySunYang
Copy link
Member

HcySunYang commented Apr 22, 2021

I get your points now, but it will return undefined only if an effect has the scheduler option and it is inactive. But here is another issue discussing whether an effect should always return non-undefined when it is inactive, so we don't need to do anything for now.

@hyf0
Copy link
Contributor Author

hyf0 commented May 28, 2021

doesn't have the problem after commit 03a7a73

@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants