fix(vue): property access on undefined in errorHandler

[Arinono]

Authors

@Arinono

Issue

Fixes #5277

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

• If you've added code that should be tested, please add tests.
• Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

[lforst]

Thank you very much for the contribution! Especially big props for adding tests! I left two suggestions - let me know if you find the time to integrate them into your PR, otherwise we can take over!

[lforst]

Did you figure out why these tests were failing for node 8? If yes, we should probably add a comment here why we're skipping these tests.

[Arinono]

No I have no idea why they are failing. I don't know if it has to do with jest or the fact that I use @sentry/browser in my tests.
But since @sentry/browser (nor @sentry/hub) tests are not skipped in node 8. And the fact that you already have a bunch of tests using getCurrentHub.withScope in the hub package tells me that it's probably jest 🤔 (or my code)

[lforst]

Your change should work perfectly fine, we (sadly) have to constantly consider bundle size in our SDKs. Optional chains are transpiled to especially large code so I suggest we change it to the following:

Suggested change

	if (options.attachProps && vm && (vm.$options?.propsData || vm.$props)) {
	// Vue2 - $options.propsData
	// Vue3 - $props
	metadata.propsData = vm.$options.propsData || vm.$props;
	metadata.propsData = vm.$options?.propsData || vm.$props;
	if (options.attachProps && vm && vm.$options) {
	// Vue2 - $options.propsData
	// Vue3 - $props
	metadata.propsData = vm.$options.propsData || vm.$props;

What do you think?

[Arinono]

Optional chains are transpiled to especially large code

I had no idea 😅

What do you think?

I'll have to suggest something else as it breaks some tests I wrote

Especially because in your suggestion, the outer condition prevents the attachement of props for vue 3 ($props will be defined, but won't satisfy the condition)

This one, less compact, works:

    if (options.attachProps && vm) {
      // Vue2 - $options.propsData
      // Vue3 - $props
      if (vm.$options && vm.$options.propsData) {
        metadata.propsData = vm.$options.propsData;
      } else if (vm.$props) {
        metadata.propsData = vm.$props;
      }
    }

Would it work for you ?

[lforst]

Ah yeah you're totally right - my bad. The last snippet you shared works. Thank you!

[lforst]

Just saw that you managed to get the tests working for node 8 - awesome! I think we can remove this again then.

[Arinono]

Indeed, I just did it

[lforst]

Thank you for your contribution!