fix(ssr): watchEffect onInvalidate runner initialization (close #3322)

[tjk]

No description provided.

[HcySunYang]

Hey, thank you for your PR, but I think there is a better way. In SSR, we do not need to initialize runner.options.onStop, just change a little bit:

  const onInvalidate: InvalidateCbRegistrator = (fn: () => void) => {
    cleanup = () => {
      callWithErrorHandling(fn, instance, ErrorCodes.WATCH_CLEANUP)
    }
    if (!(__NODE_JS__ && isInSSRComponentSetup)) {
      runner.options.onStop = cleanup
    }
  }

[HcySunYang]

I rethink about that, the callback registered with onInvalidate will not be executed in SSR at all, so we only need to provide a NOOP instead of onInvalidate.

[tjk]

@HcySunYang thanks for the feedback, that actually makes sense! I'll do another pass. Do you have a suggestion for improving the test?

PS: Edited the patch (much better!)

[LinusBorg]

done will get called by the first watchEffect bootstrap/run, which is not the SSR one, so an error in the SSR one would not be part of the test anymore, I think?

Will think about how to do this with async/await.

[tjk]

I'm assuming (and running the tests seems to validate) that setup gets called twice. The first time, instance is not set, so we just set instance. The second setup call with SSR triggers the watchEffect branch.

I could rewrite with promises but I think it would be a bit more verbose... is that preferred?

[HcySunYang]

I think it would be better if we write test case in server-renderer, as a special case, although it is related to watch, but it is also related to the rendering behavior of SSR.

[tjk]

@HcySunYang thanks for that pointer... much better place for it! Let me know if anything else should be touched up.

[HcySunYang]

@tjk , great work, thanks