feat(vue): Support Vue 3 lifecycle hooks in mixin options

[snoozbuster]

Fixes #9565 (kind of)

All I did here is add support for the vue 3 hooks - users still have to manually choose the correct one for their application.

The trouble with doing what @Lms24 suggested in this comment is that createTracingMixins doesn't get the app instance in order to check its version. version is exported by Vue at the top level (import { version } from 'vue') besides being available as app.version, but that didn't seem like a good option, so it's not really possible to automatically adjust the set of hooks used inside createTracingMixin without changing its interface. I did try writing some code in vueInit which would try to fixup invalid hooks:

  if (hasTracingEnabled(options)) {
    const tracingOptions: TracingOptions = {
      ...options,
      ...options.tracingOptions,
    };

    const vueVersion = Number(app.version.split('.')[0]);
    if (tracingOptions.hooks) {
      const [hookFrom, hookTo] =
        vueVersion === 3 ? (['destroy', 'unmount'] as const) : (['unmount', 'destroy'] as const);
      // ensure hooks are valid for the version
      tracingOptions.hooks = tracingOptions.hooks.map(hook => (hook === hookFrom ? hookTo : hook));
    }

    app.mixin(createTracingMixins(tracingOptions));
  }

however, I had trouble writing tests for this code. I tried to add two tests to init.test.ts (one for the conversion of vue 2 -> vue 3 hooks, and one for vue 3 -> vue 2). However, the first test would always fail - based on some logging I did, it seems like the options that are seen in the test are one test run "behind" for some reason. So I didn't add that code to this PR, since I couldn't test it. Let me know if you'd like me to add it without tests, or if you know of a fix for the options-being-delayed issue.

[Lms24]

Hi @snoozbuster thanks so much for contributing!

I see your point about my initial idea (admittedly that was just a quick thought, didn't look too carefully). I have another one which might ease testability:

I think we can quite easily distinguish between Vue 2 and 3. If users use the SDK in a Vue 2 app, they set Sentry.init({Vue, ...}), while in Vue 3, it's Sentry.init({app, ...}).
So ideally (and please feel free to tell me if this doesn't add up) we can just check which of the two is defined. This, we can do in vueInit:

if (hasTracingEnabled(options)) {
    const tracingOptions = {
      ...options,
      ...options.tracingOptions,
    };

    const vueVersion = options.app ? 'vue3' : 'vue2';

    app.mixin(createTracingMixins(tracingOptions, vueVersion));
  }

Then, we add an optional parameter to createTracingMixins with the vue version and set the hooks depending on the specified version. If no version is specified, we fall back to our current behaviour (so I guess vue2). This param needs to be optional though, because createTracingMixins is public API, so we can't break users. What do you think about this approach?

Re testing: I think we should be able to test the two parts in isolation which probably simplifies things a lot. If this makes sense to you but for some reason the tests that you think should work, don't just let me know and we'll look into it. Testing Sentry integrations is unfortunately notoriously hard due to us putting stuff onto globals.

[Lms24]

One more thing, whatever we do here, users need to be able to manually override the chosen hooks if they specified hooks themselves.

[snoozbuster]

@Lms24 checking the vue version in vueInit wasn't really the issue. it was more that when I passed hooks: ['destroy'] in the options in the test to confirm that the hook was translated correctly, that set of hooks wasn't observed in the test until the next test run.

if you think I should do the translation inside createTracingMixins based on a second parameter I can certainly do that. but it made more sense to me to just do it inside vueInit, since the second parameter to createTracingMixin would never really be used by a client. Unless you're also thinking that the list of valid Operations would remain the same (and the meaning of destroy would change based on the second parameter), instead of having them both as separate options? I think there is some merit in allowing explicit control of what set to use because of the vue 3 compatibility build (@vue/compat).

[Lms24]

Thanks for explaining. One important part I missed initially was that the destroy hook is not tracked by default, meaning users have to add it manually. So in this case we don't need to move anything to createTracingMixins

In fact, I think, we'll keep this PR as-is, without the additional check. This should be enough so that users can add the hook if they want to. We'll also need to update the docs page (see link above) to reflect the change.

[snoozbuster]

That works for me. Should I update the docs page code? if so, where is that?

[Lms24]

That works for me. Should I update the docs page code? if so, where is that?

That would be great, thank you! I think adding the hook and a sentence about which hooks are available in Vue 2 vs Vue 3 should be enough. You can find the docs source here:

https://github.com/getsentry/sentry-docs/blob/2717d62c83febf54e26994065fa4b039cdd02eff/src/platforms/javascript/guides/vue/features/component-tracking.mdx#L48-L64

[Lms24]

I'm going to merge this in the meantime. Thank you for contributing @snoozbuster! We'll make sure you're mentioned in the next release.