From b7dd0d4b17883b7cab233123049a0a56816248d9 Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Mon, 19 Jun 2017 16:35:40 -0700 Subject: [PATCH 1/6] Use factory `proto` to not prematurely create a controller --- addon/utils/lookup-controller.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/addon/utils/lookup-controller.js b/addon/utils/lookup-controller.js index 2fc7be4..7118bb2 100644 --- a/addon/utils/lookup-controller.js +++ b/addon/utils/lookup-controller.js @@ -11,5 +11,12 @@ const { get, getOwner } = Ember; * @returns {Ember.Controller} */ export default function lookupController(route, ownerLookupFn = getOwner) { - return route.controller || ownerLookupFn(route).lookup(`controller:${get(route, 'routeName')}`); + let controller = get(route, 'controller'); + + if (!controller) { + let factory = ownerLookupFn(route).factoryFor(`controller:${get(route, 'routeName')}`); + return factory.class.proto(); + } + + return controller; } From ded6de62b1ff4f3bba65bc302556d625c6128e6c Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Mon, 19 Jun 2017 16:55:37 -0700 Subject: [PATCH 2/6] Add some comments --- .../instance-initializers/ember-parachute.js | 42 +++++++++---------- addon/utils/lookup-controller.js | 6 +++ 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/addon/instance-initializers/ember-parachute.js b/addon/instance-initializers/ember-parachute.js index 29bc859..7645934 100644 --- a/addon/instance-initializers/ember-parachute.js +++ b/addon/instance-initializers/ember-parachute.js @@ -13,27 +13,6 @@ const { export function initialize(/* application */) { Ember.Route.reopen({ - actions: { - /** - * Route hook that fires when query params are changed. - * - * @public - * @param {object} [changed={}] - * @param {object} [present={}] - * @param {object} [removed={}] - * @returns {any} - */ - queryParamsDidChange(changed = {}, present = {}, removed = {}) { - let { controller, routeName } = this; - - if (QueryParams.hasParachute(controller)) { - this._scheduleParachuteChangeEvent(routeName, controller, assign({}, changed, removed)); - } - - return this._super(...arguments); - } - }, - /** * Serialize query param value if a given query param has a `serialize` * method. @@ -96,6 +75,27 @@ export function initialize(/* application */) { tryInvoke(controller, 'queryParamsDidChange', [changeEvent]); sendEvent(controller, 'queryParamsDidChange', [changeEvent]); }); + }, + + actions: { + /** + * Route hook that fires when query params are changed. + * + * @public + * @param {object} [changed={}] + * @param {object} [present={}] + * @param {object} [removed={}] + * @returns {any} + */ + queryParamsDidChange(changed = {}, present = {}, removed = {}) { + let { controller, routeName } = this; + + if (QueryParams.hasParachute(controller)) { + this._scheduleParachuteChangeEvent(routeName, controller, assign({}, changed, removed)); + } + + return this._super(...arguments); + } } }); } diff --git a/addon/utils/lookup-controller.js b/addon/utils/lookup-controller.js index 7118bb2..7a42bab 100644 --- a/addon/utils/lookup-controller.js +++ b/addon/utils/lookup-controller.js @@ -14,6 +14,12 @@ export default function lookupController(route, ownerLookupFn = getOwner) { let controller = get(route, 'controller'); if (!controller) { + /** + * If the controller doesnt exist, use the class proto instead. Before, we + * would create the controller if it didnt exist which caused a lot of issues + * (especially with authentication) due to the controller being created + * prematurely. + */ let factory = ownerLookupFn(route).factoryFor(`controller:${get(route, 'routeName')}`); return factory.class.proto(); } From d2ab4be7e0970d195db415f0244ec2c8e998a184 Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Wed, 21 Jun 2017 13:12:57 -0700 Subject: [PATCH 3/6] Force rebuild node-sass --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index d933820..ab44625 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,6 +38,7 @@ before_install: install: - yarn install --no-lockfile + - npm rebuild node-sass script: # Usually, it's ok to finish the test scenario without reverting From 7922b89d904d397ef098b360bed49880ce69c6b5 Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Wed, 21 Jun 2017 13:20:57 -0700 Subject: [PATCH 4/6] Move rebuild node-sass to before_script --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index ab44625..48d46c8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,6 +38,8 @@ before_install: install: - yarn install --no-lockfile + +before_script: - npm rebuild node-sass script: From 10bd3c6df56b82b9105dc288553208c315771dd7 Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Wed, 21 Jun 2017 13:29:51 -0700 Subject: [PATCH 5/6] Fix test cases --- tests/unit/utils/lookup-controller-test.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/unit/utils/lookup-controller-test.js b/tests/unit/utils/lookup-controller-test.js index 3fb8cbe..748d86f 100644 --- a/tests/unit/utils/lookup-controller-test.js +++ b/tests/unit/utils/lookup-controller-test.js @@ -1,22 +1,26 @@ +import Ember from 'ember'; import lookupController from 'ember-parachute/utils/lookup-controller'; import { module, test } from 'qunit'; module('Unit | Utility | lookup controller'); const dummyRoute = { - controller: { isController: true } + controller: Ember.Controller.create() } + function dummyLookup() { return { - lookup() { - return { isController: true }; + factoryFor() { + return { + class: Ember.Controller + }; } }; } test('it looks up the controller from a route', function(assert) { let result = lookupController(dummyRoute); - assert.ok(result.isController); + assert.equal(result, dummyRoute.controller); }); test('it looks up the controller from a route owner if route controller is not defined', function(assert) { From e9690836fbee2d52dfacfdfad7afe6da3125335b Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Wed, 21 Jun 2017 13:33:00 -0700 Subject: [PATCH 6/6] Fix test case --- tests/unit/utils/lookup-controller-test.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/unit/utils/lookup-controller-test.js b/tests/unit/utils/lookup-controller-test.js index 748d86f..b10ac89 100644 --- a/tests/unit/utils/lookup-controller-test.js +++ b/tests/unit/utils/lookup-controller-test.js @@ -4,15 +4,20 @@ import { module, test } from 'qunit'; module('Unit | Utility | lookup controller'); +const Controller = Ember.Controller.extend({ + parachuteController: true +}) + const dummyRoute = { - controller: Ember.Controller.create() + controller: Controller.create() } function dummyLookup() { return { factoryFor() { return { - class: Ember.Controller + create: () => Controller.create(), + class: Controller }; } }; @@ -20,10 +25,10 @@ function dummyLookup() { test('it looks up the controller from a route', function(assert) { let result = lookupController(dummyRoute); - assert.equal(result, dummyRoute.controller); + assert.ok(result.parachuteController); }); test('it looks up the controller from a route owner if route controller is not defined', function(assert) { let result = lookupController({}, dummyLookup); - assert.ok(result.isController); + assert.ok(result.parachuteController); });