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

Remove Vue.util.formatComponentName usage as it's not present on production #848

Merged
merged 1 commit into from
Feb 9, 2017
Merged

Conversation

diogobeda
Copy link
Contributor

@diogobeda diogobeda commented Feb 6, 2017

As you can see on the code below, Vue.util.formatComponent is not present on production bundles, so raven shouldn't depend on it for getting component names:

https://github.com/vuejs/vue/blob/51e7608528c2e958e3705425bad3d6325ce97765/src/core/util/debug.js#L7

That causes sentry to not capture any error at all. It instead throws this error on console:
screen shot 2017-02-06 at 11 00 44

This function is not subject to many changes, so I think we can just copy its content and update if needed on the future. What do you guys think?

@benvinegar
Copy link
Contributor

benvinegar commented Feb 6, 2017

I am super hesitant to accept changes that rely on internal properties and methods. We don't know if these will change or not. Maybe with the blessing of @yyx990803.

I'd sooner say: let's attempt to use formatComponent if it's available, otherwise it becomes "anonymous component".

@yyx990803
Copy link

All of these three internal properties (_isVue, $options._componentTag and $options.__file) are unlikely to ever change. Their usage also won't throw any errors even if they do change - the worst case scenario is returning anoymous component. So I'd say it's safe to use this method here.

@benvinegar benvinegar merged commit 37491f7 into getsentry:master Feb 9, 2017
@nddery
Copy link

nddery commented Feb 11, 2017

Any chance of seeing a patch release with this fix ? We've been wondering why Sentry had been so silent..

@rtheunissen
Copy link

@benvinegar also waiting for this to be released, we have multiple cases on production that aren't reaching Sentry because of this issue.

@benvinegar
Copy link
Contributor

@nddery @rtheunissen – it's out now in 3.11.0. Sorry for the delay; I try to make sure I only publish new versions when I am available to triage any potential new issues that pop up as a result of the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants