ember.js icon indicating copy to clipboard operation
ember.js copied to clipboard

[Bug] When not providing a route `model` hook, and not providing a model with a `.find` function, an error occurs

Open achambers opened this issue 4 years ago • 4 comments

🐞 Describe the Bug

If I have a route for which I do not implement the model hook, Ember tries to be clever and make a request for the inferred model for the route. In order to do this it looks up a model:${name} factory and if it finds one, it calls the .find function on it. However, if the model factory does not have a find method we get an error.

The code has have dev assertions for the .find method, however, in prod these are stripped out resulting in a hard error.

The code in question is here: https://github.com/emberjs/ember.js/blob/8455848de50242657ae156848f05553a692afce1/packages/%40ember/-internals/routing/lib/system/route.ts#L1863

🔬 Minimal Reproduction

https://stackblitz.com/edit/github-xrliqn

  1. Define a route that takes a dynamic segment"
import EmberRouter from '@ember/routing/router';
import config from 'my-app/config/environment';

export default class Router extends EmberRouter {
  location = config.locationType;
  rootURL = config.rootURL;
}

Router.map(function () {
  this.route('pet', { path: '/pets/:pet_id' });
});
  1. Define an ED model
import Model, { attr } from '@ember-data/model';

export default class PetModel extends Model {
  @attr name;
}
  1. Define the route object without a model hook
import Route from '@ember/routing/route';

export default class PetRoute extends Route {
  beforeModel() {
    console.log('BeforeModel: ', arguments);
  }

  afterModel() {
    console.log('AfterModel: ', arguments);
  }
}
  1. Navigate to /pets/1

😕 Actual Behavior

The following error is thrown

image

🤔 Expected Behavior

Nothing should happen. There should be no error. And there should be no assertion in dev.

🌍 Environment

  • Ember: - I can find examples back to 3.5.1 but I'm pretty sure it goes back much further
  • Node.js/npm: - 14.17.0
  • OS: - Macos
  • Browser: - Brave

➕ Additional Context

This bug came about out of a discussion around https://github.com/emberjs/rfcs/pull/774

achambers avatar Feb 15 '22 09:02 achambers

Hi folks. This is my first issue. Hope I captured things satisfactorily. Let me know if I missed anything.

I'm looking to fix this too so can follow up with a PR soon.

achambers avatar Feb 15 '22 09:02 achambers

Thank you for both the detailed description of the issue and volunteering to take on the work! I have assigned you to the issue, let us know if you hit any snag!

locks avatar Feb 19 '22 00:02 locks

This looks like the implicit record loading. There is an open RFC to deprecate this feature as it is very confusing. https://github.com/emberjs/rfcs/pull/774

jelhan avatar Mar 26 '22 07:03 jelhan

@achambers @locks Should we consider an early safe return a "bugfix"? If I remove the assertions, I believe the behavior of this feature as written changes. For now, I think the behavior as stated is correct, albeit very much undesired. The fix is to define the explicit model hook and do nothing in the body.

And as @jelhan noted, if someone takes up this RFC (cough cough @chriskrycho or @locks), I will gladly implement with same day service. Removing this behavior is desperately needed for ember-data and the community as a whole.

snewcomer avatar Apr 23 '22 12:04 snewcomer