Repository: ember-cli/eslint-plugin-ember
Pull request #124 (2017-08-08 03:46:38)

Order beforeModel and afterModel around model
I'm working on enforcing that route lifecycle hooks are in order by default. The first step I did was beforemodel -> model -> afterModel. I wasn't sure if it made sense to merge this separately, or at least review it and let me know if it is done correctly.

kevinkucharczyk: While I do like the idea of enforcing an order for the lifecycle hooks, I'm not sure whether this is the right way to do it. This is possibly just adding confusion, since we already have a `lifecycle-hooks` group in there.
I think that perhaps a better approach would be to move `model` into the `lifecycle-hooks` group and then add additional logic for sorting `lifecycle-hooks` themselves.
We could add a configuration option for those hooks, by making it accept an array, like
```
'lifecycle-hooks': ['beforeModel', 'model', 'afterModel', 'setupController']
```
michalsnik: I mostly agree with @kevinkucharczyk , but I was also thinking about getting rid of `lifecycle-hooks` group, then we could come up with the following configuration:

```
ember/order-in-routes: [2, {
  order: [
    'service',
    'inherited-property',
    'property',
    'beforeModel',
    'model',
    'afterModel',
    ['setupController', 'didTransition', '...']
    'actions',
    'method',
  ]
}]
```

This way we would take advantage of currently available grouping feature and all other explicitly listed hooks would be treated equally.
rwwagner90: @kevinkucharczyk @michalsnik yeah, the idea was we should remove all lifecycle hooks and list them explicitly for ordering. This PR is not meant to be merged yet, but I wanted to get feedback on how I was doing things before I continued down the path. If you are both happy with how I implemented `beforeModel` and `afterModel`, I will complete the rest of the hooks.
rwwagner90: @kevinkucharczyk @michalsnik I added more route hooks, but not sure if I have gotten them all. It feels very verbose having a separate method to check like `isBeforeModelProp`. Shouldn't we just have one method that checks all route props and returns the mapping or something? I was just trying to follow how `model` was done. Please let me know your thoughts, so we can tweak this if necessary and get it in 😄 
kevinkucharczyk: @rwwagner90 I agree, it is quite verbose. Perhaps you could define one `isRouteLifecycleHook` method, instead of a bunch of separate methods for each hook? I don't think we'll need those hook-specific methods in other places, so it should be fine.
We could later introduce similar methods for component hooks, like `isComponentLifeCycleHook`.
michalsnik: We already have `isComponentLifecycleHook` helper: https://github.com/ember-cli/eslint-plugin-ember/blob/master/lib/utils/ember.js#L204 as well as `isRouteDefaultMethod` with all methods available on routes (https://github.com/ember-cli/eslint-plugin-ember/blob/master/lib/utils/ember.js#L271) It might be worth using them instead :)

Then simple:
```
else if (ember.isRouteDefaultMethod(node)) {
  return node.key.name;
} 
```
might be enough
kevinkucharczyk: Thanks @michalsnik! Though I think it would still be good to introduce a separate method for just the route lifecycle hooks. `isRouteDefaultMethod` checks a lot of stuff, which we don't want/need here.
rwwagner90: @kevinkucharczyk @michalsnik thanks for those suggestions! I made an `isRouteLifecycleHook` method, and made it much less verbose. How is this looking now? Anything else I should do? The one verbose thing left, that we could maybe clean up, is this:

