mike-north / ember-perf

Measure user-percieved performance data in your ember.js app
https://mike.works/ember-perf
MIT License
73 stars 20 forks source link

Errors on certain route transitions #83

Open galfert opened 7 years ago

galfert commented 7 years ago

I noticed that ember-perf throws errors on certain route transitions.

The first one is thrown when transitioning between routes with the same name but different params (e.g. from /articles/1 to /articles/2). The error is:

Cannot set property 'endTime' of undefined TypeError: Cannot set property 'endTime' of undefined at TransitionData.routeFinishedSetup

which is thrown in https://github.com/mike-north/ember-perf/blob/5715a2ddcfb5ba0f2d11c78f1d43f70f93fc7f54/addon/core/transition-data.js#L43

I think the reason for this is that activateRoute from https://github.com/mike-north/ember-perf/blob/5715a2ddcfb5ba0f2d11c78f1d43f70f93fc7f54/addon/core/transition-data.js#L26 is never called on this transition, and thus this.routes stays empty.

The second error happens when transitioning between routes with the same name, but different query params (e.g. from /bookmarks?category=news to /bookmarks?category=politics). The error is

Assertion Failed: Must have at least a route name

which is thrown in https://github.com/mike-north/ember-perf/blob/5715a2ddcfb5ba0f2d11c78f1d43f70f93fc7f54/addon/services/ember-perf.js#L68.

I created a basic Ember app at https://github.com/galfert/ember-perf-bugs that reproduces these two errors.

galfert commented 7 years ago

I was able to prevent the first error by changing this.get('perfService').routeActivated(this); to get called from setupController() instead of activate() in https://github.com/mike-north/ember-perf/blob/5715a2ddcfb5ba0f2d11c78f1d43f70f93fc7f54/addon/ext/route.js#L6.

I think setupController is the next in line of route hooks after activate, but it gets called every time. Not sure this change has any other implications though.

It also doesn't prevent the second error from happening.