Add setRouteComponent API

[pzuraq]

Rendered

[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.

[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 ?

[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.

[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 :upside_down_face: 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.