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

Unhandled error during execution of watcher callback and scheduler flush #2170

Closed
kleinfreund opened this issue Sep 19, 2020 · 8 comments · Fixed by #2295
Closed

Unhandled error during execution of watcher callback and scheduler flush #2170

kleinfreund opened this issue Sep 19, 2020 · 8 comments · Fixed by #2295
Labels
has PR A pull request has already been submitted to solve the issue 🐞 bug Something isn't working

Comments

@kleinfreund
Copy link
Contributor

kleinfreund commented Sep 19, 2020

Version

3.0.0

Reproduction link

https://codesandbox.io/s/vigorous-nash-8lxt8?file=/src/Child.vue

Steps to reproduce

  • click the increment button

What is expected?

this.$el should be defined inside of the watcher like in Vue 2

What is actually happening?

it is null

@LinusBorg
Copy link
Member

Does this only happen in the test, but works fine when using the code in an app? Then that mit be something related to the test-utils. I can transfer the issue if so.

@LinusBorg LinusBorg added the need more info Further information is requested label Sep 19, 2020
@kleinfreund
Copy link
Contributor Author

When using the component that the aforementioned test file is testing in a Vue app, it doesn’t render without issues. The most glaring issue is that in the mounted hook, the $refs object is still empty, even though I have two refs defined in the template section (both not being conditionally rendered). I’m getting two Missing ref owner context. ref cannot be used on hoisted vnodes. A vnode with ref must be created inside the render function. warnings (likely one for each ref instance in the template. Furthermore, five Invalid VNode type: Symbol(Comment) (symbol) warnings are printed after the created hook is executed, but before the mounted hook is executed. Finally, a Unhandled error during execution of mounted hook warning is shown because in the mounted hook, one of the refs is being accessed, but since it’s undefined, a TypeError is thrown.

Now I’m not certain that either of those issues is necessarily related to what I reported initially. It might be that I’ve done something wrong when migrating the consuming app to Vue 3 or something of my tooling dependencies isn’t quite ready to be used with Vue 3, yet. But also, both the component that is the subject of the report above and the consuming app are relatively small and basic. I’ll try to reduce the test setup for this to figure out more.

@amrnn90
Copy link

amrnn90 commented Sep 19, 2020

Here's a minimal reproduction:
https://codesandbox.io/s/vigorous-nash-8lxt8?file=/src/Child.vue

Click the increment button and open the console:
this.$el is null inside the Child component watcher.

@sixthgear
Copy link

I can confirm a similar issue while trying to migrate a v2 codebase to v3. Inside of a component watcher I expect this.$el to be set, but it is null.

@ajimix
Copy link

ajimix commented Sep 28, 2020

I'm having the same problem also when migrating from v2 to v3 this.$el ends up being null while in v2 it works properly

@posva posva added 🐞 bug Something isn't working and removed need more info Further information is requested labels Sep 29, 2020
@LinusBorg
Copy link
Member

https://github.com/vuejs/vue-next/blob/5d825f318f1c3467dd530e43b09040d9f8793cce/packages/runtime-core/src/renderer.ts#L1427-L1432

  • updateComponentPreRender() sets the instance.vnode to the next vnode, and then calls all queued effects.
  • at that moment, the new vnode doesn't have an element assigned
  • only after that does the new vnode get the element assigned.

@kleinfreund
Copy link
Contributor Author

kleinfreund commented Oct 3, 2020

In my naïve hopes to possibly fix this myself, I wrote a two test cases which — as far as I understand — should both pass:

  • setting an instance’s watched data property
  • rendering an instance’s watched prop

Both should have access to the instance’s $el property. I wrote them in packages/runtime-core/__tests__/rendererComponent.spec.ts.

should have access to instance’s “$el” property in watcher when setting instance data

// #2170
test('should have access to instance’s “$el” property in watcher when setting instance data', async () => {
  function returnThis(this: any) {
    return this
  }
  const dataWatchSpy = jest.fn(returnThis)
  let instance: any
  const Comp = {
    data() {
      return {
        testData: undefined
      }
    },

    watch: {
      testData() {
        // @ts-ignore
        dataWatchSpy(this.$el)
      }
    },

    created() {
      instance = this
    },

    render() {
      return h('div')
    },
  }


  const root = nodeOps.createElement('div')
  render(h(Comp), root)

  expect(dataWatchSpy).not.toHaveBeenCalled()
  instance.testData = 'data'

  await nextTick()
  expect(dataWatchSpy).toHaveBeenCalledWith(instance.$el)
})

This test passes.

should have access to instance’s “$el” property in watcher when rendereing with watched prop'

// #2170
test('should have access to instance’s “$el” property in watcher when rendereing with watched prop', async () => {
  function returnThis(this: any) {
    return this
  }
  const propWatchSpy = jest.fn(returnThis)
  let instance: any
  const Comp = {
    props: {
      testProp: String
    },

    watch: {
      testProp() {
        // @ts-ignore
        propWatchSpy(this.$el)
      }
    },

    created() {
      instance = this
    },

    render() {
      return h('div')
    },
  }


  const root = nodeOps.createElement('div')

  render(h(Comp), root)
  await nextTick()
  expect(propWatchSpy).not.toHaveBeenCalled()

  render(h(Comp, { testProp: 'prop ' }), root)
  await nextTick()
  expect(propWatchSpy).toHaveBeenCalledWith(instance.$el)
})

This test fails because propWatchSpy is called with null instead of what should be the instance’s $el.

@kleinfreund
Copy link
Contributor Author

I now understand @LinusBorg’s investigation and think I can provide a fix for this. If that’s alright, I’d like to submit a pull request for this.

@LinusBorg LinusBorg added the has PR A pull request has already been submitted to solve the issue label Oct 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Nov 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has PR A pull request has already been submitted to solve the issue 🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants