Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

An alternative to Controllers #499

Open
wants to merge 17 commits into
base: master
from

Conversation

Projects
None yet
@lifeart

This comment has been minimized.

Copy link

commented Jun 10, 2019

In current queryParams implementation qp-values binded to model, it's possible to have different states for one page for different models, how we can handle it using new approach?

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

RFC 380 proposes a way to store query params per sub-route :) This is no longer the case, as that RFC is going to be changing how we think about query params. Please read the updates over there :)

the gist is that we'll probably rely on community solutions for caching of query params between routes, maybe like https://github.com/NullVoxPopuli/ember-query-params-service/blob/master/tests/acceptance/navigation-test.ts#L56

@lifeart

This comment has been minimized.

Copy link

commented Jun 10, 2019

@NullVoxPopuli my comment about this behavour https://ember-twiddle.com/b9e78f44f0a63a1a5b0f75ca28b9ebef - try type some search and change models.
each model has personal "queryParams" and if we change model, queryParams also changed

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

Yup. that works with RFC 380 :)

@ef4

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

This is on a good track but needs more detail. Can you expand upon the lookup priority rules?

For example, I think if app/templates/your-route.hbs or app/controllers/your-route.js exist, they probably need to keep doing what they do now. But if they don't, we can add the new behavior to use app/components/your-route.js and/or app/templates/components/your-route.hbs (essentially "the component named "your-route").

That would be safe as long as we gave you an upgrade step that generated explicit template files for any of your classic routes that were previously relying on {{outlet}} being auto-generated for them.

I also think we end up in an inconsistent state if we don't also allow app/templates/slow-model-loading.hbs to become app/templates/components/slow-model-loading.hbs and app/templates/error.hbs to become app/templates/components/error.hbs. As far as I can tell that doesn't require any design other than saying what parameters they will see (like maybe @error).

[Edited: I had a ton of typos in my path examples that made it hard to follow before.]

@jrjohnson

This comment has been minimized.

Copy link

commented Jun 10, 2019

The title doesn't really match the RFC @NullVoxPopuli maybe Alternative Controller APIs? Or something less final since you're not actually proposing the removal of controllers

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

@jrjohnson That's fair, removing controllers would require a deprecation RFC, which this is not

@NullVoxPopuli NullVoxPopuli changed the title Remove Controllers An alternative to Controllers Jun 10, 2019

@jasonmit

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

Should the RFC recommend alternative approaches to storing long lived state? I imagine the rouleable components will not be singleton's - correct?

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

@ef4 I've added some details around look priority :)

I also think we end up in an inconsistent state if we don't also allow...

I think addressing loading/error states should happen in a separate RFC. As far as I understand it, error and loading stuff is just templates. I'd like to keep this RFC focused on controllers, unless the community or the core teams say it absolutely needs to be done. idk :)

I've changed my mind. changing the error and loading stuff keeps cohesion :)

(but I'm in favor of @error getting sent to anything error-y) :)

@jasonmit correct. any component used for routing should be considered ephemeral. Services should be used for long-term state. :)

@Panman8201

This comment has been minimized.

Copy link

commented Jun 10, 2019

FWIW, I feel like the move away from controllers can be aligned with router refactors. In that, once folks move to (potentially) new route semantics, controllers will no longer be used/loaded. Instead, some form of a component will become the route template, such as a template-only component or maybe the route defines which component to render, etc. More thoughts coming as soon as I get to my ember2019 post.

@stefanpenner

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

@jasonmit correct. any component used for routing should be considered ephemeral. Services should be used for long-term state. :)

bingo

@jasonmit

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

I imagined the store(s) would be backed by a service but will there be a recommended approach regarding strategy or common API that can be shared? Or is this an opportunity for Ember, as a the framework, to step out of the way and lean more on community solutions?

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

@jasonmit,

I imagined the store(s) would be backed by a service but will there be a recommended approach regarding strategy or common API that can be shared?

service injections cover this :)

import Component from '@glimmer/component';
import { inject as service } from '@ember/service';
import { task } from 'ember-concurrency';

export default class MyComponent extends Component {
  @service store;
  @service toast;

