Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
achambers opened this issue Feb 15, 2022 · 4 comments
Assignees
Labels

Comments

@achambers
Copy link
Contributor

achambers commented Feb 15, 2022

🐞 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:

return modelClass.find(value);

🔬 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 emberjs/rfcs#774

@achambers
Copy link
Contributor Author

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.

@locks
Copy link
Contributor

locks commented Feb 19, 2022

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!

@jelhan
Copy link
Contributor

jelhan commented Mar 26, 2022

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

@snewcomer
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants