From e253e921c79b7c7dd40d2ade217f581a43ee2c7a Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Tue, 30 Nov 2021 14:08:49 -0500 Subject: [PATCH] [BUGFIX release] Add model hook in route blueprint 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 https://github.com/emberjs/ember.js/pull/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). --- blueprints/route/files/__root__/__path__/__name__.js | 9 ++++++++- blueprints/route/index.js | 2 ++ .../route/native-files/__root__/__path__/__name__.js | 9 ++++++++- node-tests/blueprints/route-test.js | 6 ++++-- .../route/native-route-with-dynamic-segment.js | 11 +++++++++++ .../fixtures/route/route-with-dynamic-segment.js | 11 +++++++++++ 6 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 node-tests/fixtures/route/native-route-with-dynamic-segment.js create mode 100644 node-tests/fixtures/route/route-with-dynamic-segment.js diff --git a/blueprints/route/files/__root__/__path__/__name__.js b/blueprints/route/files/__root__/__path__/__name__.js index 6c74252aa1b..7908be4f953 100644 --- a/blueprints/route/files/__root__/__path__/__name__.js +++ b/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; + },<%}%> }); diff --git a/blueprints/route/index.js b/blueprints/route/index.js index 50b10685a4b..6c197852e74 100644 --- a/blueprints/route/index.js +++ b/blueprints/route/index.js @@ -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; @@ -84,6 +85,7 @@ module.exports = useEditionDetector({ moduleName: stringUtil.dasherize(moduleName), routeName: stringUtil.classify(rawRouteName), addTitle: emberPageTitleExists, + hasDynamicSegment, }; }, diff --git a/blueprints/route/native-files/__root__/__path__/__name__.js b/blueprints/route/native-files/__root__/__path__/__name__.js index 508c4dbe00c..2bf1de81a14 100644 --- a/blueprints/route/native-files/__root__/__path__/__name__.js +++ b/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; + }<%}%> } diff --git a/node-tests/blueprints/route-test.js b/node-tests/blueprints/route-test.js index b8b55b15979..c3251ca2d23 100644 --- a/node-tests/blueprints/route-test.js +++ b/node-tests/blueprints/route-test.js @@ -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}}'); @@ -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}}'); diff --git a/node-tests/fixtures/route/native-route-with-dynamic-segment.js b/node-tests/fixtures/route/native-route-with-dynamic-segment.js new file mode 100644 index 00000000000..7856d6b3fec --- /dev/null +++ b/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; + } +} diff --git a/node-tests/fixtures/route/route-with-dynamic-segment.js b/node-tests/fixtures/route/route-with-dynamic-segment.js new file mode 100644 index 00000000000..aa466fc9446 --- /dev/null +++ b/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; + }, +});