  @task(function*() {
    const record = this.store.createRecord('post', { ...whateverAttributes });

    try {
      yield record.save();
      toast.success('yay!');
    } catch (e) {
      toast.error(e);
    }
  }) create;
}
<button {{on 'click' (fn (perform this.create))}}>
  Create a Post
</button> 
@jasonmit

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

I'm aware of the service inject API but feel it doesn't quite layer over what we have now.

Currently there's a 1:1 mapping of route backed state vs what sounds like a recommendation for arbitrary service(s) used to hang state from. Uniformity across codebases breaks down here and may be worth documenting recommended paths in the migration docs. Perhaps something for the Ember learning team.

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

what sounds like a recommendation for arbitrary service(s)

hmm, maybe I misunderstand? can you provide some concrete examples of how you use controllers today, that components wouldn't be able to handle?

Uniformity across codebases breaks down here

can you expand on this?

@willviles

This comment has been minimized.

Copy link

commented Jun 10, 2019

I understand that it's more a concern that it's a move from a highly conventional approach (FooRoute => FooController), to a more ad-hoc approach (FooRoute => ???Service/s).

Can someone explain why routes themselves can't be singletons and store their own long-lived state? Like a service that just automagically has hooks invoked on route changes?

@jasonmit

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

hmm, maybe I misunderstand? can you provide some concrete examples of how you use controllers today, that components wouldn't be able to handle?

Any state that isn't queryparam state. One example could be an email client that keeps draft state for emails in memory over the lifetime of the app instance. Today, you might throw it in a controller or a service. If you're in a large codebase then there may be many instances like this and you'd end up with really small, oddly shaped, services when migrating an app over to a controller-less world.

So the question I've been failing to pose (sorry, limited by typing on my phone) is are we recommending moving everyone to services for this state or is there a common API that can be shared for mapping route backed state without one off services where a dedicated services may be a bit too heavy.

@amyrlam amyrlam referenced this pull request Jun 11, 2019

Merged

The Ember Times No. 102 - June 14th 2019 #145

16 of 37 tasks complete
@basicBrogrammer

This comment has been minimized.

Copy link

commented Jun 12, 2019

Not sure if we can pull ideas from this post https://emberway.io/skeleton-screen-loading-in-ember-js-2f7ac2384d63 but it feels like it might have some parallels

After working with react a tiny amount, controllers started to click for me. They are very similar to the "container" component pattern in react.

Seems like the 2 major milestones for this RFC would be what do we do with actions and what to do about the loading state.

Happy to spike with you on it! (first time joining in on the RFC convos so bare with me)

@webark

This comment has been minimized.

Copy link

commented Jun 12, 2019

Perhaps we could move to utilizing the “loading” and “error” actions on the route rather then relying on the implicit “-loading” intermediary routes?

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 12, 2019

@webark

Perhaps we could move to utilizing the “loading” and “error” actions on the route

those actions are on the route: https://guides.emberjs.com/release/routing/loading-and-error-substates/#toc_the-loading-event

rather then relying on the implicit “-loading” intermediary routes?

I have a plan for a followup RFC to make some changes to loading and error handling in general to make it more explicit

loading and error templates will follow the same look up rule change as the main template

@basicBrogrammer

This comment has been minimized.

Copy link

commented Jun 12, 2019

Not 💯 familiar with the loading/error states ... would it make sense to offload these to the router service ? Then in the routed component you could set the loading state router.loaded() or router.failed() after loading data 🤔

@knownasilya

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

If people want long-lived state that is scoped per route, they can say this.thingService.things[routeName] = rememberThisThing.

A similar approach for long lived state could utilize an addon like https://github.com/stefanpenner/ember-state-services, and use the route name as the key that stores the state.

@andrewcallahan

This comment has been minimized.

Copy link

commented Jun 13, 2019

This is really well thought out. Many of us have complained about Controllers, but you've actually done something about it!

I have to agree with @flashios09 though, that this approach feels like simply replacing Controller js files with Component js files.

Assuming Module Unification isn't coming before this, I'm hesitant to add hierarchy to the /components folder and to in any way tie the /components folder structure to the URL structure. If Module Unification lands before this, I'm also hesitant to have Component .js and .hbs files in the new /routes folder. That seems confusing to me and is begging for something like Routable Components.

Speaking of Routable Components, I'm pretty excited to hear @pzuraq mention that the Core Team is interested in taking another swing at them! If that is indeed the case, then whatever Ember does to remove Controllers should have this endgame in sight.

The approach you outline seems to require a fair amount of work and imposes additional architectural decisions that could need to be undone depending on how Routable Components ultimately work.

If Ember is really going to get rid of Controllers, and move to Routable Components, here is how I envision the transition happening with the least amount of work to merge an RFC, the easiest transition path for developers, and maximum architectural flexibility for the Core Team.

Phase 1: Remove Controllers De Facto

  1. The Core Team makes a commitment to remove Controllers and replace them with Routable Components when they announce the new Roadmap.
  2. In the next Ember release, everything besides Query Params on Controllers (state, actions, etc) get deprecation warnings.
  3. Developers then begin to move over most of the logic away from Controllers. This would entail rendering a single Component in any stateful Template and involve mostly copying and pasting.
  4. Query Params service is launched.
  5. Apps that used Query Params on their Controllers move to use the new Query Params Service.

Phase 2: Remove Controllers De Jure

Hopefully, many developers would have done this work by now, and have no more Controllers in their apps. Now Controllers can officially be removed and the deprecation warnings now become actual errors. For apps that have been keeping up, it's a non-event upgrade. For apps that haven't, this is a kind of a line in the sand/wake up call that it's time they need to upgrade. What we are left with now is a world with no Controllers, stateless Templates, and many apps that use Templates simply to render a single Component. In other words, we are perfectly primed for Routable Components!

Phase 3: Routable Components

It took a bit of time to get here, which is good in two aspects. First, it gives developers time to update their apps without causing too much churn. Second, it gives the Core Team time to get the interface for Routable Components right. Routable Components would be a big change in Ember and it's vital to avoid what Sandy Metz would call the "wrong abstraction". I have no clue as to the final design of Routable Components, but my suspicion is that we would get to the best design if we give the Core Team time and imposed no more constraints during the transition phase away from Controllers.

This outline is a bit handwavy, and I'm sure there are details I'm overlooking, etc - so please consider this a thought exercise to build and iterate upon!

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 13, 2019

@andrewcallahan

I have to agree with @flashios09 though, that this approach feels like simply replacing Controller js files with Component js files.

This is not true -- or at least not what I'm trying to convey. In my opinion, I believe the happiest path is to use the template-only scenario. Which is just what we have today, but without a backing controller.
Additionally, this is how I see it working for each of the types of layouts we have:

** classic **

  • My personal happy path

    • app/routes/my-route.js
    • app/templates/my-route.hbs
  • For the case where you really do have a single root entry point

    • app/routes/my-route.js
    • app/components/my-route.js
    • app/templates/components/my-route.hbs

** MU or MU-inspired, (eventually) **
- src/ui/routes/my-route/route.js
- src/ui/routes/my-route/-components/{component.js, template.hbs} (exact path TBD, but something co-located like this -- could be index, entry, render, whatever. we aren't at that point yet)

, I'm hesitant to add hierarchy to the /components folder and to in any way tie the /components folder structure to the URL structure.

~oh, I def wouldn't. I would most certainly encourage the use of the template-only rendering context, as I have shown in some of the rendering examples from emberclear 😉 ~

I no longer think this would be that bad. With components mimicing the route structure, and with template imports, the components directory would be pretty nicely organized.

If Module Unification lands before this, I'm also hesitant to have Component .js and .hbs files in the new /routes folder. That seems confusing to me and is begging for something like Routable Components.

yeah, I wouldn't want that either. components are core to frontend, and it'd be weird to give them any sort of special treatment apart from other components (aside from resolver lookup conventions in the classic world)
maybe what would be better for an eventual convention in a MU-like app is this:

// for the path `my-route`
// this file exists at `src/ui/routes/my-route/index.js`
import Route from '@ember/routing/route';
import MyComponent from './-components/my-component';

// named export must match name of route
export class MyRoute extends Route {
  async model() { ... }
}

// the default export is the rendered thing that get's passed the `@model` arg.
export default MyComponent;

The approach you outline seems to require a fair amount of work and imposes additional architectural decisions that could need to be undone depending on how Routable Components ultimately work.

can you explain these thoughts more? (cause I either don't see it, or don't understand 😢 )

If Ember is really going to get rid of Controllers, and move to Routable Components,

can we not use the term routable components? this term has some much baggage and frustrating with it. 🤣

Phase 1, Phase 2, Phase 3

These phases feel a little backwards to me. In order to be totally backward compatible, we'll need to do something like the following:

  • query param alternative should happen first as it's a pre-req, and already a huge pain point in existing apps. So the query params RFC, #380, can help people out long before anything happens with controllers :)
  • we need to first have an alternative, see how the community adopts it, and if it really is going to work.
    • this is where components via this RFC or some derivative or something completely different comes in to play.
    • documentation would need to be updated to recommend the new happy path. the guides and api documentation either minimizes its mention of controllers or has a side-note somewhere mentioning the historical baggage that they will be at this point
    • this also means that we'll probably have a long period of time where people are migrating and have both controllers and whatever the controller-alternative is. Migrating takes a while :)
  • next we can decide to finally deprecate controllers altogether. (you import or instantiate a controller, you get deprecation warning)
    • hopefully migration speed picks up here, but we'll need a deprecation RFC to make this official
  • finally we can remove controllers as part of a major version bump as it'd be breaking change (whether that be ember 4 or 5 or further out is totally up in the air!) 😄
@andrewcallahan

This comment has been minimized.

Copy link

commented Jun 14, 2019

Thanks for the quick reply @NullVoxPopuli! Let's go one-by-one with the issues you brought up to see if we can converge in our understanding ;)

1. Does this approach simply replace Controller js files with Component js files?

Here is what I understand you are proposing

** Classic **
app/routes/my-route.js
app/components/my-route.js
app/templates/components/my-route.hbs

** MU **
src/ui/routes/my-route/route.js
src/ui/routes/my-route/-components/component.js
src/ui/routes/my-route/-components/template.hbs

For the classic style, it looks like app/controllers/my-route.js is just replaced with app/components/my-route.js. I get that it's a bit more complicated than that, but fundamentally it's the same thing - some resolver looks through the files and hooks up a .hbs file in the /templates folder with a .js file in another folder (this time /components instead of /controllers). That is something I'd like to move away from. I have comments on the MU style later on.

2. Should Templates have .js files at all?

In my opinion, I believe the happiest path is to use the template-only scenario. Which is just what we have today, but without a backing controller.

That is what I think would be optimal too. What I don't understand is why you hook up the Template file to a .js file in the /components folder (Classic) or have a Component .js file in the new /routes folder (MU) at all. Both of these would lead to stateful Templates, and not a "template-only scenario".

3. Should the /components folder have hierarchy tied to the URL

I'm confused here. On the one hand, you say "oh, I def wouldn't", but in the classic scenario you laid out app/components/my-route.js is tied to the template app/templates/my-route.hbs. For nested routes, I would presume this would mean app/components/rentals/edit.js would similarly be tied to the app/templates/rentals/edit.hbs file. If so, the /components folder structure would be directly tied to the route structure - just like the /templates folder is today.

4. Should Component .js and .hbs files be in the new /routes folder?

I'm again confused here. On the one hand, you say "yeah, I wouldn't want that either", but in the MU scenario you have the Component .js and .hbs files in it:

  • src/ui/routes/my-route/-components/component.js
  • src/ui/routes/my-route/-components/template.hbs

Now, your suggestion (below) of specifying a Component from the Route file is interesting and would avoid the pitfall of having two places for Component .js files. I could see something like this being one possible way to get Routable Components but am not sure it's needed before then.

maybe what would be better for an eventual convention in a MU-like app is this:

// for the path my-route
// this file exists at src/ui/routes/my-route/index.js
import Route from '@ember/routing/route';
import MyComponent from './-components/my-component';

// named export must match name of route
export class MyRoute extends Route {
async model() { ... }
}

// the default export is the rendered thing that get's passed the @model arg.
export default MyComponent;

5. Does this RFC, as it stands, take a fair amount of work?

I'm not sure exactly, but I would pose that question back to you or Core Team members lingering around here. Setting aside the work with the Query Params Service, it seems like more work to do anything other than just put in deprecation warnings for Controllers, which is what I'm proposing to do until Routable Components land.

6. Would this approach make additional architectural decisions before Routable Components land?

It seems so to me. This may not be a bad thing if they are the right ones. However, if for instance Components are specified in a Route in your above example, that is a decision that could potentially be unmade in the future, if say Routable Components occur somewhere else such as router.js. I really don't have too much insight here - just trying to think things through and "measure twice, cut once".

7. Should we call Routable Components "Routable Components"?

Of course 😉! In all seriousness though, if there is a better name I'm all ears! The final implementation could also help with naming - perhaps it's more along the lines of "Route Specified Component" as your example above outlines.

8. What is the proper order to move from Controllers to Routable Components?

These phases feel a little backward to me.

Good points.

Rethinking this a bit, here is an alternative proposal based on your feedback.

Phase 1: Query Params Service

  1. Merge #380
  2. Update guides on how to use Query Params
  3. Deprecate the use of legacy Query Params in Controllers
  4. Eliminate the use of legacy Query Params in Controllers

Phase 2: Remove Controllers

I still think that removing Controllers, even if Routable Components never materialize is worthwhile in and of itself. Controllers are (to me) so confusing, hard to teach, and unnecessary that I'd be happy just to see them gone. I'm certainly sympathetic to the idea of having a replacement for something before you eliminate it. In the case of Controllers though, I don't think this is needed and in fact I am proposing that the replacement to Controllers at first is...nothing! Just make Templates stateless and require developers to render a single top-level component if they want state. I'm hoping this is temporary, and that Routable Components land ASAP. However, the point of this RFC is to deliver something quick to deal with Controllers that is independent of Routable Components, which I believe simply removing Controllers fulfills.

  1. Put deprecation warnings on Controller actions and properties
  2. Update guides to remove references to Controllers, and add in references to using a single Component in a Template in order to use state and actions.
  3. Eliminate Controllers after enough time has passed.

Phase 3: Routable Components (or some other name)

I love your idea of having an alternative to test out to gauge how the community responds. Perhaps this could live as an addon at first? Part of the dream of modularization is to make these experiments easier - but I'm not sure if it's feasible for this or not. As you say "Migrating takes a while", so I'm not sure when this phase will happen, but to me, I'm in less of a rush to get Routable Components than I am to get rid of Controllers, hence why I'm okay with it being last. Also, I really hope that this time the feature actually lands 😂.

Hopefully, I helped clear up some of your questions. I would love to hear your thoughts on this and get to a place of mutual understanding!

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

@andrewcallahan I think we're misunderstanding each other on parts, but I think you've convinced me of something, which I wasn't expecting, but I'll get to that later.

  1. Does this approach simply replace Controller js files with Component js files?
    I get that it's a bit more complicated than that, but fundamentally it's the same thing - some resolver looks through the files and hooks up a .hbs file in the /templates folder with a .js file in another folder (this time /components instead of /controllers).

I think you're over thinking it a bit.

when you do ember g component my-route-name, it generates 2 files and a test. I'm just saying that for classic style, those files would be used. Nothing magic here -- just a component. :)

  1. Should Templates have .js files at all?
    What I don't understand is why you hook up the Template file to a .js file in the /components folder

I was trying to convey that the template file would take priority. if you have route template (like you would today), there would be no component lookup (like it works today).

For nested routes, I would presume this would mean app/components/rentals/edit.js would similarly be tied to the app/templates/rentals/edit.hbs file. If so, the /components folder structure would be directly tied to the route structure - just like the /templates folder is today.

Yeah, I def don't want that. Which as you bring up some of these issues, I'm thinking we should close this RFC until we have Template Imports, because I think this is kind of too hard, and we need to soley talk about it from the aspect of co-location.

I no longer think this, and realized that we can do feature work that breaks cohesion with the broader ecosystem and that editions bring that cohesion together. :)
once we get enough features together that mesh well a new edition will land, and stuff will be less awkward.

  1. Should Component .js and .hbs files be in the new /routes folder?
    but in the MU scenario you have the Component .js and .hbs files in it:

In module unification, this is call a private collection. Today, there is a bunch of resolution magic to make this work, but I want to encourage this pattern for all components that are solely for a route and nothing else. I've even used in React projects, and it's worked well. :)

I could see something like this being one possible way

My biggest concern with it would be that it would feel configuration heavy and maybe error-prone due to misspellings.

  1. Does this RFC, as it stands, take a fair amount of work?

it shouldn't. Of course I haven't looked into the rendering/resolution logic, but I assume it's adding one additional lookup rule, and then making sure the correct arg is passed to the component.
This may be a over simplification.

Phase 2: Remove Controllers

I think this is too disruptive and would require a major version bump. :-
We should be resistant to major changes.

I understand that controllers are hard to teach, but we must opt for easy migration paths when available, and not forcing people to jump ship. Remember when the "Routable Components" RFC was getting hyped, people started trying to avoid controllers at all cost? that was a bad idea. 😉

At the moment, I think this is the easiest path:

  • wait for the query params RFC
  • wait for template imports
  • wait for route/template co-location (or new project structure)
  • encourage stateless templates at the top level of a route, utilizing self-managed private collections per route to import whatever is needed at the route-template
  • with all of this, and probably a bunch of other features, cut a new edition to bring back cohesion with all the new features / layout stuff / patterns etc
  • after the edition, deprecate controllers as well as deprecate whatever else needs deprecating post-edition

So, yeah:

TL;DR: I'm not even sure controllers need to be replaced anymore... But maybe just deprecated after whatever the edition after Octane is.

@andrewcallahan

This comment has been minimized.

Copy link

commented Jun 14, 2019

Thanks again for thinking this through with me @NullVoxPopuli - I now have a much better understanding of some of the aspects of your RFC that were confusing me!

I'm sad to see this RFC close (if only for now), but I think exploring how exactly Controllers would be removed, and what the implications would be has been super illuminating.

I'm hoping that as the Core Team makes the Roadmap this year if Routable Components replacing Controllers are indeed on the table, they should definitely look through this conversation for context around pre-requisites, timing, and roll out strategy.

@ef4

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

I agree that this is probably nicer if we base it off co-location, which is already a merged RFC and not a big lift in terms of implementation. I don't think that means this RFC is premature, we have that merged RFC to build off of.

Some of the discussion I'm seeing is IMO repeating the mistakes that caused the previous thing called "routable components" to never ship. If this RFC succeeds, it will be because it keeps the scope reasonable.

I can get behind this RFC if it results in a simple rule that's easier to learn than current Ember. Something as simple as what I wrote above:

Any time you looked up route template X and didn't find it, look up component X too and if it exists, use that.

Notice that says nothing about how components are looked up, where they go, etc. All that is both outside the scope of this RFC and necessarily should work the same as the rest of Ember, so there's less to learn. If route templates are looked up in a special way, nope nope nope. If we still end up with two different kinds of templates in two different folders hierarchies, I will be a strong 👎.

I see no downside to having all your routes backed by components that are in their regular componenty directories. Yes, that means the route hierarchy can appear under app/components. I disagree that that's bad. You still have one true map to where all the route entrypoints are, which is router.js. That is the place to browse to learn every Ember app's structure. It's already the most reliable place to look, especially given classic vs pods vs aborted-MU, and eventually classic vs template-colocation.

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

Yeah, maybe some of my explicitness here has shot me in the foot.

I need to update the RFC anyway. I'll try to comb over the confusion parts and address/simplify.

Thanks, @ef4!

NullVoxPopuli added some commits Jun 14, 2019

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

I've updated this RFC as well as updating a bunch of my comments

@michaelrkn

This comment has been minimized.

Copy link

commented Jun 15, 2019

I love this RFC! Two thoughts:

  1. After this lands, would it make sense for ember new route to create app/templates/components/route-name.hbs instead of app/templates/route-name.hbs (or, assuming co-location lands first, app/components/some.hbs`) by default?

  2. I can't find it now, but I remember seeing a blog post suggesting an API that looks something like:

export default class SomeRoute extends Route {
  data() {
    return RSVP.hash({
      songs: this.store.findAll('song'),
      albums: this.store.findAll('album')
    });
  }
}

And when this data hook is used instead of model, then instead of loading a component as if it were invoked as <SomeRoute @model={{this.model}} />, the component is loaded as if it were invoked as <SomeRoute @songs={{this.model.songs}} @albums={{this.model.albums}} />.

One thing I really like about the RFC as it is now is how minimal the proposed change is. But it leaves us with an API that has this notion of a singular "model" that doesn't always make sense in a component-based world (and that actually often didn't make sense in our old controller-based world, either).

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2019

. But it leaves us with an API that has this notion of a singular "model" that doesn't always make sense in a component-based world (and that actually often didn't make sense in our old controller-based world, either).

yeah, I don't think this is a feature people would want to use right away. I think there are a few follow up RFCs that would be needed to make things noice.

I do think renaming the route's lifecycle hooks would be beneficial, as model is somewhat of a backend data term, and is def confusing to people who aren't familiar with the nomenclature.

@knownasilya

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2019

@michaelrkn a future splarguments feature would cover number 2. Like <SomeRoute ...{{this.model}}/>

@michaelrkn

This comment has been minimized.

Copy link

commented Jun 17, 2019

@NullVoxPopuli I'd be interested in what you think the follow-up RFCs would be - it might be good to add under "unresolved questions", or at least as a comment here. Thanks!

@pzuraq

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

@michaelrkn I think you may be referencing @chadhietala's blog post.

@knownasilya one thing to note about that, I think the likely path for splarguments is to first introduce just the ability to spread arguments themselves, just like ...attributes. Something like ...@arguments.

The ability to arbitrarily spread any value, especially dynamic values, is much more complicated, both on an implementation level and on a design level. I'd like to see us get there, but I'm not sure how long it'll take, whereas the much more scoped ability to spread all arguments will probably be something that can ship much sooner.

@simonihmig

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

Thanks @NullVoxPopuli for working on this, I think this is long overdue! :)

Though I do have some questions and concerns:

  • I assume this should be a non-breaking change, as new Ember features are supposed to be introduced in that way. But as @ef4 already touched mentioning the automagically generated {{outlet}} templates when no explicit route template exists, having a component with a name that by coincidence matches the name of a template-less route will make the route render that component (that is not meant to render that route at all) under the new lookup rules. Fixing this is easy by adding that explicit {{outlet}} template, but this still would be a breaking change I think!
  • Can you add some example how component-based route rendering is supposed to work with nested routes? So /foo/bar will render the <Foo::Bar> component. But how should the template for <Foo> look like? Will it have an {{outlet}}? Does that even work now?
  • What I do not like so much about the proposal is the ambiguity added through having either a template-only route or a component-based one.
    • So when you want to have a look at what is rendered for a given route, where do you look first, at app/templates/my-route.hbs or at app/templates/components/my-route.hbs? I believe this could be confusing, especially for new devs that aren't aware of the lookup rules, and wonder what kind of magic is happening there.
    • Given that named args (RFC 276) and {{this.arg}} (RFC308) have introduced more determinism and reduced ambiguity, this feels like a step backward IMHO
    • it introduces another Emberism, that build tools (Embroider) AFAIU have to understand and resolve, before other tools like Webpack would be able to do their work, e.g. route-based code splitting.
  • Given the lookup rules will implicitly try to lookup a component with the same name as the route, this opens the door for naming conflicts with existing components.
    • Say you have a /user route, that you want to refactor to not use a backing controller anymore after upgrading to the new Ember version shipping the changes here. But you already have a <User> component, to e.g. render a simple user avatar, so not related to the /user route at all, but instead spread over many dozen places all over the app.
    • That would require you to rename the existing component to make space for the new route-component, which could be a major PITA
    • It feels like you have to fight the framework, as the assumption of having a component with the same name as the route is baked into the framework

Given this, and that you already stated you would favor template-only routes, I wonder what the reason is to not propose just that, and not introduce the component lookup change at all?

Like when you do have some state/action on the controller now, you can still move that to a component (with either the same name as the route of a totally different one btw), and render that component explicitly in the route template:

<MyComponent @model={{this.model}}>

AFAICT this would solve all the problems mentioned above. In fact it wouldn't even require changes on the framework itself, it would "just" be an (officially) recommended pattern and would require "only" changes to the guides. And pave the way for a future deprecation of controllers. Or do I miss something?

@chadhietala

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

I see this has become a lively discussion...

I believe #454 #496 are critical for explaining how the component is "looked up". The TL;DR is that the component should not be looked up at all. The SFC and template import primitives are all about by-passing the container for resolution and relying directly on JS imports. For instance the below example is a direction I would welcome.

import FooComponent from '../components/Foo'
import Route from '@ember/routing/route';

@render(FooComponent)
export default class extends Route {
  arguments() {
    return { name: 'Chad' }
  }  
}
@pzuraq

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

Personally I would vote for static class fields, so we could avoid the extra decorator import and assign multiple components for different states:

import FooComponent, { LoadingComponent, ErrorComponent } from '../components/Foo';
import Route from '@ember/routing/route';

export default class extends Route {
  static render = FooComponent;
  static loading = LoadingComponent;
  static error = ErrorComponent;

  arguments() {
    return { name: 'Chad' }
  }  
}
@chriskrycho

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

Seconding/thirding @chadhietala's sentiment here. My biggest hesitation with the proposal as I understand it is that it's adding another magic rule around routes/controllers/components which new Ember devs would have to learn. If it can be built instead on other primitives that we're aiming to land, which eliminate the implicitness and give tools (IDEs, static analysis, etc.) a better point to hook into, that seems like a substantial win.

I don't want to suggest getting stuck in a "design it all up front" mode, but something like the static class fields approach given the ability to actually import components would be a very, very big win over "oh, you can use a controller or you can have a component named the same thing and that just works, but you'll have pain when migrating if you have an existing component which has the same name as a route and delete the controller…"

Another upside to the form @pzuraq proposed is that you could use the same component for all three states if you had one that composed a hypothetical <Loader> component with the actual layout for each of the custom states within it.


Given that the component colocation RFC was accepted, the text here should also be updated to reflect that in terms of the default locations of the component backing class and template files!

@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 17, 2019

I believe #454 #496 are critical for explaining how the component is "looked up". The TL;DR is that the component should not be looked up at all.

I'd be a huge fan if we just had a js-less template per route that either defaults to outlet, or just is what we have today (but eventually with no controller)

For instance the below example is a direction I would welcome. (@render)

that's interesting, but if we also need to follow the same pattern for loading and error, we'd wind up with something like this:

@render(FooComponent)
@whenLoading(LoadingComponent)
@onError(ErrorComponent)
export default class extends Route {
  arguments() {
    return { name: 'Chad' }
  }  
}

Personally I would vote for static class fields, so we could avoid the extra decorator import and assign multiple components for different states

yaaaaaas (I mentioned this in my #EmberJS2019 blogpost) :)

Given that the component colocation RFC was accepted, the text here should also be updated to reflect that in terms of the default locations of the component backing class and template files!

legit 👍

ok, so... My favorite options so far are the static fields, where:

Another upside to the form @pzuraq proposed is that you could use the same component for all three states if you had one that composed a hypothetical component with the actual layout for each of the custom states within it.

I think that's awesome, and would give a way for people who want to heavily use components for everything a way to do so while also still being semantic/ergonomic for those of us that want to use routes :)

So, next updates to this:

  • rephrase everything in to be in a post SFC/Template-import world
  • static fields
  • ditch resolving?
    • this resolves @simonihmig's concerns over accidental breaking changes, but I think @ef4 was in favor of the resolutions? idk. (sorry!)
@NullVoxPopuli

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

Updated some things, mostly trimmed away a lot things that are now out of date.

  • I've added static fields
  • I mostly don't mention resolving
  • Add links to dependent RFCs

What I still have to do is flesh out examples, learning materials, and more motivation and transition for how migrating away from controllers would work. (I mostly just wanted to remove more of the outdated stuff so conversation can continue (in case people read the RFC before the comments))

@andrewcallahan

This comment has been minimized.

Copy link

commented Jun 18, 2019

Love the changes to this RFC you've made @NullVoxPopuli! This feels so much simpler now and is a very intuitive way to connect a Route to a Component.

As this RFC stands now I see a lot of big wins:

  1. Allows for the elimination of the /controllers folder since there is now an "alternative"
  2. Allows for the elimination of the /templates folder since Components are now imported and declared in the Route (and this RFC assumes co-location is merged)
  3. No more resolving .hbs files "magically" based on file name or location - everything is explicitly imported and declared in the Route (this will especially be helpful for loading and error states)
  4. The /components folder hierarchy remains decoupled from the URL structure (which was my biggest point of confusion and disagreement before)

These are all so important that I don't know which is my favorite. It's all very exciting! Also, this feels awfully close to "Routable Components", whatever that may turn out to be 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.