```
const NAMES = {
  service: 'service injection',
  property: 'property',
  'single-line-function': 'single-line function',
  'multi-line-function': 'multi-line function',
  observer: 'observer',
  beforeModel: 'lifecycle hook',
  model: '"model" hook',
  afterModel: 'lifecycle hook',
  serialize: 'lifecycle hook',
  redirect: 'lifecycle hook',
  activate: 'lifecycle hook',
  setupController: 'lifecycle hook',
  renderTemplate: 'lifecycle hook',
  resetController: 'lifecycle hook',
  deactivate: 'lifecycle hook',
  'lifecycle-hook': 'lifecycle hook',
  actions: 'actions hash',
  method: 'method',
  unknown: 'unknown property type',
  attribute: 'attribute',
  relationship: 'relationship',
  'query-params': 'property'
};
```
rwwagner90: @kevinkucharczyk @michalsnik any other tweaks you would like? I can start on the component one as well, if this looks good 😄 
michalsnik: Additionally please fix eslint issue so the build is green @rwwagner90 :)
rwwagner90: @michalsnik I removed those two methods and fixed ESLint. Please let me know if there is anything else I should do.
rwwagner90: Also, I suppose I should update the docs to reflect these new changes, right?
michalsnik: Yeah, please update docs too.

I'm only worried about backwards compatibility. I would preferably release it in minor update, but in it's current state it will most likely be a breaking change. If we make sure however that `lifecycle-hook` still works and is properly detected as any other lifecycle hook we could release it without breaking any apps' build. We'd need to also change the default order to be:
```
[
  'property',
  'single-line-function',
  'multi-line-function',
  'model',
  [
    'beforeModel',
    'lifecycle-hook',
    'afterModel',
    'serialize',
    'redirect',
    'activate',
    'setupController',
    'renderTemplate',
    'resetController',
    'deactivate'
  ],
  'actions',
  'method'
]
```

So summarising:
I think we should support both configs, the new one ☝️  and the previous one 👇 :

```
[
  'property',
  'single-line-function',
  'multi-line-function',
  'model',
  'lifecycle-hooks',
  'actions',
  'method'
]
```

This way we'll be able to introduce this change as in the minor release. And then together with new rules that we added recently we'll update default recommendations and do one major release. What do you think guys? @rwwagner90 @kevinkucharczyk 
rwwagner90: @michalsnik that sounds like a lot of work to support both. I'd prefer to not do that. We could just update models and components and docs all as part of this PR too, and get it in for the next breaking change.
michalsnik: We could do that, but the change might not be so complicated after all, all you'd need to do is to parse settings and replace `lifecycle-hook` with array of all lifecycle hooks except `modal`. That's my first thought, let me know how do you feel about it :)
rwwagner90: @michalsnik I think I implemented what you are referring to in my last commit. Please let me know if you think this will work!
michalsnik: We should add more tests now, as we can pass different settings per test and make sure it works as expected with both settings :)
michalsnik: But overall looks good, I expect it to work properly. Though I'll test it manually too
rwwagner90: @michalsnik there is a test that I accidentally left `lifecycle-hook` in that works. I can add some more tests though.
rwwagner90: @michalsnik I added a couple more tests, and updated the docs. Please let me know if I should do anything else 😄 
rwwagner90: @michalsnik anything else I need to do before we can merge this in? Should I do components and models as part of the same PR or do them separate later?
rwwagner90: @michalsnik @kevinkucharczyk anything stopping us from merging this?
rwwagner90: @michalsnik @kevinkucharczyk bump
rwwagner90: @michalsnik @kevinkucharczyk haven't heard from you guys in awhile. Could one of you please give this one more quick look and merge it in? I think it's good to go.
michalsnik: Hey, sorry @rwwagner90 I've been quite busy recently and now I'm on a vacation. I'll take a look in the next week, I hope it's fine :)
rwwagner90: @michalsnik that's fine, didn't realize you were on vacation. Thanks for the reply 😄 
rwwagner90: @michalsnik by next week, did you mean this week or the following week?
michalsnik: Alright, I think we're good @rwwagner90 . I resolved some conflicts and will merge it as soon as the travis check passes 🚀 Great job! And sorry for such a delay.

