From d227442f2651c6e53b0a878cb227c34974e2abad Mon Sep 17 00:00:00 2001 From: Ryan LaBouve Date: Tue, 17 Apr 2018 12:15:08 -0500 Subject: [PATCH 1/2] Adds failing test to same route dif param bug > 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: https://github.com/mike-north/ember-perf/issues/83 --- tests/acceptance/event-body-test.js | 38 +++++++++++++++++++ tests/dummy/app/router.js | 3 ++ tests/dummy/app/routes/articles/article.js | 4 ++ tests/dummy/app/routes/base.js | 7 +++- .../dummy/app/templates/articles/article.hbs | 9 +++++ tests/helpers/validate-event.js | 12 +++++- 6 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 tests/dummy/app/routes/articles/article.js create mode 100644 tests/dummy/app/templates/articles/article.hbs diff --git a/tests/acceptance/event-body-test.js b/tests/acceptance/event-body-test.js index 6c90f390..2e7e6e2e 100644 --- a/tests/acceptance/event-body-test.js +++ b/tests/acceptance/event-body-test.js @@ -136,6 +136,44 @@ test('Initial load, then drilling in, then pivoting', function(assert) { }); }); +test('Nested resource with the same name ', function(assert) { + // This test covers the bug here: https://github.com/mike-north/ember-perf/issues/83 + + let datas = []; + let testStartTime = performanceNow(); + + application.perfService.on('transitionComplete', (data) => { + datas.push(data); + }); + + visit('/articles/1'); + + andThen(function() { + assert.ok(datas, 'Data is present'); + let [data] = datas; + validateEvent(assert, testStartTime, data); + assert.equal(datas.length, 1, 'Only one event fired'); + }); + + click('a[href=\'/articles/2\']'); + + andThen(function() { + assert.equal(datas.length, 2, 'Only two events fired'); + let data = datas[datas.length - 1]; + assert.equal(data.destURL, '/articles/2', 'Intent URL is correct'); + assert.equal(data.destRoute, 'articles.article', 'Intent route is correct'); + }); + + click('a[href=\'/articles/3\']'); + + andThen(function() { + assert.equal(datas.length, 3, 'Only three events fired'); + let data = datas[datas.length - 1]; + assert.equal(data.destURL, '/articles/3', 'Intent URL is correct'); + assert.equal(data.destRoute, 'articles.article', 'Intent route is correct'); + }); +}); + test('Initial measureRender', function(assert) { let datas = []; let testStartTime = performanceNow(); diff --git a/tests/dummy/app/router.js b/tests/dummy/app/router.js index f464422f..6f7a2c88 100644 --- a/tests/dummy/app/router.js +++ b/tests/dummy/app/router.js @@ -7,6 +7,9 @@ const Router = EmberRouter.extend({ }); Router.map(function() { + this.route('articles', function() { + this.route('article', { path: ':article_id' }); + }); this.route('companies', function() { this.route('info'); }); diff --git a/tests/dummy/app/routes/articles/article.js b/tests/dummy/app/routes/articles/article.js new file mode 100644 index 00000000..af611ce2 --- /dev/null +++ b/tests/dummy/app/routes/articles/article.js @@ -0,0 +1,4 @@ +// import Ember from 'ember'; +import BaseRoute from '../base'; + +export default BaseRoute.extend({ }); diff --git a/tests/dummy/app/routes/base.js b/tests/dummy/app/routes/base.js index 10e904a0..b5c8585b 100644 --- a/tests/dummy/app/routes/base.js +++ b/tests/dummy/app/routes/base.js @@ -1,5 +1,8 @@ +import Object from '@ember/object'; import Route from '@ember/routing/route'; export default Route.extend({ - -}); \ No newline at end of file + model(params) { + return Object.create({id: params.article_id}); + } +}); diff --git a/tests/dummy/app/templates/articles/article.hbs b/tests/dummy/app/templates/articles/article.hbs new file mode 100644 index 00000000..9ebdd072 --- /dev/null +++ b/tests/dummy/app/templates/articles/article.hbs @@ -0,0 +1,9 @@ +

Availible Articles

+ + + +

Article {{model.id}}

diff --git a/tests/helpers/validate-event.js b/tests/helpers/validate-event.js index d47a4942..3455bccc 100644 --- a/tests/helpers/validate-event.js +++ b/tests/helpers/validate-event.js @@ -12,7 +12,17 @@ export default function(assert, now, data, eventType = 'transition') { if (eventType === 'transition') { assert.equal(typeOf(data.routes), 'array', 'data.routes is an array'); - assert.deepEqual(data.routes.map((r) => r.name), ['application', 'loading', 'company', 'company.loading', 'company.buildings'], 'Proper routes load'); + + + let rs = data.routes.map((r) => r.name) + + if (rs.includes('articles')) { + assert.deepEqual(rs, ['application', 'articles', 'articles.article'], 'Proper routes load for articles resource'); + + } else { + assert.deepEqual(rs, ['application', 'loading', 'company', 'company.loading', 'company.buildings'], 'Proper routes load for companies/buildings resource'); + } + assert.equal(data.routes .map((r) => r.startTime) .filter((x) => typeOf(x) !== 'number'), 0, 'All route startTimes are numbers'); From 44289884e45841a378caf2e0fdf16b238671bb02 Mon Sep 17 00:00:00 2001 From: Ryan LaBouve Date: Tue, 17 Apr 2018 12:15:52 -0500 Subject: [PATCH 2/2] Moves service to fire on setupController This fixes the bug: > 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: ref https://github.com/mike-north/ember-perf/issues/83 --- addon/ext/route.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addon/ext/route.js b/addon/ext/route.js index 94732892..83d7c914 100644 --- a/addon/ext/route.js +++ b/addon/ext/route.js @@ -1,7 +1,7 @@ import Mixin from '@ember/object/mixin'; export default Mixin.create({ - activate() { + setupController() { this.get('perfService').routeActivated(this); this._super(...arguments); },