Skip to content

fix(vue-app): fix exception on property access of undefined object #5867

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

Merged
merged 1 commit into from
Jun 4, 2019
Merged

fix(vue-app): fix exception on property access of undefined object #5867

merged 1 commit into from
Jun 4, 2019

Conversation

rchl
Copy link

@rchl rchl commented Jun 4, 2019

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

After PR #5746, which has added code to remove trailing slash
in non-strict mode, I've gotten exception reported through Sentry due to
trying to get proper 'path' of undefined 'matchedRoute' object.

I don't know how it happens but it's clear error as code above checks
for null matchedRoute but not the code that checks for trailing slash.

Made the logic so that non-replaced path is returned when no route
matches. Same as before original fix.

Follow-up fix to #5593

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@codecov-io
Copy link

Codecov Report

Merging #5867 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #5867   +/-   ##
=======================================
  Coverage   95.68%   95.68%           
=======================================
  Files          82       82           
  Lines        2690     2690           
  Branches      690      690           
=======================================
  Hits         2574     2574           
  Misses         98       98           
  Partials       18       18
Flag Coverage Δ
#e2e 100% <ø> (ø) ⬆️
#fixtures 50.37% <ø> (ø) ⬆️
#unit 92.67% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afbb9f2...5957fbf. Read the comment docs.

@rchl
Copy link
Author

rchl commented Jun 4, 2019

@clarkdo please review

Copy link
Member

@clarkdo clarkdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rchl Good catch.

The checking is necessary, but I suggest returning first if matchedRoute is undefined.

const [matchedRoute] = this.$route.matched

if (!matchedRoute) {
  return this.$route.path
}

const Component = matchedRoute && matchedRoute.components.default
// ...

After PR #5746, which has added code to remove trailing slash
in non-strict mode, I've gotten exception reported through Sentry due to
trying to get proper 'path' of undefined 'matchedRoute' object.

I don't know how it happens but it's clear error as code above checks
for null `matchedRoute` but not the code that checks for trailing slash.

Made the logic so that non-replaced path is returned when no route
matches. Same as before original fix.
@pi0 pi0 requested a review from clarkdo June 4, 2019 19:46
@pi0 pi0 merged commit ab72355 into nuxt:dev Jun 4, 2019
@rchl rchl deleted the fix/null-route branch June 4, 2019 19:50
@pi0 pi0 mentioned this pull request Jun 4, 2019
@clarkdo
Copy link
Member

clarkdo commented Jun 4, 2019

@rchl Thanks

@pimlie
Copy link

pimlie commented Jun 5, 2019

@rchl You mentioned you didnt know how the error happened, are you using nested routes in your app by any chance? If so then this issue might be related: #5857

@rchl
Copy link
Author

rchl commented Jun 5, 2019

@pimlie No, I'm not using nested routes (nuxtChild I presume). It only happened once though and it is nothing given number of requests. I get strangest errors reported by Sentry browser client anyway...

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

Successfully merging this pull request may close these issues.

6 participants