From 27ff11dac38fbd445edfdb3587fb1f194600fd9c Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sun, 9 Jun 2019 08:12:16 -0400 Subject: [PATCH 01/20] Begin writing RFC --- text/0000-controller-migration-path.md | 70 ++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 text/0000-controller-migration-path.md diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md new file mode 100644 index 0000000000..3b5f1ccae0 --- /dev/null +++ b/text/0000-controller-migration-path.md @@ -0,0 +1,70 @@ +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- Relevant Team(s): (fill this in with the [team(s)](README.md#relevant-teams) to which this RFC applies) +- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) +- Tracking: (leave this empty) + +# Migration away from controllers + +## Summary + + - Provide alternative APIs that encompass existing behavior currently implemented by controllers. + - Out of scope: + - Deprecating and eliminating controllers -- but the goal of this RFC is to provide a path to remove controllers from Ember altogether. + - Not Proposing + - Adding the ability to access more properties than `model` from the route template + - Adding the ability to invoke actions on the route. + +This'll explore how controllers are used and how viable alternatives can be for migrating away from controllers so that they may eventually be removed. + +## Motivation + + - Many were excited about [Routable Components](https://github.com/ef4/rfcs/blob/routeable-components/active/0000-routeable-components.md). + - Many agree that controllers are awkward, and should be considered for removal (as mentioned in many #EmberJS2019 Roadmap blogposts). + - There is, without a doubt, some confusion with respect to controllers: + - should actions live on controllers? + - how are they different from components? + - controllers may feel like a route-blessed component (hence the prior excitement over the Routable-Components RFC) + - controllers aren't created by default during `ember g route my-route`, so there is a learning curve around the fact that route templates can't access things on the route, and must create another new file, called a controller. + + +## Detailed design + +> This is the bulk of the RFC. + +> Explain the design in enough detail for somebody +familiar with the framework to understand, and for somebody familiar with the +implementation to implement. This should get into specifics and corner-cases, +and include examples of how the feature is used. Any new terminology should be +defined here. + +## How we teach this + +> What names and terminology work best for these concepts and why? How is this +idea best presented? As a continuation of existing Ember patterns, or as a +wholly new one? + +> Would the acceptance of this proposal mean the Ember guides must be +re-organized or altered? Does it change how Ember is taught to new users +at any level? + +> How should this feature be introduced and taught to existing Ember +users? + +## Drawbacks + +> Why should we *not* do this? Please consider the impact on teaching Ember, +on the integration of this feature with other existing and planned features, +on the impact of the API churn on existing apps, etc. + +> There are tradeoffs to choosing any path, please attempt to identify them here. + +## Alternatives + +> What other designs have been considered? What is the impact of not doing this? + +> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. + +## Unresolved questions + +> Optional, but suggested for first drafts. What parts of the design are still +TBD? From 77d51004364832a44ce84e8275a669fa3211addc Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sun, 9 Jun 2019 08:31:14 -0400 Subject: [PATCH 02/20] Add alternatives/prior art --- text/0000-controller-migration-path.md | 66 ++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 3b5f1ccae0..39897f42c5 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -52,19 +52,67 @@ users? ## Drawbacks -> Why should we *not* do this? Please consider the impact on teaching Ember, -on the integration of this feature with other existing and planned features, -on the impact of the API churn on existing apps, etc. - -> There are tradeoffs to choosing any path, please attempt to identify them here. +These changes would require many changes to existing apps, and there likely won't be an ability to codemod people's code to the new way of doing things. This could greatly slow down the migration, or have people opt to not migrate at all. It's very important that every use case for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. ## Alternatives -> What other designs have been considered? What is the impact of not doing this? +To date (2019-06-09 at the time of writing this), there has been one roadmap blogpost to say to get rid of both routes and controllers and have everything be components, like React. + +React projects typically use dynamic routing which is only possible to build the full route tree through static analysis of a build tool that doesn't exist (yet?). Ember's Route pattern is immensely powerful, especially for managing the minimally required data for a route. + +However, there is a pattern that can be implement using React + React Router which would be good to have an equivelant of in Ember apps. + +The Route override + ErrorBoundary combo. + +Consider the following + +```tsx +import { Route as ReactRouterRoute } from 'react-router-dom'; +import ErrorBoundary from '~/ui/components/errors/error-boundary'; + +export function Route(passedProps) { + const {...props } = passedProps; + + return ( + + + + ); +} +``` -> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. +where `ErrorBoundary` is defined as: +```ts +import React, { Component } from 'react'; +import PageError from './errors/page'; + +// https://reactjs.org/docs/error-boundaries.html +export default class ErrorBoundary extends Componen { + state = { hasError: false }; + // Update state so the next render will show the fallback UI. + static getDerivedStateFromError(error) { + return { hasError: true, error }; + } + // You can also log the error to an error reporting service + componentDidCatch(error, info) { console.error(error, info); } + + render() { + const { hasError, error } = this.state; + + if (hasError) return ; + + return this.props.children; + } +} +``` + +what this allows is our apps to on a per-route basis be able to catch _all_ errors within a sub-route and perform some behavior local to that route -- provided that the entire app uses this custom `Route` and no longer uses the default one from `react-router-dom`. This is advantageous for a couple reasons: + - whenever implementing a feature, bugfix, or whatever, anything wrong you do will be caught and displayed on the UI, rather than entirely breaking the UI or _causing infinite rendering loops_ + - Whenever there is an unexpected uncaught exception due to a yet-to-be-fixed bug, the UI for handling the error and displaying that error to the user is implemented _for free_. This helps with error reporting from users as the error will be rendered and there'd be no need to ask for a reproduction with the console open. + + Switching to totally dynamic routing would too big of a paradigm shift and it would remove one of the best features about Ember: the asyncronous state management for required data when entering a route. With dynamic routing, as in react-router, all of that responsibility would be pushed to the user -- every app would have a different way to handle loading and other intermediate state. ## Unresolved questions -> Optional, but suggested for first drafts. What parts of the design are still -TBD? + - What use cases of controllers are not covered here? + From a03a830fe491982ed421c5ab292f6513379ec46d Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sun, 9 Jun 2019 08:36:58 -0400 Subject: [PATCH 03/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 39897f42c5..75e0e4bc42 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -29,6 +29,13 @@ This'll explore how controllers are used and how viable alternatives can be for ## Detailed design +### Query Params + +In this other RFC that [proposes adding a @queryParam decorator and service to manage query params](https://github.com/emberjs/rfcs/pull/380), query-param management would be pulled out of the controller entirely. + +### Error and Loading States + + > This is the bulk of the RFC. > Explain the design in enough detail for somebody From 5ad749dd60a8c5dc797a8c10bf5c1889f56f2eb0 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sun, 9 Jun 2019 10:49:23 -0400 Subject: [PATCH 04/20] add examples for error and loading handling --- text/0000-controller-migration-path.md | 143 +++++++++++++++++++++++-- 1 file changed, 137 insertions(+), 6 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 75e0e4bc42..954404438f 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -35,14 +35,145 @@ In this other RFC that [proposes adding a @queryParam decorator and service to m ### Error and Loading States +According to the routing guides on [error and loading substates](https://guides.emberjs.com/release/routing/loading-and-error-substates/), for both errors and loading state, we need a template in various locations depending on our route structure and, for loading, which routes we think are slow enough to warrant a loading indicator (whether that be a spinner, or fake cards or other elements on the page). Additionally, there is a loading action fired per route that can optionally be customized in order to set parameters on the route's controller. -> This is the bulk of the RFC. +Maybe something we could do to make things less configuration-based and also be more explicit is to set properties on routes. + +For example, maybe we want to set the error and loading UI on a particular route: +```ts +import Route from '@ember/routing/route'; +import ErrorComponent from 'my-app/components/error-handler'; +import LoadingComponent from 'my-app/components/loading-spinner'; + +export default class MyRoute extends Route { + onError = ErrorComponent; + onLoading = LoadingComponent; + + async model() { + const myData = await this.store.findAll('slow-data'); + + return { myData }; + } +} +``` +This setting of components to use as the error and loading state render contexts allows us to more intuitively tie in to the dependency injection system as well as have very clear shared resources for this common behavior. + +The actions that are called on error and loading could still exist, as those may be useful for maybe redirecting to different places in case of a 401 Unauthorized / 403 Forbidden error + + +Another scenario - maybe someone wants to use the same error and loading +configuration for every route: + +```ts +// utils/configured-route.js +import Route from '@ember/routing/route'; +import ErrorComponent from 'my-app/components/error-handler'; +import LoadingComponent from 'my-app/components/loading-spinner'; + +export default class ConfiguredRoute extends Route { + onError = ErrorComponent; + onLoading = LoadingComponent; +} + +// routes/posts.js +import Route from 'my-app/utils/configured-route'; + +export default class PostsRoute extends Route { + async model() { + const posts = await this.store.findAll('post'); + + return { posts }; + } +} +``` + +This may not end up being a common pattern to have the same loading state of every route, but having at least the same configured Error handling per route could provide a way to display meaningful messages to end-users by default, rather than unexpected uncaught exceptions causing the UI to break unexpectedly. + +The last scenario is using a one-off configuration, which would become more ergonomic whenever apps can move away from the 'classic' project structure. + +Assuming apps will eventually get something similar to the module unification layout where one-off components can be co-located with routes: +```ts +import Route from '@ember/routing/route'; +import ErrorComponent from './-components/error-handler'; +import LoadingComponent from './-components/loading-spinner'; + +export default class MyRoute extends Route { + onError = ErrorComponent; + onLoading = LoadingComponent; + + async model() { + const myData = await this.store.findAll('slow-data'); + + return { myData }; + } +} +``` +This would be most useful for patterns that use fake UI for loading, such as what facebook and linkedin do for their feeds. + + +Ideally, the onError component should catch _any_ error that occurs within the route's subtree. This should catch network errors that occur within the beforeModel, model, or afterModel hooks, and also during rendering. With component centric development, errors can happen anywhere, and it would be fantastic to have a centralized placed to handle those errors -- though this level of error handling may be outside the scope of this RFC. + +#### Rendering the Error and Loading components + +The loading component should only be rendered if the `onLoading` property is set and the model hook has not yet resolved. It does not need to receive any arguments. + +The error component should only be rendered if the `onError` property is set and an error occurs. It should receive an argument named `error` whos signature is any of, but not limited to, any of the subtypes of [`Error`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error). It'll be up to the application developer to handle how to specifically render the error. + +Example: + +```ts +import Component from '@glimmer/component'; +import { parseError } from 'app-name/utils/errors'; + +export default class ErrorComponent extends Component { + get errorData() { + const { error } = this.args; + + return parseError(error); + } +} +``` +```hbs +
+ {{error.title}} + + {{#if error.body}} +

{{error.body}}

+ {{/if}} +
+``` + +```ts +// elsewhere, in app-name/utils/errors; +export function parseError(error: any): ParsedError { + if (error instanceof ClientError) { + const maybeJsonApiError = getFirstJSONAPIError(error); + + if (Object.keys(maybeJsonApiError).length > 0) { + return { title: maybeJsonApiError.title, body: maybeJsonApiError.detail }; + } + + return { + title: error.description, + body: error.message, + }; + } + + if (error instanceof NetworkError) { + // parse something different for network errors + } + + if (error instanceof SomeOtherError) { + // parse something different unique to SomeOtherError + } + + // All other errors + const title = error.message || error; + const body = undefined; + + return { title, body }; +``` -> Explain the design in enough detail for somebody -familiar with the framework to understand, and for somebody familiar with the -implementation to implement. This should get into specifics and corner-cases, -and include examples of how the feature is used. Any new terminology should be -defined here. ## How we teach this From fbef2763fc9a32b3757378f89db28f4f05a83dff Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sun, 9 Jun 2019 12:27:23 -0400 Subject: [PATCH 05/20] update stuff --- text/0000-controller-migration-path.md | 98 +++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 954404438f..8486adb6ac 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -25,14 +25,110 @@ This'll explore how controllers are used and how viable alternatives can be for - how are they different from components? - controllers may feel like a route-blessed component (hence the prior excitement over the Routable-Components RFC) - controllers aren't created by default during `ember g route my-route`, so there is a learning curve around the fact that route templates can't access things on the route, and must create another new file, called a controller. + - For handling actions from the UI, component-local actions and actions that live on services are a better long-term maintenance pattern. Dependency injection especially from the store and router services for fetching / updating data as well as contextually redirecting to different routes. + - State and Actions on the controller encourage prop drilling where arguments are passed through many layers of components which make maintenance more difficult. ## Detailed design +### Rendering + +The templates used for routes should remain the same. Any context that would go in controllers can be split out to components. This has the advantage of components defining the semantic intent behind what they are rendering, such as in this example application template: [routes/application/template.hbs from emberclear](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/application/template.hbs) +```hbs + + + + + + + + {{outlet}} + + + + + +``` + +This route-level template only defines the layout. Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. + +In some cases, the route template may be as small as only having some wrapping styles ([routes/contacts/template.hbs from emberclear](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/contacts/template.hbs)) +```hbs +
+
+
+ +
+ +
+
+
+``` +Here, model is the only property that the template has access to. Because this template has a more narrow and specific purpose, it should be referred to explicitly as a "route template". + ### Query Params In this other RFC that [proposes adding a @queryParam decorator and service to manage query params](https://github.com/emberjs/rfcs/pull/380), query-param management would be pulled out of the controller entirely. +Presently, query params live on controllers, which are singletons, allowing query param values to be set, and unset, when navigation to and away from a route. Query params can have the same behavior implemented on a service, which can be tested out on an addon, [ember-query-params-service](https://github.com/NullVoxPopuli/ember-query-params-service). + +Behavior like `replaceState` and `refreshModel: true`, would still live on the route as that behavior is more of a route concern than it is a concern of the value of the query params. + +- **Mapping a Query Param to state** + + Old: query params _must_ come from a controller. + ```ts + import Controller from '@ember/controller'; + + export default class SomeController extends Controller { + queryParams = ['foo']; + foo = null; + } + ``` + ```hbs + + ``` + + New: query params can be used anywhere via dependency injection. + ```ts + import Component from "@glimmer/component"; + import { queryParam } from "ember-query-params-service"; + + export default class SomeComponent extends Component { + @queryParam foo; + + addToFoo() { + this.foo = (this.foo || 0) + 1; + } + } + ``` + +- **Mapping a Query Param to a state of a different name** + + Old: + ```ts + import Controller from '@ember/controller'; + + export default class SomeController extends Controller { + queryParams = ['foo']; + foo = null; + } + ``` + + New: + ```ts + import Component from "@glimmer/component"; + import { queryParam } from "ember-query-params-service"; + + export default class SomeComponent extends Component { + @queryParam('foo') bar; + + addToFoo() { + this.bar = (this.bar || 0) + 1; + } + } + ``` + ### Error and Loading States According to the routing guides on [error and loading substates](https://guides.emberjs.com/release/routing/loading-and-error-substates/), for both errors and loading state, we need a template in various locations depending on our route structure and, for loading, which routes we think are slow enough to warrant a loading indicator (whether that be a spinner, or fake cards or other elements on the page). Additionally, there is a loading action fired per route that can optionally be customized in order to set parameters on the route's controller. @@ -198,7 +294,7 @@ To date (2019-06-09 at the time of writing this), there has been one roadmap blo React projects typically use dynamic routing which is only possible to build the full route tree through static analysis of a build tool that doesn't exist (yet?). Ember's Route pattern is immensely powerful, especially for managing the minimally required data for a route. -However, there is a pattern that can be implement using React + React Router which would be good to have an equivelant of in Ember apps. +However, there is a pattern that can be implement using React + React Router which would be good to have an equivalent of in Ember apps. The Route override + ErrorBoundary combo. From 96498ff91bb40b0f79944d47e634e1d225f3d3ff Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sun, 9 Jun 2019 12:29:32 -0400 Subject: [PATCH 06/20] remove unfinished part --- text/0000-controller-migration-path.md | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 8486adb6ac..45a90953f6 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -271,19 +271,6 @@ export function parseError(error: any): ParsedError { ``` -## How we teach this - -> What names and terminology work best for these concepts and why? How is this -idea best presented? As a continuation of existing Ember patterns, or as a -wholly new one? - -> Would the acceptance of this proposal mean the Ember guides must be -re-organized or altered? Does it change how Ember is taught to new users -at any level? - -> How should this feature be introduced and taught to existing Ember -users? - ## Drawbacks These changes would require many changes to existing apps, and there likely won't be an ability to codemod people's code to the new way of doing things. This could greatly slow down the migration, or have people opt to not migrate at all. It's very important that every use case for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. From d555db6530fc7c261fda350285b8f64c754e3a5f Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 10 Jun 2019 09:05:43 -0400 Subject: [PATCH 07/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 332 +++++++++---------------- 1 file changed, 115 insertions(+), 217 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 45a90953f6..82b85ee841 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -8,11 +8,16 @@ ## Summary - Provide alternative APIs that encompass existing behavior currently implemented by controllers. + 1. query params + 2. passing state and handling actions to/from the template + - Out of scope: - Deprecating and eliminating controllers -- but the goal of this RFC is to provide a path to remove controllers from Ember altogether. + - Not Proposing - Adding the ability to access more properties than `model` from the route template - Adding the ability to invoke actions on the route. + - Any changes to the `Route` This'll explore how controllers are used and how viable alternatives can be for migrating away from controllers so that they may eventually be removed. @@ -31,42 +36,17 @@ This'll explore how controllers are used and how viable alternatives can be for ## Detailed design -### Rendering - -The templates used for routes should remain the same. Any context that would go in controllers can be split out to components. This has the advantage of components defining the semantic intent behind what they are rendering, such as in this example application template: [routes/application/template.hbs from emberclear](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/application/template.hbs) -```hbs - - - - - - +First, what would motivate someone to use a Controller today? Borrowing from [this blog post](https://medium.com/ember-ish/what-are-controllers-used-for-in-ember-js-cba712bf3b3e) by [Jen Weber](https://twitter.com/jwwweber): - {{outlet}} +1. You need to use query parameters in the URL that the user sees (such as filters that you use for display or that you need to send with back end requests) - +2. There are actions or variables that you want to share with child components - - -``` - -This route-level template only defines the layout. Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. - -In some cases, the route template may be as small as only having some wrapping styles ([routes/contacts/template.hbs from emberclear](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/contacts/template.hbs)) -```hbs -
-
-
+3. (2a) You have a computed property that depends on the results of the model hook, such as filtering a list -
- -
-
-
-``` -Here, model is the only property that the template has access to. Because this template has a more narrow and specific purpose, it should be referred to explicitly as a "route template". +All of this behavior can be implemented elsewhere, and most of what controllers _can_ do is already more naturally covered by components. -### Query Params +### 1. Query Params In this other RFC that [proposes adding a @queryParam decorator and service to manage query params](https://github.com/emberjs/rfcs/pull/380), query-param management would be pulled out of the controller entirely. @@ -105,17 +85,17 @@ Behavior like `replaceState` and `refreshModel: true`, would still live on the r - **Mapping a Query Param to a state of a different name** - Old: + Old: controller must exist to define query params ```ts import Controller from '@ember/controller'; export default class SomeController extends Controller { - queryParams = ['foo']; - foo = null; + queryParams = {foo: 'bar' }; + bar = null; } ``` - New: + New: No controller exists ```ts import Component from "@glimmer/component"; import { queryParam } from "ember-query-params-service"; @@ -129,209 +109,127 @@ Behavior like `replaceState` and `refreshModel: true`, would still live on the r } ``` -### Error and Loading States - -According to the routing guides on [error and loading substates](https://guides.emberjs.com/release/routing/loading-and-error-substates/), for both errors and loading state, we need a template in various locations depending on our route structure and, for loading, which routes we think are slow enough to warrant a loading indicator (whether that be a spinner, or fake cards or other elements on the page). Additionally, there is a loading action fired per route that can optionally be customized in order to set parameters on the route's controller. +### 2. Passing State and Handling Actions to/from the Template -Maybe something we could do to make things less configuration-based and also be more explicit is to set properties on routes. +In order to fully eliminate the controller, there _must_ be a way to handle computed properties or state along with responding to actions from the template. Components can cover this already, and would be an intuitive way to interact with the view and behavioral layer. -For example, maybe we want to set the error and loading UI on a particular route: -```ts -import Route from '@ember/routing/route'; -import ErrorComponent from 'my-app/components/error-handler'; -import LoadingComponent from 'my-app/components/loading-spinner'; +Currently, controllers + templates must be used in the app by having: + - for classic apps: + - `app/controllers/{my-route-name/or-path}.js` + - `app/templates/{my-route-name/or-path}.hbs` + - for pods apps: + - `{pod-namespace}/{my-route-name/or-path}/controller.js` + - `{pod-namespace}/{my-route-name/or-path}/template.hbs` + - for MU or (future) MU-inspired layouts: + - `src/ui/routes/{my-route-name/or-path}/controller.js` + - `src/ui/routes/{my-route-name/or-path}/template.hbs` -export default class MyRoute extends Route { - onError = ErrorComponent; - onLoading = LoadingComponent; - - async model() { - const myData = await this.store.findAll('slow-data'); - - return { myData }; - } -} -``` -This setting of components to use as the error and loading state render contexts allows us to more intuitively tie in to the dependency injection system as well as have very clear shared resources for this common behavior. - -The actions that are called on error and loading could still exist, as those may be useful for maybe redirecting to different places in case of a 401 Unauthorized / 403 Forbidden error - - -Another scenario - maybe someone wants to use the same error and loading -configuration for every route: - -```ts -// utils/configured-route.js -import Route from '@ember/routing/route'; -import ErrorComponent from 'my-app/components/error-handler'; -import LoadingComponent from 'my-app/components/loading-spinner'; - -export default class ConfiguredRoute extends Route { - onError = ErrorComponent; - onLoading = LoadingComponent; -} - -// routes/posts.js -import Route from 'my-app/utils/configured-route'; - -export default class PostsRoute extends Route { - async model() { - const posts = await this.store.findAll('post'); - - return { posts }; - } -} -``` - -This may not end up being a common pattern to have the same loading state of every route, but having at least the same configured Error handling per route could provide a way to display meaningful messages to end-users by default, rather than unexpected uncaught exceptions causing the UI to break unexpectedly. - -The last scenario is using a one-off configuration, which would become more ergonomic whenever apps can move away from the 'classic' project structure. - -Assuming apps will eventually get something similar to the module unification layout where one-off components can be co-located with routes: -```ts -import Route from '@ember/routing/route'; -import ErrorComponent from './-components/error-handler'; -import LoadingComponent from './-components/loading-spinner'; - -export default class MyRoute extends Route { - onError = ErrorComponent; - onLoading = LoadingComponent; - - async model() { - const myData = await this.store.findAll('slow-data'); - - return { myData }; - } -} -``` -This would be most useful for patterns that use fake UI for loading, such as what facebook and linkedin do for their feeds. +The proposed new behavior would be that when a controller is not present, and there is only a template, that template becomes a template-only/stateless component. The template-only/stateless component has a number of advantages as it forces the developer to separate UI layout and to think about the semantic breakdown of the template. Here are some existing examples of template-only/stateless route templates in an app made today: +from emberclear.io + - [routes/application/template.hbs](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/application/template.hbs) + - [routes/contacts/template.hbs](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/contacts/template.hbs) -Ideally, the onError component should catch _any_ error that occurs within the route's subtree. This should catch network errors that occur within the beforeModel, model, or afterModel hooks, and also during rendering. With component centric development, errors can happen anywhere, and it would be fantastic to have a centralized placed to handle those errors -- though this level of error handling may be outside the scope of this RFC. +Sample: -#### Rendering the Error and Loading components - -The loading component should only be rendered if the `onLoading` property is set and the model hook has not yet resolved. It does not need to receive any arguments. - -The error component should only be rendered if the `onError` property is set and an error occurs. It should receive an argument named `error` whos signature is any of, but not limited to, any of the subtypes of [`Error`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error). It'll be up to the application developer to handle how to specifically render the error. - -Example: - -```ts -import Component from '@glimmer/component'; -import { parseError } from 'app-name/utils/errors'; - -export default class ErrorComponent extends Component { - get errorData() { - const { error } = this.args; - - return parseError(error); - } -} -``` ```hbs -
- {{error.title}} +
+
+
- {{#if error.body}} -

{{error.body}}

- {{/if}} -
+
+ +
+
+ ``` -```ts -// elsewhere, in app-name/utils/errors; -export function parseError(error: any): ParsedError { - if (error instanceof ClientError) { - const maybeJsonApiError = getFirstJSONAPIError(error); +Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. However, this is only the simplest use case, where all derived state is handled by the components rendered by the existing template. - if (Object.keys(maybeJsonApiError).length > 0) { - return { title: maybeJsonApiError.title, body: maybeJsonApiError.detail }; - } +For the case where we _need_ derived state at the root of our route's tree. + - for classic apps: + - `app/components/{my-route-name/or-path}.js` + - `app/templates/{my-route-name/or-path}.hbs` + - for pods apps: + - `{pod-namespace}/{my-route-name/or-path}/component.js` + - `{pod-namespace}/{my-route-name/or-path}/template.hbs` + - for classic apps after [RFC #481](https://github.com/emberjs/rfcs/pull/481) (Colocated Component and Template files) + - `app/components/{my-route-name/or-path}.js` + - `app/components/{my-route-name/or-path}.hbs` + - for MU or (future) MU-inspired layouts: + - `src/ui/routes/{my-route-name/or-path}/component.js` + - `src/ui/routes/{my-route-name/or-path}/template.hbs` - return { - title: error.description, - body: error.message, - }; - } +For current classic apps, this may feel a bit like resolution magic, but it's just the same lookup rule as for controllers, just in a different folder. This changes makes more contextual sense with pods and module unification (or some future derivitive of module unification) where the backing context to the template is co-located alongside the template. - if (error instanceof NetworkError) { - // parse something different for network errors - } +These are not "routable components", but components that are invoked after the `model` hook resolves in the route, recieving only a single property `@model`. - if (error instanceof SomeOtherError) { - // parse something different unique to SomeOtherError - } + - Before: with controllers + ```ts + import Route from '@ember/routing/route'; - // All other errors - const title = error.message || error; - const body = undefined; - - return { title, body }; -``` + export default class SomeRoute extends Route { + async model() { + const contacts = await this.store.findAll('contact'); + return { contacts }; + } + } + ``` + ```ts + import Controller from '@ember/controller'; + + export default class SomeController extends Controller { + get onlineContacts() { + return this.model.contacts.filter(contact => { + return contact.onlineStatus === 'online'; + }); + } + } + ``` + ```hbs + {{#each this.onlineContacts as |contact|}} + {{contact.name}} + {{/each}} + ``` + + - After: the controller is swapped for a component + ```ts + import Route from '@ember/routing/route'; + + export default class SomeRoute extends Route { + async model() { + const contacts = await this.store.findAll('contact'); + + return { contacts }; + } + } + ``` + ```ts + import Component from "@glimmer/component"; + + export default class SomeComponent extends Component { + get onlineContacts() { + const { model } = this.args; + + return model.contacts.filter(contact => { + return contact.onlineStatus === 'online'; + }); + } + } + ``` + ```hbs + {{#each this.onlineContacts as |contact|}} + {{contact.name}} + {{/each}} + ``` ## Drawbacks -These changes would require many changes to existing apps, and there likely won't be an ability to codemod people's code to the new way of doing things. This could greatly slow down the migration, or have people opt to not migrate at all. It's very important that every use case for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. - -## Alternatives - -To date (2019-06-09 at the time of writing this), there has been one roadmap blogpost to say to get rid of both routes and controllers and have everything be components, like React. - -React projects typically use dynamic routing which is only possible to build the full route tree through static analysis of a build tool that doesn't exist (yet?). Ember's Route pattern is immensely powerful, especially for managing the minimally required data for a route. - -However, there is a pattern that can be implement using React + React Router which would be good to have an equivalent of in Ember apps. - -The Route override + ErrorBoundary combo. - -Consider the following - -```tsx -import { Route as ReactRouterRoute } from 'react-router-dom'; -import ErrorBoundary from '~/ui/components/errors/error-boundary'; - -export function Route(passedProps) { - const {...props } = passedProps; - - return ( - - - - ); -} -``` - -where `ErrorBoundary` is defined as: -```ts -import React, { Component } from 'react'; -import PageError from './errors/page'; - -// https://reactjs.org/docs/error-boundaries.html -export default class ErrorBoundary extends Componen { - state = { hasError: false }; - // Update state so the next render will show the fallback UI. - static getDerivedStateFromError(error) { - return { hasError: true, error }; - } - // You can also log the error to an error reporting service - componentDidCatch(error, info) { console.error(error, info); } - - render() { - const { hasError, error } = this.state; - - if (hasError) return ; - - return this.props.children; - } -} -``` +It's very important that every use case for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. -what this allows is our apps to on a per-route basis be able to catch _all_ errors within a sub-route and perform some behavior local to that route -- provided that the entire app uses this custom `Route` and no longer uses the default one from `react-router-dom`. This is advantageous for a couple reasons: - - whenever implementing a feature, bugfix, or whatever, anything wrong you do will be caught and displayed on the UI, rather than entirely breaking the UI or _causing infinite rendering loops_ - - Whenever there is an unexpected uncaught exception due to a yet-to-be-fixed bug, the UI for handling the error and displaying that error to the user is implemented _for free_. This helps with error reporting from users as the error will be rendered and there'd be no need to ask for a reproduction with the console open. +It may be possible to write a codemod to help with the migration as the biggest difference, at leaste in this RFC's initial samples is that `model` is accessed on `this.args` instead of just `this`. The tool and maybe even a linter rule could look for pre-existing components that have any of the same names as routes, and assert that they handle the `model` argument. - Switching to totally dynamic routing would too big of a paradigm shift and it would remove one of the best features about Ember: the asyncronous state management for required data when entering a route. With dynamic routing, as in react-router, all of that responsibility would be pushed to the user -- every app would have a different way to handle loading and other intermediate state. ## Unresolved questions From 0243ab6592efa4527e85c5c2b65adb1811429ec6 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 10 Jun 2019 16:29:53 -0400 Subject: [PATCH 08/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 82b85ee841..1fde8b4a3b 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -224,6 +224,19 @@ These are not "routable components", but components that are invoked after the ` {{/each}} ``` +### Lookup Priority Rules + +For each of these, assume that all parent routes/templates/etc have resolved. +Because there are 4 or 5 project structure layouts in flux, the notation here is what is used inside the resolver. + +Given a visit to `/posts` +1. lookup `template:posts` + a. if a controller exists, the template will be rendered as it is today. + b. if a controller does not exist, a template-only component will be assumed, passing the `@model` argument. This will make interop between controllers' templates and template-only component / used-for-route template components inconsistent, so controllers' templates should also get a `@model` argument. +2. lookup a component named "posts", `component:posts` and `template:components/posts`. + NOTE: if the `template:posts` template was found, this step does not happen. + a. this component is invoked _as if_ `template:posts` was defined as `` + ## Drawbacks It's very important that every use case for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. From bc0595a70f63234832f39706bb17d827ca243ba2 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 10 Jun 2019 16:32:09 -0400 Subject: [PATCH 09/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 1 + 1 file changed, 1 insertion(+) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 1fde8b4a3b..aad0686481 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -236,6 +236,7 @@ Given a visit to `/posts` 2. lookup a component named "posts", `component:posts` and `template:components/posts`. NOTE: if the `template:posts` template was found, this step does not happen. a. this component is invoked _as if_ `template:posts` was defined as `` +3. default to `{{outlet}}` for sub routes ## Drawbacks From 692accc65ff509e5f6529fb69f31d09fad25deca Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 10 Jun 2019 16:32:32 -0400 Subject: [PATCH 10/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index aad0686481..4dbadcf042 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -236,7 +236,7 @@ Given a visit to `/posts` 2. lookup a component named "posts", `component:posts` and `template:components/posts`. NOTE: if the `template:posts` template was found, this step does not happen. a. this component is invoked _as if_ `template:posts` was defined as `` -3. default to `{{outlet}}` for sub routes +3. default to `{{outlet}}` or `{{yield}}` for sub routes ## Drawbacks From b6c36dca3e6b051398d994838ecf9e24cbb80662 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 10 Jun 2019 16:33:46 -0400 Subject: [PATCH 11/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 4dbadcf042..71d2ce8d6e 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -230,13 +230,13 @@ For each of these, assume that all parent routes/templates/etc have resolved. Because there are 4 or 5 project structure layouts in flux, the notation here is what is used inside the resolver. Given a visit to `/posts` -1. lookup `template:posts` - a. if a controller exists, the template will be rendered as it is today. - b. if a controller does not exist, a template-only component will be assumed, passing the `@model` argument. This will make interop between controllers' templates and template-only component / used-for-route template components inconsistent, so controllers' templates should also get a `@model` argument. -2. lookup a component named "posts", `component:posts` and `template:components/posts`. +- lookup `template:posts` + - if a controller exists, the template will be rendered as it is today. + - if a controller does not exist, a template-only component will be assumed, passing the `@model` argument. This will make interop between controllers' templates and template-only component / used-for-route template components inconsistent, so controllers' templates should also get a `@model` argument. +- lookup a component named "posts", `component:posts` and `template:components/posts`. NOTE: if the `template:posts` template was found, this step does not happen. - a. this component is invoked _as if_ `template:posts` was defined as `` -3. default to `{{outlet}}` or `{{yield}}` for sub routes + - this component is invoked _as if_ `template:posts` was defined as `` +- default to `{{outlet}}` or `{{yield}}` for sub routes ## Drawbacks From 736374505ec74ab09f004f263c0166d5bace3300 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 10 Jun 2019 16:45:21 -0400 Subject: [PATCH 12/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 1 + 1 file changed, 1 insertion(+) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 71d2ce8d6e..0cecce6e52 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -233,6 +233,7 @@ Given a visit to `/posts` - lookup `template:posts` - if a controller exists, the template will be rendered as it is today. - if a controller does not exist, a template-only component will be assumed, passing the `@model` argument. This will make interop between controllers' templates and template-only component / used-for-route template components inconsistent, so controllers' templates should also get a `@model` argument. + - if a component by the same name exists, it is ignored and must be invoked manually. - lookup a component named "posts", `component:posts` and `template:components/posts`. NOTE: if the `template:posts` template was found, this step does not happen. - this component is invoked _as if_ `template:posts` was defined as `` From fe652034acfe89895862a8ef47f97dfa58183e67 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 10 Jun 2019 20:15:54 -0400 Subject: [PATCH 13/20] add how we teach this --- text/0000-controller-migration-path.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 0cecce6e52..6cc7545847 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -239,6 +239,17 @@ Given a visit to `/posts` - this component is invoked _as if_ `template:posts` was defined as `` - default to `{{outlet}}` or `{{yield}}` for sub routes +## How we teach this + +Since this is a new programming paradigm, we'll want to update the guides and tutorials to reflect a new "happy path". + - Remove most mentions of controllers in the guides / tutorials. maybe just leave the bare informational "what controllers are", at least until a deprecation can be figured out. + - Swap out the controllers page in the guides for a page that shows examples of how a component is chosen to be the entry point / outlet of the route. Additionally on this page, it should mention that there can be an optional template (like we have today) that receives the `@model` argument that takes priority over any component with the same name as the route and without any template at all, it's just an `{{outlet}}` for subroutes. + + The controllers page for octane has already moved under routing, so talking about just rendering behavior should make things feel simpler. + + - There is already a separate guides page for query params. Query params should remain on their own page, but just be updated to use the `@queryParam` decorator, as described in [RFC 380](https://github.com/emberjs/rfcs/pull/380) + + ## Drawbacks It's very important that every use case for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. From 0d25bc268e895546f8505ca6a4784c60a2aa9918 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 13 Jun 2019 13:59:50 -0400 Subject: [PATCH 14/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 6cc7545847..5012f1aadd 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -239,6 +239,8 @@ Given a visit to `/posts` - this component is invoked _as if_ `template:posts` was defined as `` - default to `{{outlet}}` or `{{yield}}` for sub routes +**NOTE** as a side effect of treating the default template in this manner, it would become a requirement, in order to maintain cohesion, that we treat loading and error templates similar to the above lookup rules. + ## How we teach this Since this is a new programming paradigm, we'll want to update the guides and tutorials to reflect a new "happy path". From 60bf33254e662ec4f171d1db71919b27ce2591d1 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 14 Jun 2019 10:18:23 -0400 Subject: [PATCH 15/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 128 ++++++------------------- 1 file changed, 31 insertions(+), 97 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 5012f1aadd..451fc2886a 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -7,23 +7,23 @@ ## Summary + - Goal: For components to eventually make up the entirety of the view layer - Provide alternative APIs that encompass existing behavior currently implemented by controllers. 1. query params 2. passing state and handling actions to/from the template - Out of scope: - - Deprecating and eliminating controllers -- but the goal of this RFC is to provide a path to remove controllers from Ember altogether. + - Deprecating and eliminating controllers -- but the goal of this RFC is to provide a _path_ to remove controllers from Ember altogether. - Not Proposing - - Adding the ability to access more properties than `model` from the route template - Adding the ability to invoke actions on the route. - - Any changes to the `Route` + - Any changes to the `Route` or `Controller` as they exist today This'll explore how controllers are used and how viable alternatives can be for migrating away from controllers so that they may eventually be removed. ## Motivation - - Many were excited about [Routable Components](https://github.com/ef4/rfcs/blob/routeable-components/active/0000-routeable-components.md). + - Many were excited about [Routable Components](https://github.com/ef4/rfcs/blob/routeable-components/active/0000-routeable-components.md) (This RFC is _not_ "Routable Components"). - Many agree that controllers are awkward, and should be considered for removal (as mentioned in many #EmberJS2019 Roadmap blogposts). - There is, without a doubt, some confusion with respect to controllers: - should actions live on controllers? @@ -48,85 +48,19 @@ All of this behavior can be implemented elsewhere, and most of what controllers ### 1. Query Params -In this other RFC that [proposes adding a @queryParam decorator and service to manage query params](https://github.com/emberjs/rfcs/pull/380), query-param management would be pulled out of the controller entirely. - -Presently, query params live on controllers, which are singletons, allowing query param values to be set, and unset, when navigation to and away from a route. Query params can have the same behavior implemented on a service, which can be tested out on an addon, [ember-query-params-service](https://github.com/NullVoxPopuli/ember-query-params-service). - -Behavior like `replaceState` and `refreshModel: true`, would still live on the route as that behavior is more of a route concern than it is a concern of the value of the query params. - -- **Mapping a Query Param to state** - - Old: query params _must_ come from a controller. - ```ts - import Controller from '@ember/controller'; - - export default class SomeController extends Controller { - queryParams = ['foo']; - foo = null; - } - ``` - ```hbs - - ``` - - New: query params can be used anywhere via dependency injection. - ```ts - import Component from "@glimmer/component"; - import { queryParam } from "ember-query-params-service"; - - export default class SomeComponent extends Component { - @queryParam foo; - - addToFoo() { - this.foo = (this.foo || 0) + 1; - } - } - ``` - -- **Mapping a Query Param to a state of a different name** - - Old: controller must exist to define query params - ```ts - import Controller from '@ember/controller'; - - export default class SomeController extends Controller { - queryParams = {foo: 'bar' }; - bar = null; - } - ``` - - New: No controller exists - ```ts - import Component from "@glimmer/component"; - import { queryParam } from "ember-query-params-service"; - - export default class SomeComponent extends Component { - @queryParam('foo') bar; - - addToFoo() { - this.bar = (this.bar || 0) + 1; - } - } - ``` +See this RFC for [adding query params to the router service](https://github.com/emberjs/rfcs/pull/380), query-param management would be pulled out of the controller entirely. ### 2. Passing State and Handling Actions to/from the Template In order to fully eliminate the controller, there _must_ be a way to handle computed properties or state along with responding to actions from the template. Components can cover this already, and would be an intuitive way to interact with the view and behavioral layer. Currently, controllers + templates must be used in the app by having: - - for classic apps: - - `app/controllers/{my-route-name/or-path}.js` - - `app/templates/{my-route-name/or-path}.hbs` - - for pods apps: - - `{pod-namespace}/{my-route-name/or-path}/controller.js` - - `{pod-namespace}/{my-route-name/or-path}/template.hbs` - - for MU or (future) MU-inspired layouts: - - `src/ui/routes/{my-route-name/or-path}/controller.js` - - `src/ui/routes/{my-route-name/or-path}/template.hbs` - -The proposed new behavior would be that when a controller is not present, and there is only a template, that template becomes a template-only/stateless component. The template-only/stateless component has a number of advantages as it forces the developer to separate UI layout and to think about the semantic breakdown of the template. Here are some existing examples of template-only/stateless route templates in an app made today: - -from emberclear.io + - `app/controllers/{my-route-name/or-path}.js` + - `app/templates/{my-route-name/or-path}.hbs` + +The proposed new behavior would be that when a controller is not present, and there is only a route template, that template becomes a template-only/stateless component (no backing js file). The template-only/stateless component has a number of advantages as it forces the developer to separate UI layout and to think about the semantic breakdown of the template. Here are some existing examples of template-only/stateless route templates in an app made today: + +from emberclear.io (using module unification) - [routes/application/template.hbs](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/application/template.hbs) - [routes/contacts/template.hbs](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/contacts/template.hbs) @@ -144,28 +78,23 @@ Sample: ``` -Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. However, this is only the simplest use case, where all derived state is handled by the components rendered by the existing template. +Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. + +The reason for explicitly calling out this behavior of template-only components at the route-level is because the template already exists there today, and if we make it a component, that makes future transitions easier. Making the route template a component _only_ means that instead of only existing a `this.model`, there would be a `@model` argument. -For the case where we _need_ derived state at the root of our route's tree. - - for classic apps: - - `app/components/{my-route-name/or-path}.js` - - `app/templates/{my-route-name/or-path}.hbs` - - for pods apps: - - `{pod-namespace}/{my-route-name/or-path}/component.js` - - `{pod-namespace}/{my-route-name/or-path}/template.hbs` - - for classic apps after [RFC #481](https://github.com/emberjs/rfcs/pull/481) (Colocated Component and Template files) - - `app/components/{my-route-name/or-path}.js` - - `app/components/{my-route-name/or-path}.hbs` - - for MU or (future) MU-inspired layouts: - - `src/ui/routes/{my-route-name/or-path}/component.js` - - `src/ui/routes/{my-route-name/or-path}/template.hbs` +However, this is only the simplest use case, where all derived state is handled by the components rendered by the existing route template. -For current classic apps, this may feel a bit like resolution magic, but it's just the same lookup rule as for controllers, just in a different folder. This changes makes more contextual sense with pods and module unification (or some future derivitive of module unification) where the backing context to the template is co-located alongside the template. +For the case where we _need_ derived state at the root of our route's tree, we _only_ add a resolution to a component of the same name.. + - `app/components/{my-route-name/or-path}.js` + - `app/templates/{my-route-name/or-path}.hbs` + +It's just the same lookup rule as for controllers, just in a different folder. If people end up having a ton of top-level state-based components per route, their component sturcture will end up matching their routing structure. This is fine, as it means that certain components are _only_ used for particular routes, which may make the unification of to a new layout more straight forward. These are not "routable components", but components that are invoked after the `model` hook resolves in the route, recieving only a single property `@model`. - Before: with controllers ```ts + // app/routes/some.js import Route from '@ember/routing/route'; export default class SomeRoute extends Route { @@ -177,6 +106,7 @@ These are not "routable components", but components that are invoked after the ` } ``` ```ts + // app/controllers/some.js import Controller from '@ember/controller'; export default class SomeController extends Controller { @@ -188,6 +118,7 @@ These are not "routable components", but components that are invoked after the ` } ``` ```hbs + {{!-- app/templates/some.hbs --}} {{#each this.onlineContacts as |contact|}} {{contact.name}} {{/each}} @@ -195,6 +126,7 @@ These are not "routable components", but components that are invoked after the ` - After: the controller is swapped for a component ```ts + // app/routes/some.js import Route from '@ember/routing/route'; export default class SomeRoute extends Route { @@ -206,6 +138,7 @@ These are not "routable components", but components that are invoked after the ` } ``` ```ts + // app/components/some.js import Component from "@glimmer/component"; export default class SomeComponent extends Component { @@ -219,6 +152,7 @@ These are not "routable components", but components that are invoked after the ` } ``` ```hbs + // app/templates/components/some.js {{#each this.onlineContacts as |contact|}} {{contact.name}} {{/each}} @@ -239,24 +173,24 @@ Given a visit to `/posts` - this component is invoked _as if_ `template:posts` was defined as `` - default to `{{outlet}}` or `{{yield}}` for sub routes -**NOTE** as a side effect of treating the default template in this manner, it would become a requirement, in order to maintain cohesion, that we treat loading and error templates similar to the above lookup rules. +**NOTE** as a side effect of treating the default template in this manner, it would become a requirement, in order to maintain cohesion, that we treat loading and error templates similar to the above lookup rules. This means that loading and error components can be used with service injections, and other state. ## How we teach this Since this is a new programming paradigm, we'll want to update the guides and tutorials to reflect a new "happy path". - Remove most mentions of controllers in the guides / tutorials. maybe just leave the bare informational "what controllers are", at least until a deprecation can be figured out. - - Swap out the controllers page in the guides for a page that shows examples of how a component is chosen to be the entry point / outlet of the route. Additionally on this page, it should mention that there can be an optional template (like we have today) that receives the `@model` argument that takes priority over any component with the same name as the route and without any template at all, it's just an `{{outlet}}` for subroutes. + + - Swap out the controllers page in the guides for a page that shows examples of how a component is chosen to be the entry point / outlet of the route. Additionally on this page, it should mention that there is a default template (like we have today) that receives the `@model` argument that takes priority over any component with the same name as the route and without any template at all, it's just an `{{outlet}}` for subroutes. The controllers page for octane has already moved under routing, so talking about just rendering behavior should make things feel simpler. - There is already a separate guides page for query params. Query params should remain on their own page, but just be updated to use the `@queryParam` decorator, as described in [RFC 380](https://github.com/emberjs/rfcs/pull/380) - ## Drawbacks -It's very important that every use case for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. +It's important that common use cases for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. -It may be possible to write a codemod to help with the migration as the biggest difference, at leaste in this RFC's initial samples is that `model` is accessed on `this.args` instead of just `this`. The tool and maybe even a linter rule could look for pre-existing components that have any of the same names as routes, and assert that they handle the `model` argument. +It may be possible to write a codemod to help with the migration as the biggest difference, at least in this RFC's initial samples is that `model` is accessed on `this.args` instead of just `this`. The tool and maybe even a linter rule could look for pre-existing components that have any of the same names as routes, and assert that they handle the `model` argument. ## Unresolved questions From 5462d80abf516f507849cad83432aff64abc0b9e Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 14 Jun 2019 10:23:27 -0400 Subject: [PATCH 16/20] Update 0000-controller-migration-path.md --- text/0000-controller-migration-path.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 451fc2886a..a3686c76cd 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -173,7 +173,7 @@ Given a visit to `/posts` - this component is invoked _as if_ `template:posts` was defined as `` - default to `{{outlet}}` or `{{yield}}` for sub routes -**NOTE** as a side effect of treating the default template in this manner, it would become a requirement, in order to maintain cohesion, that we treat loading and error templates similar to the above lookup rules. This means that loading and error components can be used with service injections, and other state. +**NOTE** as a side effect of treating the default template in this manner, it would become a requirement, in order to maintain cohesion, that we treat loading and error templates similar to the above lookup rules. This means that loading and error components can be used with service injections, and other state. The error template currently receive a `this.model`, but that will be changed to `@error` for components and be the error that caused the error template/component to render (existing behavior will not be changed)` ## How we teach this From 5ee3eca3114fae2b2628a332dc9558d8b5d2fba0 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 17 Jun 2019 21:51:58 -0400 Subject: [PATCH 17/20] Re-adopt the idea of a configuration-based Route --- text/0000-controller-migration-path.md | 176 ++++++++----------------- 1 file changed, 52 insertions(+), 124 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index a3686c76cd..89edc18425 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -7,10 +7,16 @@ ## Summary - - Goal: For components to eventually make up the entirety of the view layer +_The goal is for components to make up the entirety of the view layer_. + + - Requires: + - [RFC 496: Handlebars Strict Mode](https://github.com/emberjs/rfcs/pull/496) + - [RFC 454: SFC & Template Import Primitives](https://github.com/emberjs/rfcs/pull/454) + - [RFC 380: Add queryParams to the router service](https://github.com/emberjs/rfcs/pull/380) + - Provide alternative APIs that encompass existing behavior currently implemented by controllers. 1. query params - 2. passing state and handling actions to/from the template + 2. passing state and handling actions to/from the route, loading, and error templates - Out of scope: - Deprecating and eliminating controllers -- but the goal of this RFC is to provide a _path_ to remove controllers from Ember altogether. @@ -58,142 +64,64 @@ Currently, controllers + templates must be used in the app by having: - `app/controllers/{my-route-name/or-path}.js` - `app/templates/{my-route-name/or-path}.hbs` -The proposed new behavior would be that when a controller is not present, and there is only a route template, that template becomes a template-only/stateless component (no backing js file). The template-only/stateless component has a number of advantages as it forces the developer to separate UI layout and to think about the semantic breakdown of the template. Here are some existing examples of template-only/stateless route templates in an app made today: - -from emberclear.io (using module unification) - - [routes/application/template.hbs](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/application/template.hbs) - - [routes/contacts/template.hbs](https://github.com/NullVoxPopuli/emberclear/blob/master/packages/frontend/src/ui/routes/contacts/template.hbs) +The new behavior would allow components to be imported from anywhere in the app, and assigned to static properties on the Route. -Sample: +```ts +import FooComponent, { LoadingComponent, ErrorComponent } from './components/Foo'; +import Route from '@ember/routing/route'; -```hbs -
-
-
+export default class extends Route { + static render = FooComponent; + static loading = LoadingComponent; + static error = ErrorComponent; -
- -
-
-
+ async model() { + const contacts = await this.store.findAll('contact'); + + return { contacts }; + } +} ``` -Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. +Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. +Additionally, all three properties may be set to the same component if it was desired to manage loading, error, and success states within components. -The reason for explicitly calling out this behavior of template-only components at the route-level is because the template already exists there today, and if we make it a component, that makes future transitions easier. Making the route template a component _only_ means that instead of only existing a `this.model`, there would be a `@model` argument. +This RFC proposes _three_ new static properties on the Route class. -However, this is only the simplest use case, where all derived state is handled by the components rendered by the existing route template. +#### **render** -For the case where we _need_ derived state at the root of our route's tree, we _only_ add a resolution to a component of the same name.. - - `app/components/{my-route-name/or-path}.js` - - `app/templates/{my-route-name/or-path}.hbs` - -It's just the same lookup rule as for controllers, just in a different folder. If people end up having a ton of top-level state-based components per route, their component sturcture will end up matching their routing structure. This is fine, as it means that certain components are _only_ used for particular routes, which may make the unification of to a new layout more straight forward. - -These are not "routable components", but components that are invoked after the `model` hook resolves in the route, recieving only a single property `@model`. - - - Before: with controllers - ```ts - // app/routes/some.js - import Route from '@ember/routing/route'; - - export default class SomeRoute extends Route { - async model() { - const contacts = await this.store.findAll('contact'); - - return { contacts }; - } - } - ``` - ```ts - // app/controllers/some.js - import Controller from '@ember/controller'; - - export default class SomeController extends Controller { - get onlineContacts() { - return this.model.contacts.filter(contact => { - return contact.onlineStatus === 'online'; - }); - } - } - ``` - ```hbs - {{!-- app/templates/some.hbs --}} - {{#each this.onlineContacts as |contact|}} - {{contact.name}} - {{/each}} - ``` - - - After: the controller is swapped for a component - ```ts - // app/routes/some.js - import Route from '@ember/routing/route'; - - export default class SomeRoute extends Route { - async model() { - const contacts = await this.store.findAll('contact'); - - return { contacts }; - } - } - ``` - ```ts - // app/components/some.js - import Component from "@glimmer/component"; - - export default class SomeComponent extends Component { - get onlineContacts() { - const { model } = this.args; - - return model.contacts.filter(contact => { - return contact.onlineStatus === 'online'; - }); - } - } - ``` - ```hbs - // app/templates/components/some.js - {{#each this.onlineContacts as |contact|}} - {{contact.name}} - {{/each}} - ``` - -### Lookup Priority Rules - -For each of these, assume that all parent routes/templates/etc have resolved. -Because there are 4 or 5 project structure layouts in flux, the notation here is what is used inside the resolver. - -Given a visit to `/posts` -- lookup `template:posts` - - if a controller exists, the template will be rendered as it is today. - - if a controller does not exist, a template-only component will be assumed, passing the `@model` argument. This will make interop between controllers' templates and template-only component / used-for-route template components inconsistent, so controllers' templates should also get a `@model` argument. - - if a component by the same name exists, it is ignored and must be invoked manually. -- lookup a component named "posts", `component:posts` and `template:components/posts`. - NOTE: if the `template:posts` template was found, this step does not happen. - - this component is invoked _as if_ `template:posts` was defined as `` -- default to `{{outlet}}` or `{{yield}}` for sub routes - -**NOTE** as a side effect of treating the default template in this manner, it would become a requirement, in order to maintain cohesion, that we treat loading and error templates similar to the above lookup rules. This means that loading and error components can be used with service injections, and other state. The error template currently receive a `this.model`, but that will be changed to `@error` for components and be the error that caused the error template/component to render (existing behavior will not be changed)` +This is the component that will be rendered after the model hook resolves. +Today, there is a default template generated per-route that only contains `{{outlet}}`. In order to not break that behavior, the `render` property must not be required until both controllers and the route template are removed from the framework, after a deprecation RFC and the deprecation process). -## How we teach this +If set, and there exists a classic route template, an error will be thrown saying that there is a conflict. -Since this is a new programming paradigm, we'll want to update the guides and tutorials to reflect a new "happy path". - - Remove most mentions of controllers in the guides / tutorials. maybe just leave the bare informational "what controllers are", at least until a deprecation can be figured out. - - - Swap out the controllers page in the guides for a page that shows examples of how a component is chosen to be the entry point / outlet of the route. Additionally on this page, it should mention that there is a default template (like we have today) that receives the `@model` argument that takes priority over any component with the same name as the route and without any template at all, it's just an `{{outlet}}` for subroutes. - - The controllers page for octane has already moved under routing, so talking about just rendering behavior should make things feel simpler. - - - There is already a separate guides page for query params. Query params should remain on their own page, but just be updated to use the `@queryParam` decorator, as described in [RFC 380](https://github.com/emberjs/rfcs/pull/380) +The render component will receive the `@model` argument, which represents the value of a resolved `model` hook. -## Drawbacks +#### **loading** -It's important that common use cases for controllers today _can_ be implemented using the aforementioned techniques. If people are willing to share their controller scenarios, we can provide a library of examples of translation so that others may migrate more quickly. +This is the component that will be rendered before the model hook has resolved, if present. -It may be possible to write a codemod to help with the migration as the biggest difference, at least in this RFC's initial samples is that `model` is accessed on `this.args` instead of just `this`. The tool and maybe even a linter rule could look for pre-existing components that have any of the same names as routes, and assert that they handle the `model` argument. +If set, and there exists a classic loading template, an error will be thrown saying that there is a conflict. +The loading component does not receive any arguments. -## Unresolved questions +#### **error** + +This is the component that will be rendered if an error is thrown as they are thrown today. - - What use cases of controllers are not covered here? +If set, and there exists a classic error template, an error will be thrown saying that there is a conflict. + +The error component will receive the `@error` argument, which will be the error object that is presently passed to the error action. + +## How we teach this + +TODO: write this + +## Drawbacks + +- Routes may feel like they are configuration (this may be fine) + +## Unresolved questions + - Should this RFC include the idea that's been floating around recently where `Route` defines an `arguments` getter where those arguments are splatted onto the `render` component, rather than receiving a single `@model` argument. This would mean that the `Route` would need to have a way to manage the resolved model so that `arguments` doesn't need to worry about pending promises. + - For error templates today, is `this.model` the error? From b3bbda2eeff7d5893f1a47a9007cb9ba5c590fbd Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Wed, 19 Jun 2019 22:13:35 -0400 Subject: [PATCH 18/20] add examples --- text/0000-controller-migration-path.md | 292 ++++++++++++++++++++++++- 1 file changed, 287 insertions(+), 5 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 89edc18425..7105ff5233 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -86,7 +86,25 @@ export default class extends Route { Everything is encapsulated in components so that intent is clear as to which part of the template is responsible for what behavior. Additionally, all three properties may be set to the same component if it was desired to manage loading, error, and success states within components. -This RFC proposes _three_ new static properties on the Route class. +Depending on the state of the model hook, the different components will be rendered at have splatted arguments following this type signature: + +```ts +enum State { + Loading = "loading", + Resolved = "resolved", + Error = "error" +} + +type RouteState = + | { state: State.Loading } + | { state: State.Resolved, data: Data } + | { state: State.Error, error: E }; +``` + +for non-typescript users, the values of splatted to the components will still adhere to the type signature. + + +This RFC proposes _three_ new static properties on the Route class as well the above mapping for arguments to the components for the static properties. #### **render** @@ -95,7 +113,7 @@ Today, there is a default template generated per-route that only contains `{{out If set, and there exists a classic route template, an error will be thrown saying that there is a conflict. -The render component will receive the `@model` argument, which represents the value of a resolved `model` hook. +The render component will receive the `@state` argument as well as the `@data` argument, which represents the value of a resolved `model` hook. #### **loading** @@ -103,7 +121,7 @@ This is the component that will be rendered before the model hook has resolved, If set, and there exists a classic loading template, an error will be thrown saying that there is a conflict. -The loading component does not receive any arguments. +The loading component only receives the `@state` argument. #### **error** @@ -111,11 +129,275 @@ This is the component that will be rendered if an error is thrown as they are th If set, and there exists a classic error template, an error will be thrown saying that there is a conflict. -The error component will receive the `@error` argument, which will be the error object that is presently passed to the error action. +The error component will receive the `@state` argument as well as the `@error` argument, which will be the error object that is presently passed to the error action. + + +## Examples + +Note that the _after_ paths are made up to just show that components are being imported from anywhere. Additionally, sample rendering in the template comments is for demonstrative purposes only. + +- **a loading template related to the route** + + Given + ```ts + // app/router.js + Router.map(function() { + this.route('slow-model'); + }); + ``` + + Before + ```ts + // app/routes/slow-model.js + import Route from '@ember/routing/route'; + + export default class extends Route { + async model() { + const slow = await this.store.findAll('slow-model'); + + return { slow } + } + } + ``` + ```hbs + {{! app/routes/slow-model.hbs}} + {{this.model.slow}} + + {{! app/templates/slow-model-loading.hbs}} + Loading... + ``` + + After + ```ts + // app/routes/slow-model.js + import Route from '@ember/routing/route'; + import LoaderComponent from './components/loader'; + import ShowModelComponent from './components/show-the-model'; + + export default class extends Route { + static loading = LoaderComponent; + static render = ShowModelComponent; + + async model() { + const slow = await this.store.findAll('slow-model'); + + return { slow } + } + } + + ``` + ```hbs + {{! app/routes/components/show-the-model.hbs}} + {{@state}}, {{@data.slow}} {{! renders: "resolved", > }} + + {{! app/routes/components/loader.hbs}} + {{@state}} {{! renders: "loading"}} + ``` + +- **an error template related to the route** + + Given + ```ts + // app/router.js + Router.map(function() { + this.route('erroring-model'); + }); + ``` + + Before + ```ts + // app/routes/erroring-model.js + import Route from '@ember/routing/route'; + + export default class extends Route { + async model() { + throw new Error('whoops'); + } + } + ``` + ```hbs + {{! app/routes/erroring-model.hbs}} + {{this.model.slow}} {{! never renders}} + + {{! app/templates/slow-model-error.hbs}} + {{! this.model is the error instance}} + {{this.model.message}} {{! renders: "whoops"}} + ``` + + After + ```ts + // app/routes/slow-model.js + import Route from '@ember/routing/route'; + import ErrorComponent from 'app-name/components/errors/route-error'; + import ShowModelComponent from './components/show-the-model'; + + export default class extends Route { + static error = LoaderComponent; + static render = ShowModelComponent; + + async model() { + throw new Error('whoops'); + } + } + + ``` + ```hbs + {{! app/routes/components/show-the-model.hbs}} + {{@state}}, {{@data.slow}} {{! never renders }} + + {{! app/components/errors/route-error/template.hbs}} + {{@state}} {{@error}} {{! renders: "error", }} + ``` + +- **all async state to be managed in a component** + + Given + ```ts + // app/router.js + Router.map(function() { + this.route('slow-model'); + }); + ``` + + Before + ```ts + // app/routes/slow-model.js + import Route from '@ember/routing/route'; + + export default class extends Route { + async model() { + const slow = await this.store.findAll('slow-model'); + + return { slow } + } + } + ``` + ```hbs + {{! app/routes/slow-model.hbs}} + {{this.model.slow}} + + {{! app/templates/slow-model-loading.hbs}} + Loading... + + {{! app/templates/slow-model-error.hbs}} + {{this.model.error.message}} + ``` + + + After + ```ts + // app/routes/slow-model.js + import Route from '@ember/routing/route'; + import ShowModelComponent from './components/show-the-model'; + + export default class extends Route { + static error = ShowModelComponent; + static render = ShowModelComponent; + static loading = ShowModelComponent; + + async model() { + throw new Error('whoops'); + } + } + + ``` + ```hbs + {{! app/routes/components/show-the-model.hbs}} + {{#if (eq @state 'loading')}} + {{@state}} {{! renders: "loading"}} + {{else if (eq @state 'error')}} + {{@state}} {{@error}} {{! renders: "error", }} + {{else}} + {{@state}}, {{@data.slow}} {{! renders: "resolved", > }} + {{/if}} + ``` + +- **all async state to be managed in a component with backing class** + ```ts + // app/routes/slow-model.js + import Route from '@ember/routing/route'; + import ShowModelComponent from './components/show-the-model'; + + export default class extends Route { + static error = ShowModelComponent; + static render = ShowModelComponent; + static loading = ShowModelComponent; + + async model() { + throw new Error('whoops'); + } + } + + // app/routes/components/show-the-model.js; + import Component from '@glimmer/component'; + import { inject as service } from '@ember/service'; + + export default class extends Component { + @service router; + @tracked readyYet = 'no'; + + @action + onUpdate(/* element */) { + switch (args.state) { + case 'resolved': + this.readyYet = 'yes'; + break; + case 'loading': + this.readyYet = 'loading!'; + break; + case 'error': + this.router.transitionTo('some-other-place'); + break; + default: + // can't actually get here + break; + } + } + } + + ``` + ```hbs +
+ {{this.readeYet}} + + {{! app/routes/components/show-the-model.hbs}} + {{#if (eq @state 'resolved')}} + {{@state}}, {{@data.slow}} {{! renders: "resolved", > }} + {{/if}} +
+ ``` + + The component signature in typescript could be something like this: + + ```ts + import Component from '@glimmer/component'; + import { RouteState } from '@ember/routing/route'; + + type Data = { + user?: User; + slow: SlowModel[]; + }; + + type PossibleErrors = ClientError | NetworkError; + type Args = RouteState; + + export default class extends Component { + // ... + } + ``` + + ## How we teach this -TODO: write this +The RFCs, [496](https://github.com/emberjs/rfcs/pull/496) and [454](https://github.com/emberjs/rfcs/pull/454) are mainly required for the conceptual ideas that there can exist a world with no component resolver lookups and that components can be imported explicitly. + +The current / classic / old ways of having the resolver _able_ to look up components isn't going to go away any time soon though. For backwards compatibility it most stay for a while. But as this'd be the recommended/explicit path forward, documentation/guides/etc will be updated to show how to use the aforementioned static properties as well as describing how the model hook is mapped to each of the components. + +More specifically recommended will be to use a separate component for each of the three states, but there should still be examples (as above) showing the one-component-does-everything approach. + +In the release that makes these capabilities a default, any pages mentioning the controller, or the route template, or the loading/error templates should be removed or updated to fit the new way of using components + ## Drawbacks From 18cf2ccddd8fed5c7284f6dabfbc091ca8dcf4c2 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Wed, 19 Jun 2019 22:19:50 -0400 Subject: [PATCH 19/20] remove unresolved questions --- text/0000-controller-migration-path.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index 7105ff5233..c81a36e95e 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -402,8 +402,3 @@ In the release that makes these capabilities a default, any pages mentioning the ## Drawbacks - Routes may feel like they are configuration (this may be fine) - -## Unresolved questions - - - Should this RFC include the idea that's been floating around recently where `Route` defines an `arguments` getter where those arguments are splatted onto the `render` component, rather than receiving a single `@model` argument. This would mean that the `Route` would need to have a way to manage the resolved model so that `arguments` doesn't need to worry about pending promises. - - For error templates today, is `this.model` the error? From bd13cf203a3f972d743bbf9e95b28ef254dbb1c8 Mon Sep 17 00:00:00 2001 From: Thomas Leperou Date: Sat, 13 Jul 2019 10:54:26 +0700 Subject: [PATCH 20/20] Fix typo Loading to ErrorComponent --- text/0000-controller-migration-path.md | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/text/0000-controller-migration-path.md b/text/0000-controller-migration-path.md index c81a36e95e..02bd03c444 100644 --- a/text/0000-controller-migration-path.md +++ b/text/0000-controller-migration-path.md @@ -19,12 +19,12 @@ _The goal is for components to make up the entirety of the view layer_. 2. passing state and handling actions to/from the route, loading, and error templates - Out of scope: - - Deprecating and eliminating controllers -- but the goal of this RFC is to provide a _path_ to remove controllers from Ember altogether. + - Deprecating and eliminating controllers -- but the goal of this RFC is to provide a _path_ to remove controllers from Ember altogether. - Not Proposing - Adding the ability to invoke actions on the route. - Any changes to the `Route` or `Controller` as they exist today - + This'll explore how controllers are used and how viable alternatives can be for migrating away from controllers so that they may eventually be removed. ## Motivation @@ -36,13 +36,13 @@ This'll explore how controllers are used and how viable alternatives can be for - how are they different from components? - controllers may feel like a route-blessed component (hence the prior excitement over the Routable-Components RFC) - controllers aren't created by default during `ember g route my-route`, so there is a learning curve around the fact that route templates can't access things on the route, and must create another new file, called a controller. - - For handling actions from the UI, component-local actions and actions that live on services are a better long-term maintenance pattern. Dependency injection especially from the store and router services for fetching / updating data as well as contextually redirecting to different routes. + - For handling actions from the UI, component-local actions and actions that live on services are a better long-term maintenance pattern. Dependency injection especially from the store and router services for fetching / updating data as well as contextually redirecting to different routes. - State and Actions on the controller encourage prop drilling where arguments are passed through many layers of components which make maintenance more difficult. ## Detailed design -First, what would motivate someone to use a Controller today? Borrowing from [this blog post](https://medium.com/ember-ish/what-are-controllers-used-for-in-ember-js-cba712bf3b3e) by [Jen Weber](https://twitter.com/jwwweber): +First, what would motivate someone to use a Controller today? Borrowing from [this blog post](https://medium.com/ember-ish/what-are-controllers-used-for-in-ember-js-cba712bf3b3e) by [Jen Weber](https://twitter.com/jwwweber): 1. You need to use query parameters in the URL that the user sees (such as filters that you use for display or that you need to send with back end requests) @@ -77,7 +77,7 @@ export default class extends Route { async model() { const contacts = await this.store.findAll('contact'); - + return { contacts }; } } @@ -117,7 +117,7 @@ The render component will receive the `@state` argument as well as the `@data` a #### **loading** -This is the component that will be rendered before the model hook has resolved, if present. +This is the component that will be rendered before the model hook has resolved, if present. If set, and there exists a classic loading template, an error will be thrown saying that there is a conflict. @@ -161,7 +161,7 @@ Note that the _after_ paths are made up to just show that components are being i ``` ```hbs {{! app/routes/slow-model.hbs}} - {{this.model.slow}} + {{this.model.slow}} {{! app/templates/slow-model-loading.hbs}} Loading... @@ -232,7 +232,7 @@ Note that the _after_ paths are made up to just show that components are being i import ShowModelComponent from './components/show-the-model'; export default class extends Route { - static error = LoaderComponent; + static error = ErrorComponent; static render = ShowModelComponent; async model() { @@ -274,7 +274,7 @@ Note that the _after_ paths are made up to just show that components are being i ``` ```hbs {{! app/routes/slow-model.hbs}} - {{this.model.slow}} + {{this.model.slow}} {{! app/templates/slow-model-loading.hbs}} Loading... @@ -331,12 +331,12 @@ Note that the _after_ paths are made up to just show that components are being i // app/routes/components/show-the-model.js; import Component from '@glimmer/component'; import { inject as service } from '@ember/service'; - + export default class extends Component { @service router; @tracked readyYet = 'no'; - @action + @action onUpdate(/* element */) { switch (args.state) { case 'resolved': @@ -359,7 +359,7 @@ Note that the _after_ paths are made up to just show that components are being i ```hbs
{{this.readeYet}} - + {{! app/routes/components/show-the-model.hbs}} {{#if (eq @state 'resolved')}} {{@state}}, {{@data.slow}} {{! renders: "resolved", > }} @@ -375,7 +375,7 @@ Note that the _after_ paths are made up to just show that components are being i type Data = { user?: User; - slow: SlowModel[]; + slow: SlowModel[]; }; type PossibleErrors = ClientError | NetworkError; @@ -392,7 +392,7 @@ Note that the _after_ paths are made up to just show that components are being i The RFCs, [496](https://github.com/emberjs/rfcs/pull/496) and [454](https://github.com/emberjs/rfcs/pull/454) are mainly required for the conceptual ideas that there can exist a world with no component resolver lookups and that components can be imported explicitly. -The current / classic / old ways of having the resolver _able_ to look up components isn't going to go away any time soon though. For backwards compatibility it most stay for a while. But as this'd be the recommended/explicit path forward, documentation/guides/etc will be updated to show how to use the aforementioned static properties as well as describing how the model hook is mapped to each of the components. +The current / classic / old ways of having the resolver _able_ to look up components isn't going to go away any time soon though. For backwards compatibility it most stay for a while. But as this'd be the recommended/explicit path forward, documentation/guides/etc will be updated to show how to use the aforementioned static properties as well as describing how the model hook is mapped to each of the components. More specifically recommended will be to use a separate component for each of the three states, but there should still be examples (as above) showing the one-component-does-everything approach.