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

Current instance is not unset after error in data() on SSR #7733

Closed
DrPhil opened this issue Feb 16, 2023 · 2 comments · Fixed by #7743
Closed

Current instance is not unset after error in data() on SSR #7733

DrPhil opened this issue Feb 16, 2023 · 2 comments · Fixed by #7743
Assignees
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working scope: ssr

Comments

@DrPhil
Copy link
Contributor

DrPhil commented Feb 16, 2023

Vue version

3.2.47

Link to minimal reproduction

https://stackblitz.com/edit/github-qw7apw-58cjm8

Steps to reproduce

Click 'Trigger error in data'. it will try to load a component where there is a JS error inside the data function on the component.

Then click 'click here to see stale instance' - it will perform a hard reload, calling getCurrentInstance() on the server and revealing that the instance was not unset.

What is expected?

I expect the current component instance to be unset if there is an error inside data.

What is actually happening?

The current component instance is not unset.

System Info

No response

Any additional comments?

This is very similar to #6110 which has already been fixed. The difference is that the error happens in data instead of during rendering.

I had some troubles with our SSR setup and Pinia using the wrong instance after an error. Pinia looks for getCurrentInstance and uses that instance to inject the current pinia instance.

@DrPhil
Copy link
Contributor Author

DrPhil commented Feb 16, 2023

Similarly, throwing an error in created and then also throwing inside errorCaptured also leaves getCurrentInstance set. You can see this in the same reproducing example.

@DrPhil
Copy link
Contributor Author

DrPhil commented Feb 16, 2023

Here are two testcases that can be added to server-renderer/__tests__/render.spec.ts. (Not entirely sure that these test-cases are correct, or that this is the right place to put them. I trust the reproducing example more!)

      // #7733
      test('reset current instance after error in data', async () => {
        const prev = getCurrentInstance()
        expect(prev).toBe(null)
        try {
          await render(
              createApp({
                data() {
                  throw new Error();
                },
                template: `<div>hello</div>`
              })
          )
        } catch {}
        expect(getCurrentInstance()).toBe(null)
      })
    })

    test('reset current instance after error in errorCaptured', async () => {
      const prev = getCurrentInstance()
      expect(prev).toBe(null)
      try {
        await render(
            createApp({
              errorCaptured() {
                throw new Error();
              },
              template: `<div>hello</div>`,
              created() {
                throw new Error();
              }
            })
        )
      } catch {}
      expect(getCurrentInstance()).toBe(null)
    })

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working scope: ssr
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants