Add setRouteComponent API

[pzuraq]

Rendered

[chriskrycho]

Thanks for calling this out

[chriskrycho]

One unanswered question I thought of here: how does this interact with the existing abilities to muck with specific rendering patterns on routes, render and renderTemplate? Is using those with this a hard error, a deprecation, a warning, etc.? And what's the actual behavior—is renderTemplate ignored if using setRouteComponent?

[pzuraq]

Ah yes, we should address that. I believe these APIs would be ignored, since we are fully ignoring the route's template in general. Since these APIs are deprecated, I think this also won't introduce any inconsistencies.

[chriskrycho]

Had one question, but my overall sentiment is: ❗

[snewcomer]

@Ravenstine 👋

[snewcomer]

Some patterns I have seen is

articles
  list
  detail

and articles.hbs is just an {{outlet}}.

How strict is this API to only include a component? Could handlebars expressions be allowed as the first argument?

[Ravenstine]

How strict is this API to only include a component? Could handlebars expressions be allowed as the first argument?

Whether or not this fits in to the current proposal, I think this is a really cool idea! I've found it useful in the past to be able to have all a route's assets (templates and controllers) come from one JS file.

This is something I've achieved before by overriding the resolver so that it would look for a template named export from a route.

Being able to use setRouteComponent to do something similar would be pretty awesome, even if it required a separate createComponent function to maintain the semantics.

[pzuraq]

So the idea with ember-template-imports is that an hbs expression is a template-only component, and using them together you could definitely do this in practice:

import { hbs } from 'ember-template-imports';
import Route, { setRouteComponent } from '@ember/routing/route';

export default class MyRoute extends Route {}

setRouteComponent(hbs`Hello, world!`, MyRoute);

If we find that it still makes sense to associate a template that is not a component with a route in the future, then we can add that functionality either via a separate API, or via this API. My hope is that we can start to do the opposite though, lowering the distinction between templates and components even more in the future. I would like, for instance, for hbs used in tests to actually be inline-template-only-components, rather than just templates.

[chriskrycho]

@Ravenstine note that this is a low-level API on which exactly those kinds of things would be built. In the same way that setComponentTemplate allows us to add ember-template-imports syntax on top of a component definition (rather than using setComponentTemplate directly, this API is the necessary building block to allow you to do something similar in the future.

Worth note, though, that it wouldn't be exactly like that, because the backing class for a component is not a Route. You'd always want to be attaching a component definition, whether that's a template-only component or a class-backed component. With template imports and this API, though, you can easily imagine being able to do something like this, in a hypothetical future where we did add the decorator approach you suggested in the main thread:

import Route, { withComponent } from '@ember/routing/route';
import Component from '@glimmer/component';

const MyRouteView = 
  <template>
    <div>{{@model.someProp}}</div>
  </template>

@withComponent(MyRouteView)
export default class MyRoute extends Route {
  model(params, transition) {
    // ...
  }
}

(Note that I'm not suggesting I want that specifically; just noting that the point is having the appropriate set of primitives that lest us build and experiment with a variety of things in this space.)

As a closely related point, @snewcomer we wouldn't want this to accept strings there, but with strict mode we can straightforwardly use any tool which produces a strict-mode component, so you can imagine just passing <template>{{outlet}}</template> there (or the equivalent with hbs) and it would Just Work™.

Edit: to clarify re: "strings", basically what @pzuraq said. We were apparently typing simultaneously. 😂

[Ravenstine]

How do we feel about supporting decorator syntax? setComponentTemplate, which is obviously similar to this idea, doesn't support it, but I'm guessing that's because ES decorators weren't fully fleshed out when that was being designed. Now that ES decorators are here to stay, I think allowing the option of using this new API that way would provide a slightly nicer way to use it, although it's only a hair different than just calling it as a normal function.

[chriskrycho]

The decorators syntax is still not stable, and setComponentTemplate actually came after our adoption of decorators. Decorators were officially adopted in #408 (merged January 2019), while setComponentTemplate was #481 (merged May 2019). Note that you definitely could write a decorator which does this, though; it's not especially complicated:

function withComponent(SomeComponent) {
  return (Route) => {
    setComponentTemplate(SomeComponent, Route);
    return Route;
  }
}

import Route from '@ember/routing/route';
import { withComponent } from 'somewhere';
import CoolComponent from '../components/cool-component';

@withComponent(CoolComponent)
export default class MyRoute extends Route {
  // ...
}

(I'm not at all suggesting this is an API we should adopt, just showing that it's trivial with this primitive in place. The point of this RFC is to add the primitive so we can build something nice on top of it after experimenting in the community!)

[pzuraq]

Echoing @chriskrycho's concerns, the point of the setRouteComponent API is to enable experimentation without having a strong opinion on the final authoring format. Ideally, users should not use it directly, as we may over time find better ways to support this functionality.

For instance, setComponentTemplate, precompileTemplate, and templateOnlyComponent are primitive APIs that we built to enable experimentation and basic functionality, and they work really well for this! However, they are very verbose, and we are currently working on a better compile/runtime target for the combination of using these together that will have a better overall output. We won't be able to do that optimization for users who directly used these APIs, but that's fine, because for the most part they were only used by early-adopters and in experiments, and the APIs will still be stable, just not optimal.

In the same way, we really don't want to focus on adding high level API features here. If the function happened to work as a decorator without any additional modifications, then I would be ok with saying that's supported. However, it would require us to actually support two different calling conventions, which is extra complexity, with the only benefit being that it's slightly nicer to use directly, which is a non-goal, so I don't think we should support it.

[rwjblue]

A few concerns on first review:

• Why would we only pass @controller if you use this API? It seems reasonable to always pass @controller. For example, it's a really convenient transition path for existing route templates to move all this. references to @controller and then move the template to be a template only component and 💥 you are "in the new world".
• The RFC should describe the semantics of lifecycle of the component. Saying that it is the same as invoking a component from the route template is fine, but then we should 1) say that explicitly and 2) provide some "non-normative" summary of the semantics.
• Implementation wise, I'd prefer to make the @controller a lazily reified value. That gives us a possible path away from eagerly constructing controllers for routes that do not use query params (eventually).

[pzuraq]

Updated to address your concerns @rwjblue, along with a few other changes!

[knownasilya]

Beautiful! 🎉

[SolPier]

Will there be an issue with the {{outlet}} helper ?

[sly7-7]

app/templates/loading.hbs instead ?

[chadhietala]

For historical reasons I understand why @controller is conceptually being passed but have you considered decoupling further? For instance we could introduce the following:

// strawman example 
import Route, { setRouteComponent, setComponentController } from '@ember/routing/route';
import Controller from '@ember/controller';

class MyRoute extends Route {}
class Ember2009 extends Controller {}

const RouteComponent = hbs`Hello, world!`;
setRouteComponent(RouteComponent, MyRoute);
setComponentController(Ember2009, RouteComponent); // Validates its actually associated with a Route

This decouples the associated route even further away from controller references. If controllers ever go away the migration path is:

1. Find all setComponentController references
2. Determine what, if anything, in the controller needs to be singleton state
3. Migrate that to a service
4. Inject service into component
5. Move actions out of controller and into the component
6. Delete all setComponentController references
7. Delete all controllers

Finding all @controller references and doing steps 2 - 5 and 7 is also possible, I'm just wondering if we want to be more explicit about the association here.

[scottmessinger]

I really like @chadhietala proposal. The implicit passing of @controller was the most unexpected piece of this proposal. It didn't feel necessary to make the proposal work and felt like it unnecessarily coupled a historical concept to a new proposal. Unless there is a need to automatically add @controller, I agree with @chadhietala that separating the controller part is a good idea.

[pzuraq]

We discussed this a bit on the core team call on last week, and I believe the consensus so far was to keep @controller. The reasons being:

1. It provides functionality that is not currently possible to achieve in any other possible way.
2. It is much simpler to implement immediately if we do not need to potentially change the way that controllers are instantiated, created, or passed around.
3. Even though it is passed as an argument, there is no requirement to use it. Users can avoid using it and write completely future-proof, controller-less code without it.
4. We can pretty easily deprecate it, and even make instantiating the controller lazy so that it is only done upon first usage.

From a pragmatic perspective, keeping @controller is much easier to implement and doesn't really hamper anyone's abilities to move forward in the meantime. This will likely change anyways when we continue the "glimmerification" of the router, figuring out basic route primitives like route managers that will allow us to specify the arguments directly, rather than having them be locked to @model and @controller.

[pzuraq]

@SolPier

Will there be an issue with the {{outlet}} helper ?

I don't believe there will, the {{outlet}} helper can already be used within components today I believe, it's a special helper but it does not need to be used specifically in a route template.

[mehulkar]

Exciting stuff!

• Can we add a note on how setComponentTemplate would affect route sub and super classes?
• Greenfield apps that use this feature exclusively would still incur the penalty of all the other route/template loading features in their vendor bundle. I realize that it's not a goal to do anything about that in this RFC, but would it be possible to get a sense of what this penalty is for such a greenfield app? In other words, if Ember could shake out all code related to loading route templates, how much smaller would the vendor bundle be? I imagine it would be much more exciting if we could connect this to some tangible user-facing benefit, rather than just better DX.

[luxzeitlos]

I just wrote essentially a polyfill for this: https://github.com/luxferresum/experimental-set-route-component
This is also compatible with ember-template-imports, so this works:

import Route from '@ember/routing/route';
import { setRouteComponent } from 'experimental-set-route-component';

const RoutableComponentYeah = <template>
  We now have routable components! \o/
</template>

export default class FooRoute extends Route {}
setRouteComponent(RoutableComponentYeah, FooRoute);

[NullVoxPopuli]

Anything we want to change in this RFC I'd like to see this move forward 🙃 Excited to have a way to have private layout / route scoped components

[wagenet]

It looks like there's solid interest in this RFC. Let's see if we can get it moving forwards again.

[michaelrkn]

I could be misunderstanding, but based on feedback I got on Discord, it seems to me that things are going in a different direction and this isn't needed: https://wycats.github.io/polaris-sketchwork/routing.html

[chriskrycho]

The Framework team will need to get on the same page to make sure, but I think we need this as part of the path to that most likely. In particular, because we do not want to block the adoption of strict mode templates on having a brand new routing design fully designed and then also implemented and then also adopted!

[luxzeitlos]

Whats the status here?
@chriskrycho is IMHO right, we need this as a stepping stone. Its already polyfillable, so adoption can be fast. And moving to a new route design will benefit from it, when route templates are already normal components, and often no controller is around at all.

[wagenet]

It seems to me that it's unlikely that this will be the actual primitive we want long term. The existing polyfill allows for experimentation in this area so I think it may make sense to close this. A more ergonomic solution would be to allow for replacing the route template file with a gjs/gts file.

[ef4]

Yeah, the problem here is that the goal of a low-level primitive like setRouteComponent is that we make one when we're confident we can support it long-term, even when we're not yet sure of the long-term high-level API. That lets addons build on top of it with confidence, and lets us figure out the high-level API from real usage.

But in this case, I'm not sure we want to commit to supporting this API. It probably defeats route-based code-splitting, because it requires the entire component graph for a route to be loaded before you can assign that component to the route.

I agree we need an immediate-term solution for strict-mode adoption in routes. I just don't think this is the API for that. We need either a low-level API that we can support long-term, or a declarative solution where the implementation can be changed and/or codemodded reliably in the future. For example, I think we could probably ship an addon that lets you convert app/templates/*.hbs to app/templates/*.gjs, and that would be more future-safe than this, because what you're trying to do is more statically visible.

[bwbuchanan]

But in this case, I'm not sure we want to commit to supporting this API. It probably defeats route-based code-splitting, because it requires the entire component graph for a route to be loaded before you can assign that component to the route.

Does it?

I have an app built using 100% strict mode components (no app/templates directory at all) and use my own implementation of setRouteComponent() and route splitting seems to work. I see different chunks getting loaded depending on which route I hit.

import type Controller from '@ember/controller';
import { guidFor } from '@ember/object/internals';
import type Route from '@ember/routing/route';
import type { ComponentLike } from '@glint/template';
//@ts-expect-error No typedefs
import { precompileTemplate } from '@ember/template-compilation';
import E from 'ember';
import type { TemplateFactory } from 'ember-cli-htmlbars';

const Ember = E as typeof E & { TEMPLATES: Record<string, TemplateFactory> };

type RouteComponent<M> =
  | ComponentLike<{
      Args: { Named: { model: M; controller: Controller } };
      Element: unknown;
    }>
  | ComponentLike<{ Args: unknown; Element: unknown }>;

declare function precompileTemplate(
  template: string,
  options: { strict?: boolean; scope?: () => Record<string, unknown> },
): TemplateFactory;

export type RouteClass<M = unknown> = abstract new () => Route<M>;

export function setRouteComponent<M>(
  route: RouteClass<M>,
  component: RouteComponent<M>,
): void {
  setRouteTemplate(route, templateForRouteComponent<M>(component));
}

function templateForRouteComponent<M>(routeComponent: RouteComponent<M>): TemplateFactory {
  return precompileTemplate('<routeComponent @model={{@model}} @controller={{this}} />', {
    strict: true,
    scope: () => ({
      routeComponent,
    }),
  });
}

export function setRouteTemplate(route: RouteClass, template: TemplateFactory): void {
  const guid = guidFor(route);
  const templateName = `route-${guid}`;
  route.prototype.templateName = templateName;
  Ember.TEMPLATES[templateName] = template;
}

//
// Route Class Decorators
//

export function withComponent<M>(
  component: RouteComponent<M>,
): <R extends RouteClass<M>>(route: R) => R {
  return route => {
    setRouteComponent(route, component);
    return route;
  };
}

export function withTemplate(
  template: TemplateFactory,
): <R extends RouteClass>(route: R) => R {
  return route => {
    setRouteTemplate(route, template);
    return route;
  };
}

[bwbuchanan]

I've done some further testing on this and can't find any issues with route-based code splitting when the routes import the components. The split must be happening above the route level, i.e. the route itself isn't loaded until it is navigated to.

Thinking back, I should've known this already because I had to implement a workaround in test-helper to preload all routes, otherwise it was not possible to run route unit tests individually if code splitting was enabled for the development environment (below).

At minimum, it would be great to have a setRouteTemplate low-level primitive, so that this can be accomplished without resorting to hacks or private APIs. As in my example above, setRouteComponent is trivial to build on top of setRouteTemplate.

import { setApplication } from '@ember/test-helpers';
import Application from 'client-dashboard/app';
import config from 'client-dashboard/config/environment';
import { start } from 'ember-qunit';
import * as QUnit from 'qunit';
import { setup } from 'qunit-dom';

setup(QUnit.assert);

setApplication(Application.create(config.APP));

declare const window: Window & {
  _embroiderRouteBundles_?: { loaded: boolean; load(): Promise<void> }[];
};

if (window._embroiderRouteBundles_) {
  const bundles = window._embroiderRouteBundles_;

  Promise.all(bundles.filter(bundle => !bundle.loaded).map(bundle => bundle.load())).then(() =>
    start(),
  );
} else {
  start();
}

[ef4]

The split must be happening above the route level, i.e. the route itself isn't loaded until it is navigated to.

yes, exactly. That’s the behavior I don’t think we want to keep forever. It means you wait two round trips to the server, in series: one to load the Route and then another for the Route to load all the data.

It’s probably better to load the data and components in parallel.

[luxzeitlos]

Could it be an option to pass a function that resolves to a promise to setRouteComponent?

setRouteComponent(route, async () => import('components/some-component'))

[bwbuchanan]

Could it be an option to pass a function that resolves to a promise to setRouteComponent?

Some questions I don't know the answer to, but could probably be tested easily:

1. Is the import() of a literal string going to result in Embroider bundling that file directly into the route's chunk, or is it going to load it only at runtime?
2. If the latter, is Embroider going to figure out that it does need to include the file in a chunk?

If Embroider does do the nice thing here, then the only other thing that I think would be needed is to stall the completion of the afterModel hook until the import() promise has resolved, so that the component is available when the route's template gets rendered.

[ef4]

Could it be an option to pass a function that resolves to a promise to setRouteComponent?

Yes, that is one possibility.

Is the import() of a literal string going to result in Embroider bundling that file directly into the route's chunk, or is it going to load it only at runtime?

The short answer is, yes, in general Embroider lazily loads import(). The longer answer is: the exact details of Embroider's implementation shouldn't matter because the goal is to follow the ES module spec, and import() is how the spec defines on-demand loading of ES modules.

[ef4]

People who have been wanting this feature may want to check out the ember-route-template addon, which while not a formalized part of Ember's public API is built on stable public APIs and addresses the pain point that this low-level RFC was attempting to address.

I think this RFC was primarily intended to close that gap, and (intentionally) doesn't try to be a new low-level primitive for a reformed router, which is where we expect our efforts to do, so I'm suggesting we close this one.