Patch:
diff --git a/docs/rules/order-in-routes.md b/docs/rules/order-in-routes.md
index 0c79d147..46a805cf 100644
--- a/docs/rules/order-in-routes.md
+++ b/docs/rules/order-in-routes.md
@@ -10,8 +10,18 @@ ember/order-in-routes: [2, {
     'service',
     'inherited-property',
     'property',
+    'single-line-function',
+    'multi-line-function',
+    'beforeModel',
     'model',
-    'lifecycle-hook',
+    'afterModel',
+    'serialize',
+    'redirect',
+    'activate',
+    'setupController',
+    'renderTemplate',
+    'resetController',
+    'deactivate',
     'actions',
     ['method', 'empty-method'],
   ]
@@ -25,7 +35,18 @@ order: [
   'service',
   ['inherited-property', 'property'],
   'model',
-  'lifecycle-hook',
+  [
+    'beforeModel',
+    'model',
+    'afterModel',
+    'serialize',
+    'redirect',
+    'activate',
+    'setupController',
+    'renderTemplate',
+    'resetController',
+    'deactivate'
+  ],
   'actions',
   ['method', 'empty-method'],
 ]
@@ -40,10 +61,12 @@ You should write code grouped and ordered in this way:
 1. Services
 2. Default route's properties
 3. Custom properties
-4. model() hook
-5. Other route's methods (beforeModel etc.)
-6. Actions
-7. Custom / private methods
+4. beforeModel() hook
+5. model() hook
+6. afterModel() hook
+7. Other lifecycle hooks in execution order (serialize, redirect, etc)
+8. Actions
+9. Custom / private methods
 
 ```javascript
 const { Route, inject: { service }, get } = Ember;
@@ -60,26 +83,38 @@ export default Route.extend({
   // 3. Custom properties
   customProp: 'test',
 
-  // 4. Model hook
-  model() {
-    return this.store.findAll('article');
-  },
-
-  // 5. Other route's methods
+  // 4. beforeModel hook
   beforeModel() {
     if (!get(this, 'currentUser.isAdmin')) {
       this.transitionTo('index');
     }
   },
+  
+  // 5. model hook
+  model() {
+    return this.store.findAll('article');
+  },
+  
+  // 6. afterModel hook
+  afterModel(articles) {
+    articles.forEach((article) => {
+      article.set('foo', 'bar');
+    });
+  },
+
+  // 7. Other route's methods
+  setupController(controller) {
+    controller.set('foo', 'bar');
+  },
 
-  // 6. All actions
+  // 8. All actions
   actions: {
     sneakyAction() {
       return this._secretMethod();
     },
   },
 
-  // 7. Custom / private methods
+  // 9. Custom / private methods
   _secretMethod() {
     // custom secret method logic
   },
diff --git a/lib/rules/order-in-routes.js b/lib/rules/order-in-routes.js
index 68af1d1b..a3e5fe6f 100644
--- a/lib/rules/order-in-routes.js
+++ b/lib/rules/order-in-routes.js
@@ -12,10 +12,18 @@ const ORDER = [
   'property',
   'single-line-function',
   'multi-line-function',
+  'beforeModel',
   'model',
-  'lifecycle-hook',
+  'afterModel',
+  'serialize',
+  'redirect',
+  'activate',
+  'setupController',
+  'renderTemplate',
+  'resetController',
+  'deactivate',
   'actions',
-  ['method', 'empty-method'],
+  ['method', 'empty-method']
 ];
 
 //------------------------------------------------------------------------------
@@ -29,20 +37,37 @@ module.exports = {
       category: 'Organizing',
       recommended: true
     },
-    fixable: null, // or "code" or "whitespace"
+    fixable: null // or "code" or "whitespace"
   },
 
   create(context) {
     const options = context.options[0] || {};
+    let order = options.order ? addBackwardsPosition(options.order, 'empty-method', 'method') : ORDER;
+    order = order.slice(0);
+    const indexOfLifecycleHook = order.indexOf('lifecycle-hook');
+
+    if (indexOfLifecycleHook !== -1) {
+      order.splice(indexOfLifecycleHook, 1, [
+        'beforeModel',
+        'afterModel',
+        'serialize',
+        'redirect',
+        'activate',
+        'setupController',
+        'renderTemplate',
+        'resetController',
+        'deactivate'
+      ]);
+    }
+
     const filePath = context.getFilename();
-    const order = options.order ? addBackwardsPosition(options.order, 'empty-method', 'method') : ORDER;
 
     return {
       CallExpression(node) {
         if (!ember.isEmberRoute(node, filePath)) return;
 
         reportUnorderedProperties(node, context, 'route', order);
-      },
+      }
     };
   }
 };
diff --git a/lib/utils/ember.js b/lib/utils/ember.js
index e20caebb..283eac61 100644
--- a/lib/utils/ember.js
+++ b/lib/utils/ember.js
@@ -27,7 +27,7 @@ module.exports = {
   isComputedProp,
   isCustomProp,
   isActionsProp,
-  isModelProp,
+  isRouteLifecycleHook,
 
   isRelation,
 
@@ -36,8 +36,6 @@ module.exports = {
   isComponentCustomFunction,
 
   isRoute,
-  isRouteMethod,
-  isRouteDefaultMethod,
   isRouteCustomFunction,
   isRouteProperty,
   isRouteDefaultProp,
@@ -177,8 +175,9 @@ function isCustomProp(property) {
     utils.isTaggedTemplateExpression(value);
 }
 
-function isModelProp(property) {
-  return property.key.name === 'model' && utils.isFunctionExpression(property.value);
+function isRouteLifecycleHook(property) {
+  return isFunctionExpression(property.value) &&
+    isRouteLifecycleHookName(property.key.name);
 }
 
 function isActionsProp(property) {
@@ -218,64 +217,24 @@ function isRoute(node) {
     node.callee.property.name === 'route';
 }
 
-function isRouteMethod(name) {
+function isRouteLifecycleHookName(name) {
   return [
     'activate',
-    'addObserver',
     'afterModel',
     'beforeModel',
-    'cacheFor',
-    'controllerFor',
-    'create',
     'deactivate',
-    'decrementProperty',
-    'destroy',
-    'disconnectOutlet',
-    'extend',
-    'get',
-    'getProperties',
-    'getWithDefault',
-    'has',
-    'incrementProperty',
-    'init',
-    'intermediateTransitionTo',
     'model',
-    'modelFor',
-    'notifyPropertyChange',
-    'off',
-    'on',
-    'one',
-    'paramsFor',
     'redirect',
-    'refresh',
-    'removeObserver',
-    'render',
     'renderTemplate',
-    'reopen',
-    'reopenClass',
-    'replaceWith',
     'resetController',
-    'send',
     'serialize',
-    'set',
-    'setProperties',
-    'setupController',
-    'toString',
-    'toggleProperty',
-    'transitionTo',
-    'trigger',
-    'willDestroy',
+    'setupController'
   ].indexOf(name) > -1;
 }
 
-function isRouteDefaultMethod(property) {
-  return isFunctionExpression(property.value) &&
-    isRouteMethod(property.key.name);
-}
-
 function isRouteCustomFunction(property) {
   return isFunctionExpression(property.value) &&
-    !isRouteMethod(property.key.name);
+    !isRouteLifecycleHookName(property.key.name);
 }
 
 function isRouteProperty(name) {
diff --git a/lib/utils/property-order.js b/lib/utils/property-order.js
index 164768d6..b9b653e2 100644
--- a/lib/utils/property-order.js
+++ b/lib/utils/property-order.js
@@ -6,7 +6,7 @@ const utils = require('./utils');
 module.exports = {
   determinePropertyType,
   reportUnorderedProperties,
-  addBackwardsPosition,
+  addBackwardsPosition
 };
 
 const NAMES = {
@@ -15,7 +15,16 @@ const NAMES = {
   'single-line-function': 'single-line function',
   'multi-line-function': 'multi-line function',
   observer: 'observer',
+  beforeModel: 'lifecycle hook',
   model: '"model" hook',
+  afterModel: 'lifecycle hook',
+  serialize: 'lifecycle hook',
+  redirect: 'lifecycle hook',
+  activate: 'lifecycle hook',
+  setupController: 'lifecycle hook',
+  renderTemplate: 'lifecycle hook',
+  resetController: 'lifecycle hook',
+  deactivate: 'lifecycle hook',
   'lifecycle-hook': 'lifecycle hook',
   actions: 'actions hash',
   method: 'method',
@@ -23,7 +32,7 @@ const NAMES = {
   unknown: 'unknown property type',
   attribute: 'attribute',
   relationship: 'relationship',
-  'query-params': 'property',
+  'query-params': 'property'
 };
 
 function determinePropertyType(node, parentType) {
@@ -56,10 +65,8 @@ function determinePropertyType(node, parentType) {
   if (parentType === 'route') {
     if (ember.isRouteDefaultProp(node)) {
       return 'inherited-property';
-    } else if (ember.isModelProp(node)) {
-      return 'model';
-    } else if (ember.isRouteDefaultMethod(node)) {
-      return 'lifecycle-hook';
+    } else if (ember.isRouteLifecycleHook(node)) {
+      return node.key.name;
     }
   }
 
@@ -114,7 +121,9 @@ function getOrder(ORDER, type) {
 
 function getName(type, node) {
   let prefix;
-  if (!node.computed && type !== 'actions' && type !== 'model') {
+  if (!node.computed &&
+    type !== 'actions' &&
+    type !== 'model') {
     if (node.key.type === 'Identifier') {
       prefix = node.key.name;
     } else if (node.key.type === 'Literal' && typeof node.key.value === 'string') {
@@ -142,7 +151,7 @@ function reportUnorderedProperties(node, context, parentType, ORDER) {
 
     const info = {
       node: property,
-      type,
+      type
     };
 
     // check if this property should be moved further upwards


Test patch:
diff --git a/tests/lib/rules/order-in-routes.js b/tests/lib/rules/order-in-routes.js
index baf84de2..57338bbd 100644
--- a/tests/lib/rules/order-in-routes.js
+++ b/tests/lib/rules/order-in-routes.js
@@ -14,7 +14,7 @@ eslintTester.run('order-in-routes', rule, {
   valid: [
     {
       code: 'export default Route.extend();',
-      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
     },
     {
       code: `export default Route.extend({
@@ -24,14 +24,22 @@ eslintTester.run('order-in-routes', rule, {
         vehicle: alias("car"),
         levelOfHappiness: computed("attitude", "health", () => {
         }),
-        model() {},
         beforeModel() {},
+        model() {},
+        afterModel() {},
+        serialize() {},
+        redirect() {},
+        activate() {},
+        setupController() {},
+        renderTemplate() {},
+        resetController() {},
+        deactivate() {},
         actions: {},
         _customAction() { const foo = 'bar'; },
         _customAction2: function() { const foo = 'bar'; },
         tSomeTask: task(function* () {})
       });`,
-      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
     },
     {
       code: `export default Route.extend({
@@ -41,14 +49,15 @@ eslintTester.run('order-in-routes', rule, {
         vehicle: alias("car"),
         levelOfHappiness: computed("attitude", "health", () => {
         }),
-        model() {},
         beforeModel() {},
+        model() {},
+        afterModel() {},
         actions: {},
         _customAction() {},
         _customAction2: function() {},
         tSomeTask: task(function* () {})
       });`,
-      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
     },
     {
       code: `export default Route.extend({
@@ -60,7 +69,7 @@ eslintTester.run('order-in-routes', rule, {
         },
         _customAction() { const foo = 'bar'; }
       });`,
-      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
     },
     {
       code: `export default Route.extend({
@@ -68,7 +77,7 @@ eslintTester.run('order-in-routes', rule, {
         render() {},
         init() {}
       });`,
-      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
     },
     {
       code: `export default Route.extend({
@@ -79,7 +88,7 @@ eslintTester.run('order-in-routes', rule, {
         model() {},
         actions: {}
       });`,
-      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
     },
     {
       code: `export default Route.extend({
@@ -88,23 +97,60 @@ eslintTester.run('order-in-routes', rule, {
         vehicle: alias("car"),
         model() {}
       });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+    },
+    {
+      code: `export default Route.extend({
+        model() {},
+        beforeModel() {},
+        currentUser: service(),
+      });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      options: [{
+        order: [
+          'model',
+          'lifecycle-hook',
+          'service'
+        ]
+      }]
     },
     {
       code: `export default Route.extend({
+        deactivate() {},
         beforeModel() {},
-        model() {},
         currentUser: service(),
+        model() {}
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       options: [{
         order: [
           'lifecycle-hook',
-          'model',
           'service',
-        ],
-      }],
+          'model'
+        ]
+      }]
     },
+    {
+      code: `export default Route.extend({
+        deactivate() {},
+        setupController() {},
+        beforeModel() {},
+        currentUser: service(),
+        model() {}
+      });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      options: [{
+        order: [
+          [
+            'deactivate',
+            'setupController',
+            'beforeModel'
+          ],
+          'service',
+          'model'
+        ]
+      }]
+    }
   ],
   invalid: [
     {
@@ -112,8 +158,8 @@ eslintTester.run('order-in-routes', rule, {
         queryParams: {},
         currentUser: service(),
         customProp: "test",
-        model() {},
         beforeModel() {},
+        model() {},
         vehicle: alias("car"),
         actions: {},
         _customAction() {}
@@ -121,19 +167,19 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "currentUser" service injection should be above the inherited "queryParams" property on line 2',
-        line: 3,
+        line: 3
       }, {
-        message: 'The "vehicle" single-line function should be above the "model" hook on line 5',
-        line: 7,
-      }],
+        message: 'The "vehicle" single-line function should be above the "beforeModel" lifecycle hook on line 5',
+        line: 7
+      }]
     },
     {
       code: `export default Route.extend({
         queryParams: {},
         currentUser: inject(),
         customProp: "test",
-        model() {},
         beforeModel() {},
+        model() {},
         vehicle: alias("car"),
         actions: {},
         _customAction() {}
@@ -141,18 +187,18 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "currentUser" service injection should be above the inherited "queryParams" property on line 2',
-        line: 3,
+        line: 3
       }, {
-        message: 'The "vehicle" single-line function should be above the "model" hook on line 5',
-        line: 7,
-      }],
+        message: 'The "vehicle" single-line function should be above the "beforeModel" lifecycle hook on line 5',
+        line: 7
+      }]
     },
     {
       code: `export default Route.extend({
         customProp: "test",
         queryParams: {},
-        model() {},
         beforeModel() {},
+        model() {},
         actions: {},
         _customAction() {},
         levelOfHappiness: computed("attitude", "health", () => {
@@ -161,29 +207,29 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The inherited "queryParams" property should be above the "customProp" property on line 2',
-        line: 3,
+        line: 3
       }, {
-        message: 'The "levelOfHappiness" multi-line function should be above the "model" hook on line 4',
-        line: 8,
-      }],
+        message: 'The "levelOfHappiness" multi-line function should be above the "beforeModel" lifecycle hook on line 4',
+        line: 8
+      }]
     },
     {
       code: `export default Route.extend({
         customProp: "test",
         queryParams: {},
-        beforeModel() {},
         model() {},
+        beforeModel() {},
         actions: {},
         _customAction() {}
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The inherited "queryParams" property should be above the "customProp" property on line 2',
-        line: 3,
+        line: 3
       }, {
-        message: 'The "model" hook should be above the "beforeModel" lifecycle hook on line 4',
-        line: 5,
-      }],
+        message: 'The "beforeModel" lifecycle hook should be above the "model" hook on line 4',
+        line: 5
+      }]
     },
     {
       code: `export default Route.extend({
@@ -197,11 +243,11 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "customProp" property should be above the "vehicle" single-line function on line 3',
-        line: 4,
+        line: 4
       }, {
         message: 'The actions hash should be above the "_customAction" method on line 6',
-        line: 7,
-      }],
+        line: 7
+      }]
     },
     {
       code: `export default Route.extend({
@@ -212,8 +258,8 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "customProp" property should be above the "model" hook on line 2',
-        line: 3,
-      }],
+        line: 3
+      }]
     },
     {
       code: `export default Route.extend({
@@ -224,8 +270,49 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The inherited "mergedProperties" property should be above the "test" property on line 2',
-        line: 3,
-      }],
+        line: 3
+      }]
+    },
+    {
+      code: `export default Route.extend({
+        currentUser: service(),
+        queryParams: {},
+        customProp: "test",
+        vehicle: alias("car"),
+        levelOfHappiness: computed("attitude", "health", () => {
+        }),
+        beforeModel() {},
+        model() {},
+        afterModel() {},
+        setupController() {},
+        redirect() {},
+        serialize() {},
+        activate() {},
+        deactivate() {},
+        renderTemplate() {},
+        resetController() {},
+        actions: {},
+        _customAction() {},
+        _customAction2: function() {},
+        tSomeTask: task(function* () {})
+      });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'The "redirect" lifecycle hook should be above the "setupController" lifecycle hook on line 11',
+        line: 12
+      }, {
+        message: 'The "serialize" lifecycle hook should be above the "setupController" lifecycle hook on line 11',
+        line: 13
+      }, {
+        message: 'The "activate" lifecycle hook should be above the "setupController" lifecycle hook on line 11',
+        line: 14
+      }, {
+        message: 'The "renderTemplate" lifecycle hook should be above the "deactivate" lifecycle hook on line 15',
+        line: 16
+      }, {
+        message: 'The "resetController" lifecycle hook should be above the "deactivate" lifecycle hook on line 15',
+        line: 17
+      }]
     },
     {
       code: `export default Route.extend({
@@ -236,8 +323,8 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "model" hook should be above the "_test2" method on line 3',
-        line: 4,
-      }],
+        line: 4
+      }]
     },
     {
       filename: 'example-app/routes/some-route.js',
@@ -248,8 +335,8 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "test" property should be above the "model" hook on line 2',
-        line: 3,
-      }],
+        line: 3
+      }]
     },
     {
       filename: 'example-app/some-feature/route.js',
@@ -260,8 +347,8 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "test" property should be above the "model" hook on line 2',
-        line: 3,
-      }],
+        line: 3
+      }]
     },
     {
       filename: 'example-app/twisted-path/some-file.js',
@@ -272,8 +359,8 @@ eslintTester.run('order-in-routes', rule, {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The "test" property should be above the "model" hook on line 2',
-        line: 3,
-      }],
-    },
-  ],
+        line: 3
+      }]
+    }
+  ]
 });
diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js
index ed82fe95..15c72ca2 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -327,15 +327,39 @@ describe('isCustomProp', () => {
   });
 });
 
-describe('isModelProp', () => {
+describe('isRouteLifecycleHook', () => {
   let node;
 
-  it('should be a model prop', () => {
+  it('should be a route lifecycle hook', () => {
+    node = getProperty('test = { beforeModel() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
     node = getProperty('test = { model() {} }');
-    expect(emberUtils.isModelProp(node)).toBeTruthy();
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
+    node = getProperty('test = { afterModel() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
+    node = getProperty('test = { serialize() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
+    node = getProperty('test = { redirect() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
+    node = getProperty('test = { activate() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
+    node = getProperty('test = { setupController() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
+    node = getProperty('test = { renderTemplate() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
+
+    node = getProperty('test = { resetController() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
 
-    node = getProperty('test = { model: function() {} }');
-    expect(emberUtils.isModelProp(node)).toBeTruthy();
+    node = getProperty('test = { deactivate() {} }');
+    expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
   });
 });