Implement no-route-action rule

[thiagofesta]

no-route-action

This rule disallows the usage of route-action.

ember-route-action-helper was a popular addon used to add actions to a route without creating a separate controller. Given the changes in Ember since ember-route-action-helper was a widely used pattern, controllers are now encouraged and we want to discourage the use of route-action.

Most route actions should either be sent to the controller first or encapsulated within a downstream component instead. We should never be escaping the DDAU hierarchy to lob actions up to the route.

Examples

This rule forbids the following:

<CustomComponent @onUpdate={{route-action 'updateFoo'}} />

<CustomComponent @onUpdate={{route-action 'updateFoo' 'bar'}} />

{{custom-component onUpdate=(route-action 'updateFoo')}}

{{custom-component onUpdate=(route-action 'updateFoo' 'bar')}}

With the given route:

// app/routes/foo.js
export default class extends Route {
  @action
  updateFoo(baz) {
    // ...
  }
}

This rule allows the following:

<CustomComponent @onUpdate={{this.updateFoo}} />

<CustomComponent @onUpdate={{fn this.updateFoo 'bar'}} />

{{custom-component onUpdate=this.updateFoo}}

{{custom-component onUpdate=(fn this.updateFoo 'bar')}}

With the given controller:

// app/controllers/foo.js
export default class extends Controller {
  @action
  updateFoo(baz) {
    // ...
  }
}

Migration

The example below shows how to migrate from route-action to controller actions.

Before

// app/routes/posts.js
export default class extends Route {
  model(params) {
    return this.store.query('post', { page: params.page })
  }

  @action
  goToPage(pageNum) {
    this.transitionTo({ queryParams: { page: pageNum } });
  }
}

// app/controllers/posts.js
export default class extends Controller {
  queryParams = ['page'];
  page = 1;
}

{{#each @model as |post|}}
  <Post @title={{post.title}} @content={{post.content}} />
{{/each}}

<button {{action (route-action 'goToPage' 1)}}>1</button>
<button {{action (route-action 'goToPage' 2)}}>2</button>
<button {{action (route-action 'goToPage' 3)}}>3</button>

After

// app/routes/posts.js
export default class extends Route {
  model(params) {
    return this.store.query('post', { page: params.page })
  }
}

// app/controllers/posts.js
export default class extends Controller {
  queryParams = ['page'];
  page = 1;

  @action
  goToPage(pageNum) {
    this.transitionToRoute({ queryParams: { page: pageNum } });
  }
}

{{#each @model as |post|}}
  <Post @title={{post.title}} @content={{post.content}} />
{{/each}}

<button {{on 'click' (fn this.goToPage 1)}}>1</button>
<button {{on 'click' (fn this.goToPage 2)}}>2</button>
<button {{on 'click' (fn this.goToPage 3)}}>3</button>

References

• ember-route-action-helper
• Ember guides/Controllers
• Ember Best Practices: What are controllers good for?

[elwayman02]

I think this is a fantastic rule because, despite still being on Ember 2.14, having no updates in the last 2 years, and having had a warning in its README telling people not to use it for the last 4 years, ember-route-action-helper is still in the top 10% of most downloaded addons: https://emberobserver.com/addons/ember-route-action-helper

It's clear that this is a common problem that would help the community by adding an optional lint rule, and rules like no-mut-helper (which suggests ember-set-helper as replacement/correct code) have already established that it's reasonable to have context of open-source addons within some of the optional lint rules provided by this package.

[bertdeblock]

What about trying to contact the maintainers and see if the addon can officially be deprecated?

[elwayman02]

@bertdeblock I filed this ticket to see if the maintainers are willing to take action on that suggestion: DockYard/ember-route-action-helper#109

[thiagofesta]

@bertdeblock great suggestion, thanks @elwayman02 for sending the request out!

[elwayman02]

@scalvert @bmish @rwjblue are any of you able to take a look at this PR? We'd love to get this merged soon for some ongoing projects.

[scalvert]

This looks good overall. I do think it a bit odd to use function.bind rather than methods of the class in the rules, and using a bespoke string replacement mechanism for the error message, but I realize you're just following the pattern in no-action, so I wouldn't suggest changing it now.

Thanks for adding this!

[scalvert]

It's possible there's more test cases we could/should add, but I'm comfortable merging this for now with an eye to expanding those cases as they arise.

[bertdeblock]

The addon doesn't contain a route-action modifier, yet this rule checks for it. Any reason?

[thiagofesta]

The addon doesn't contain a route-action modifier, yet this rule checks for it. Any reason?

No reason, that is a mistake. As @scalvert mentioned, I based on this from no-action and I didn't notice that route-action doesn't have a modifier, I assumed it had.

I will remove it and put out a PR soon, thanks.

[AmilKey]

@thiagofesta before we called route-actions in child routes, and the action was declared at the top route level, for example:

route1 -> route2 -> route3 (call route-action "route1")
      \-> route4 -> route5 (call route-action "route1")

please tell me what is the best practice in this case now