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.

Sign up for GitHub

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

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

Merged
merged 7 commits into from
Mar 25, 2021
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
76fcde1
fix(ssr): watchEffect onInvalidate runner initialization
tjk Feb 27, 2021
3b1818a
simplify and setup earlier noop
tjk Feb 28, 2021
b48400f
be more liberal and ensure no error is thrown
tjk Feb 28, 2021
509a906
move and simplify the test
tjk Mar 1, 2021
96f9ebe
use await to be safer
tjk Mar 1, 2021
8cf16b9
Merge branch 'master' into fix-3322
LinusBorg Mar 9, 2021
9896d60
Merge branch 'master' into fix-3322
yyx990803 Mar 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions packages/runtime-core/__tests__/apiWatch.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ComponentInternalInstance,
ComponentPublicInstance
} from '../src/index'
import { setupComponent } from '../src/component'
import {
render,
nodeOps,
Expand Down Expand Up @@ -44,6 +45,28 @@ describe('api: watch', () => {
expect(dummy).toBe(1)
})

// https://github.com/vuejs/vue-next/issues/3322
it('effect onInvalidate succeeds in ssr context', done => {
// XXX do this without double setup and generally less hackily!
let instance: ComponentInternalInstance | null
const noop = () => {}
const Comp = defineComponent({
setup() {
if (!instance) {
instance = getCurrentInstance()
} else {
watchEffect(onInvalidate => {
expect(() => onInvalidate(noop)).not.toThrow(ReferenceError)
done()
Copy link
Member

@LinusBorg LinusBorg Feb 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@tjk tjk Feb 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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?

})
}
},
render: noop
})
createApp(Comp).mount(nodeOps.createElement('div'))
setupComponent(instance!, true)
})

it('watching single source: getter', async () => {
const state = reactive({ count: 0 })
let dummy
Expand Down
4 changes: 3 additions & 1 deletion packages/runtime-core/src/apiWatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ function doWatch(
}

let cleanup: () => void
const onInvalidate: InvalidateCbRegistrator = (fn: () => void) => {
let onInvalidate: InvalidateCbRegistrator = (fn: () => void) => {
cleanup = runner.options.onStop = () => {
callWithErrorHandling(fn, instance, ErrorCodes.WATCH_CLEANUP)
}
Expand All @@ -232,6 +232,8 @@ function doWatch(
// in SSR there is no need to setup an actual effect, and it should be noop
// unless it's eager
if (__NODE_JS__ && isInSSRComponentSetup) {
// we will also not call the invalidate callback (+ runner is not set up)
onInvalidate = NOOP
if (!cb) {
getter()
} else if (immediate) {
Expand Down