guides-source icon indicating copy to clipboard operation
guides-source copied to clipboard

Example in guide suggests accessing controller in route, which causes linter error

Open silhusk opened this issue 5 years ago • 4 comments

The example on Preventing Transitions via willTransition is using this.controller from inside route willTransition. This behaviour is flagged as error ember/no-controller-access-in-routes by es-lint.

error  Do not access controller in route outside of setupController/resetController  ember/no-controller-access-in-routes

Is there a better (not discouraged) way to implement the example? Or should the linter take this use case into account?

silhusk avatar Oct 30 '20 16:10 silhusk

Hi, @rwjblue. I was wondering if you can help us by recommending an action for the Ember Guides / eslint-plugin-ember. From https://github.com/ember-cli/eslint-plugin-ember/issues/896, I thought that you may be able to provide context.

The API Guides for willTransition also shows the example of preventing a route transition when a form hasn't been saved.

Similar questions on Discord:

  • https://discord.com/channels/480462759797063690/483601670685720591/757978312554316018
  • https://discord.com/channels/480462759797063690/483601670685720591/754986510578745404

ijlee2 avatar Oct 30 '20 17:10 ijlee2

I also see controllerFor recommended by the guides for handling loading actions in routes: https://guides.emberjs.com/release/routing/loading-and-error-substates/

More context from Discord here: https://discord.com/channels/480462759797063690/480777444203429888/886001971918696448

elwayman02 avatar Sep 10 '21 22:09 elwayman02

Hi @ijlee2 @rwjblue , is there any update on this ? :slightly_smiling_face:

Ideas

Remove the lint error

Maybe the eslint rule should just be removed as no alternative has been given since more than a year, and its still present in many parts of the latest Ember release documentation.

Set router event listeners in activate and update lint to allows controllers access in activate method

Or in init method directly, but activate may be more relevant.

activate() {
  this.activate(...arguments);
  this.on('routeWillChange', transition => {
    if (transition.to.find((route) => route.name === this.routeName)) return;
    if (this.controller.get('userHasEnteredData')) { // eslint accepts this
      this.controller.displayNavigationConfirm();
      transition.abort();
    }
  });
}

Manually set the controller on setupController

Probably not the way to go

init() {
  super.init(...arguments);
  this.router.on('routeWillChange', transition => {
    this.myController;
  });
}

setupController(controller) {
  super.setupController(controller);
  this.myController = controller;
}

Force users to use service for sharing controller stuff with router event callbacks

Pretty heavy changes for users, but maybe this is what the team expects ?

export default class MyRoute extends Route {
  @service myDataUsedByTheController;

  init() {
    this.init(...arguments);
    this.on('routeWillChange', transition => {
      if (this.myDataUsedByTheController.hasPendingChanges) {
        if (!confirm("Do you really want to discard your changes ?")) {
          transition.abort();
        }
      }
    });
  }
}

Documentation that triggers this linter error

Ember API Reference - Route#willTransition

@action
willTransition(transition) {
  if (this.controller.get('userHasEnteredData')) {
    this.controller.displayNavigationConfirm();
    transition.abort();
  }
}

I guess there are 2 things to do here :

1/ Warns users that this event will (probably) be deprecated and should be replaced with router service events. Like the example on the deprecate-router-events :

init() {
  this.init(...arguments);
  this.on('routeWillChange', transition => {
    if (!this.currentUser.isLoggedIn) {
      transition.abort();
      this.transitionTo('login');
    }
  });
}

2/ Give users the way to access controllers on this event callback.

Ember API Reference - Route#controller

@action
willTransition(transition) {
  if (this.controller.get('userHasEnteredData') &&
    !confirm('Are you sure you want to abandon progress?')) {
    transition.abort();
  } else {
    // Bubble the `willTransition` action so that
    // parent routes can decide whether or not to abort.
    return true;
  }
}

The same fix as the previous point should probably be done here.

Ember Guide Core Concept > Preventing and Retrying Transitions > Storing and retrying a transition

beforeModel(transition) {
  if (!this.controllerFor('auth').userIsLoggedIn) {
    let loginController = this.controllerFor('login');
    loginController.previousTransition = transition;
    this.router.transitionTo('login');
 }
}

The this.controllerFor calls triggers a lint error as it is called outside of setupController / resetController.

Related links :

  • https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md#new-events-routewillchange--routedidchange
  • https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-controller-access-in-routes.md
  • https://github.com/ember-cli/eslint-plugin-ember/issues/1108

lcoq avatar Mar 04 '22 15:03 lcoq

Hi, @lcoq. Thanks for asking for an update and providing possible solutions.

I'm not on the Learning Team now and don't think I'll have time to follow up on this issue, sorry. On Discord, I will forward the link to the issue to the core team and ask that they have a look at the issue.

Best,

ijlee2 avatar Mar 04 '22 19:03 ijlee2