Skip to content

Commit

Permalink
[BUGFIX release] Add model hook in route blueprint
Browse files Browse the repository at this point in the history
When generating a route with a dynamic segment, say via:

    ember g route foo --path="bar/:buzz_id"

The default empty route definition will cause an awkward assertion to be
thrown.

* In 3.28 without any data layer, the user is prompted via assertion to
  implement a model hook.
* In 3.28 with Ember Data, an implicit fetch via Ember Data happens.
* In 4.0 without any data layer, the user would be prompted via
  assertion to implement a model hook.
* In 4.0 with Ember Data, the user would be prompted via assertion to
  either add a `find` method (old assertion) or to implement a model
  hook (new assertion via
  #19858).

It is doubtless that many users will still encounter these behaviors,
but updating the blueprints to generate a model hook by default improves
on the happy path.

In theory this could do back to 3.28, however the value there is
somewhat less since Ember Data's implicit store injection remains in
that version (and therefore the assertions/messages are less confusing).
  • Loading branch information
mixonic committed Nov 30, 2021
1 parent b88bd24 commit e253e92
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 4 deletions.
9 changes: 8 additions & 1 deletion blueprints/route/files/__root__/__path__/__name__.js
@@ -1,4 +1,11 @@
import Route from '@ember/routing/route';

export default Route.extend({
export default Route.extend({<% if (hasDynamicSegment) {%>
model(params) {
/**
* This route was generated with a dynamic segment. Implement data loading
* based on that dynamic segment here in the model hook.
*/
return params;
},<%}%>
});
2 changes: 2 additions & 0 deletions blueprints/route/index.js
Expand Up @@ -75,6 +75,7 @@ module.exports = useEditionDetector({
let moduleName = options.entity.name;
let rawRouteName = moduleName.split('/').pop();
let emberPageTitleExists = 'ember-page-title' in options.project.dependencies();
let hasDynamicSegment = options.path && options.path.includes(':');

if (options.resetNamespace) {
moduleName = rawRouteName;
Expand All @@ -84,6 +85,7 @@ module.exports = useEditionDetector({
moduleName: stringUtil.dasherize(moduleName),
routeName: stringUtil.classify(rawRouteName),
addTitle: emberPageTitleExists,
hasDynamicSegment,
};
},

Expand Down
9 changes: 8 additions & 1 deletion blueprints/route/native-files/__root__/__path__/__name__.js
@@ -1,4 +1,11 @@
import Route from '@ember/routing/route';

export default class <%= classifiedModuleName %>Route extends Route {
export default class <%= classifiedModuleName %>Route extends Route {<% if (hasDynamicSegment) {%>
model(params) {
/**
* This route was generated with a dynamic segment. Implement data loading
* based on that dynamic segment here in the model hook.
*/
return params;
}<%}%>
}
6 changes: 4 additions & 2 deletions node-tests/blueprints/route-test.js
Expand Up @@ -81,7 +81,7 @@ describe('Blueprint: route', function () {

it('route foo --path=:foo_id/show', function () {
return emberGenerateDestroy(['route', 'foo', '--path=:foo_id/show'], (_file) => {
expect(_file('app/routes/foo.js')).to.equal(fixture('route/route.js'));
expect(_file('app/routes/foo.js')).to.equal(fixture('route/route-with-dynamic-segment.js'));

expect(_file('app/templates/foo.hbs')).to.equal('{{outlet}}');

Expand Down Expand Up @@ -560,7 +560,9 @@ describe('Blueprint: route', function () {

it('route foo --path=:foo_id/show', function () {
return emberGenerateDestroy(['route', 'foo', '--path=:foo_id/show'], (_file) => {
expect(_file('app/routes/foo.js')).to.equal(fixture('route/native-route.js'));
expect(_file('app/routes/foo.js')).to.equal(
fixture('route/native-route-with-dynamic-segment.js')
);

expect(_file('app/templates/foo.hbs')).to.equal('{{outlet}}');

Expand Down
11 changes: 11 additions & 0 deletions node-tests/fixtures/route/native-route-with-dynamic-segment.js
@@ -0,0 +1,11 @@
import Route from '@ember/routing/route';

export default class FooRoute extends Route {
model(params) {
/**
* This route was generated with a dynamic segment. Implement data loading
* based on that dynamic segment here in the model hook.
*/
return params;
}
}
11 changes: 11 additions & 0 deletions node-tests/fixtures/route/route-with-dynamic-segment.js
@@ -0,0 +1,11 @@
import Route from '@ember/routing/route';

export default Route.extend({
model(params) {
/**
* This route was generated with a dynamic segment. Implement data loading
* based on that dynamic segment here in the model hook.
*/
return params;
},
});

0 comments on commit e253e92

Please sign in to comment.