From 14ee387c7c259ce477ce6953cacf4e79af168054 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Thu, 2 Apr 2026 16:53:57 +0100 Subject: [PATCH 01/20] initial stab at the route manager and encapsulating classic route behaviour in a classic route manager --- package.json | 3 + packages/@ember/-internals/routing/index.ts | 13 ++ .../route-managers/classic-route-manager.ts | 177 ++++++++++++++++++ .../routing/route-managers/route-manager.ts | 81 ++++++++ .../routing/route-managers/utils.ts | 65 +++++++ packages/@ember/routing/route.ts | 13 +- packages/@ember/routing/router.ts | 33 ++++ packages/router_js/index.ts | 1 + packages/router_js/lib/route-info.ts | 94 +++++++++- packages/router_js/lib/router.ts | 51 ++++- packages/router_js/lib/transition-state.ts | 9 +- 11 files changed, 528 insertions(+), 12 deletions(-) create mode 100644 packages/@ember/-internals/routing/route-managers/classic-route-manager.ts create mode 100644 packages/@ember/-internals/routing/route-managers/route-manager.ts create mode 100644 packages/@ember/-internals/routing/route-managers/utils.ts diff --git a/package.json b/package.json index e59028634c6..fc01187546e 100644 --- a/package.json +++ b/package.json @@ -180,6 +180,9 @@ "@ember/-internals/metal/index.js": "ember-source/@ember/-internals/metal/index.js", "@ember/-internals/owner/index.js": "ember-source/@ember/-internals/owner/index.js", "@ember/-internals/routing/index.js": "ember-source/@ember/-internals/routing/index.js", + "@ember/-internals/routing/route-managers/classic-route-manager.js": "ember-source/@ember/-internals/routing/route-managers/classic-route-manager.js", + "@ember/-internals/routing/route-managers/route-manager.js": "ember-source/@ember/-internals/routing/route-managers/route-manager.js", + "@ember/-internals/routing/route-managers/utils.js": "ember-source/@ember/-internals/routing/route-managers/utils.js", "@ember/-internals/runtime/index.js": "ember-source/@ember/-internals/runtime/index.js", "@ember/-internals/runtime/lib/ext/rsvp.js": "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", "@ember/-internals/runtime/lib/mixins/-proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js", diff --git a/packages/@ember/-internals/routing/index.ts b/packages/@ember/-internals/routing/index.ts index a55d506a555..ad95753d3e2 100644 --- a/packages/@ember/-internals/routing/index.ts +++ b/packages/@ember/-internals/routing/index.ts @@ -4,3 +4,16 @@ export { generateControllerFactory, DSL as RouterDSL, } from '@ember/routing/-internals'; + +export { setRouteManager, getRouteManager } from './route-managers/utils'; +export { ClassicRouteManager } from './route-managers/classic-route-manager'; +export type { ClassicRouteBucket } from './route-managers/classic-route-manager'; +export type { + RouteManager, + RouteCapabilities, + NavigationState, + NavigationActions, + AsyncNavigationState, + CreateRouteArgs, +} from './route-managers/route-manager'; +export { routeCapabilities } from './route-managers/route-manager'; diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts new file mode 100644 index 00000000000..1176e36b0e1 --- /dev/null +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -0,0 +1,177 @@ +import { getOwner } from '@ember/-internals/owner'; +import { assert } from '@ember/debug'; +import { DEBUG } from '@glimmer/env'; +import { hasInternalComponentManager } from '@glimmer/manager'; +import type { Destroyable, TemplateFactory } from '@glimmer/interfaces'; +import type Route from '@ember/routing/route'; +import type { + RouteManager, + RouteCapabilities, + NavigationState, + NavigationActions, + AsyncNavigationState, + CreateRouteArgs, +} from './route-manager'; +import { routeCapabilities } from './route-manager'; +import type { RouteStateBucket } from './utils'; + +// --- Bucket --- + +export interface ClassicRouteBucket extends RouteStateBucket { + route: Route; + context: unknown; + invokable: object | undefined; +} + +// --- ClassicRouteManager --- + +/** + * The ClassicRouteManager wraps the classic Route lifecycle so that all + * routes flow through the RouteManager interface. Every method delegates + * to the Route instance stored in the bucket. + * + * Capabilities: classicInterop = true + */ +export class ClassicRouteManager implements RouteManager { + capabilities: RouteCapabilities = routeCapabilities('1.0', { classicInterop: true }); + + createRoute(routeInstance: object, args: CreateRouteArgs): ClassicRouteBucket { + let route = routeInstance as Route; + route._setRouteName(args.name); + + return { + route, + context: undefined, + invokable: undefined, + instance: route, + args, + }; + } + + getDestroyable(bucket: ClassicRouteBucket): Destroyable | null { + return bucket.route; + } + + // --- Enter lifecycle --- + + willEnter(_bucket: ClassicRouteBucket, _args: NavigationState & NavigationActions): void { + // No-op for classic routes + } + + enter( + bucket: ClassicRouteBucket, + _args: NavigationState & NavigationActions & AsyncNavigationState + ): Promise { + let route = bucket.route; + + // The classic resolve chain: beforeModel -> model -> afterModel + // These are called from the router_js resolve phase via resolveViaClassicHooks(). + // + // For the ClassicRouteManager, `enter()` is called from the manager-driven + // resolve path in route-info.ts. It runs the three classic hooks in sequence, + // checking for abort between each step. + // + // NOTE: In the current additive migration, the classic hooks are still called + // via resolveViaClassicHooks() in route-info.ts (the fallback path). Once all + // routes go through the manager, this method will be the sole entry point for + // the classic beforeModel/model/afterModel chain. + // + // For now, this returns the resolved model (context) and stores it on the bucket. + // The actual hook calls are handled by the resolveViaClassicHooks path since + // they need access to the transition and route-info internals. + + // Return the already-resolved context from the bucket (set by resolveViaClassicHooks) + return Promise.resolve(bucket.context); + } + + didEnter(bucket: ClassicRouteBucket, _args: NavigationState): void { + let route = bucket.route; + let context = bucket.context; + // Extract transition and enter flag from args (passed by router_js) + let transition = (_args as any).transition; + let enter = (_args as any).enter; + + // Classic enter: activate + trigger 'activate' (only on fresh enter, not updates) + if (enter) { + route.enter(transition); + } + + // Set the resolved context on the route object + route.context = context; + route.contextDidChange(); + + // Run setup (controller wiring, QP handling, rendering) + route.setup(context, transition); + } + + // --- Exit lifecycle --- + + willExit(bucket: ClassicRouteBucket, _args: NavigationState & NavigationActions): void { + let route = bucket.route; + // Extract isExiting and transition from args (passed by router_js) + let isExiting = (_args as any).isExiting ?? true; + let transition = (_args as any).transition; + // _internalReset handles resetting QP delegate on the controller + route._internalReset(isExiting, transition); + } + + exit(bucket: ClassicRouteBucket, _args: NavigationState): void { + let route = bucket.route; + let transition = (_args as any).transition; + delete route.context; + route.exit(transition); + } + + didExit(_bucket: ClassicRouteBucket, _args: NavigationState): void { + // No-op for classic routes + } + + // --- Template/Component lookup --- + + getInvokable(bucket: ClassicRouteBucket): Promise { + let route = bucket.route; + let owner = getOwner(route); + assert('Route is unexpectedly missing an owner', owner); + + let name = route.templateName || route.routeName; + let templateFactoryOrComponent = owner.lookup(`template:${name}`) as + | TemplateFactory + | object + | undefined; + + let template: object; + + if (templateFactoryOrComponent) { + if (hasInternalComponentManager(templateFactoryOrComponent)) { + // ComponentLike - pass through directly + template = templateFactoryOrComponent; + } else { + if (DEBUG && typeof templateFactoryOrComponent !== 'function') { + let label: string; + + try { + label = `\`${String(templateFactoryOrComponent)}\``; + } catch { + label = 'an unknown object'; + } + + assert( + `Failed to render the ${name} route, expected ` + + `\`template:${name}\` to resolve into ` + + `a component or a \`TemplateFactory\`, got: ${label}. ` + + `Most likely an improperly defined class or an invalid module export.` + ); + } + + // TemplateFactory -> Template + template = (templateFactoryOrComponent as TemplateFactory)(owner); + } + } else { + // Default {{outlet}} template + template = route._topLevelViewTemplate(owner); + } + + bucket.invokable = template; + return Promise.resolve(template); + } +} diff --git a/packages/@ember/-internals/routing/route-managers/route-manager.ts b/packages/@ember/-internals/routing/route-managers/route-manager.ts new file mode 100644 index 00000000000..4f4186e7d87 --- /dev/null +++ b/packages/@ember/-internals/routing/route-managers/route-manager.ts @@ -0,0 +1,81 @@ +import type { Capabilities, Destroyable } from '@glimmer/interfaces'; +import type { RouteStateBucket } from './utils'; +import type { RouteInfo } from '@ember/routing/-internals'; + +// --- Capabilities --- + +export interface RouteCapabilitiesVersions { + '1.0': { + classicInterop?: boolean; + }; +} + +export interface RouteCapabilities extends Capabilities { + classicInterop: boolean; +} + +export function routeCapabilities( + _managerAPI: Version, + options: RouteCapabilitiesVersions[Version] = {} +): RouteCapabilities { + let capabilities = { + classicInterop: Boolean(options.classicInterop), + }; + + Object.freeze(capabilities); + + // The Capabilities brand is structural (a unique symbol that only exists in the + // type system). We cast here the same way @glimmer/manager's buildCapabilities does. + return capabilities as RouteCapabilities; +} + +// --- Navigation args --- + +export interface NavigationState { + from?: RouteInfo; + to: RouteInfo; +} + +export interface NavigationActions { + /** Cancels the current navigation */ + cancel: () => void; +} + +export interface AsyncNavigationState { + /** Signal for the current navigation */ + signal: AbortSignal; + + /** Retrieve the ancestor promise for an ancestor route, to await async ancestor behaviour. */ + getAncestorPromise(routeInfo: RouteInfo): ReturnType['enter']>; +} + +export interface CreateRouteArgs { + name: string; +} + +// --- RouteManager interface --- + +export interface RouteManager { + capabilities: RouteCapabilities; + + createRoute(definition: object, args: CreateRouteArgs): R; + getDestroyable(bucket: R): Destroyable | null; + + willEnter(bucket: R, args: NavigationState & NavigationActions): void; + enter( + bucket: R, + args: NavigationState & NavigationActions & AsyncNavigationState + ): Promise; + didEnter(bucket: R, args: NavigationState): void; + + willExit(bucket: R, args: NavigationState & NavigationActions): void; + exit(bucket: R, args: NavigationState): void; + didExit(bucket: R, args: NavigationState): void; + + getInvokable(bucket: R): Promise; +} + +// --- Factory type --- + +export type Manager = RouteManager; +export type ManagerFactory = (owner: O) => D; diff --git a/packages/@ember/-internals/routing/route-managers/utils.ts b/packages/@ember/-internals/routing/route-managers/utils.ts new file mode 100644 index 00000000000..eb8204520b3 --- /dev/null +++ b/packages/@ember/-internals/routing/route-managers/utils.ts @@ -0,0 +1,65 @@ +import type { Owner } from '@glimmer/interfaces'; +import type { ManagerFactory, RouteManager } from './route-manager'; + +export interface RouteStateBucket { + instance?: unknown; + args: object; +} + +const ROUTE_MANAGERS = new WeakMap>(); + +/** + * There is also Reflect.getPrototypeOf, + * which errors when non-objects are passed. + * + * Since our conditional for figuring out whether to render primitives or not + * may contain non-object values, we don't want to throw errors when we call this. + */ +const getPrototypeOf = Object.getPrototypeOf; + +function setManager( + map: WeakMap, + manager: object, + obj: Def +): Def { + map.set(obj, manager); + return obj; +} + +function getManager>( + map: WeakMap, + obj: object +): M | undefined { + let pointer: object | null = obj; + while (pointer !== null) { + const manager = map.get(pointer); + + if (manager !== undefined) { + return manager; + } + + pointer = getPrototypeOf(pointer) as object | null; + } + + return undefined; +} + +export function setRouteManager( + factory: ManagerFactory>, + definition: T +): T { + let manager = factory(undefined); + setManager(ROUTE_MANAGERS, manager, definition); + + return definition; +} + +export function getRouteManager( + definition: T +): RouteManager | undefined { + if (!definition) { + return undefined; + } + + return getManager(ROUTE_MANAGERS, definition); +} diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 7b4a8f24eac..56fc95ea6ac 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -958,7 +958,9 @@ class Route extends EmberObject.extend(ActionHandler, Evented) this.setupController(controller, context, transition); - if (this._environment.options.shouldRender) { + // When a route manager is present, rendering is driven by the router via + // getInvokable() — skip the classic [RENDER]() path. + if (this._environment.options.shouldRender && !getRouteManager(this.constructor)) { this[RENDER](); } @@ -2230,3 +2232,12 @@ Route.reopen({ }); export default Route; + +// --- Wire the ClassicRouteManager on the Route base class --- +// Every Route subclass inherits this via the prototype-walking lookup in getRouteManager. + +import { setRouteManager } from '@ember/-internals/routing/route-managers/utils'; +import { ClassicRouteManager } from '@ember/-internals/routing/route-managers/classic-route-manager'; +import { getRouteManager } from '@ember/-internals/routing'; + +setRouteManager(() => new ClassicRouteManager(), Route); diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 114c398476e..957707b1a7f 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -1,5 +1,6 @@ import { privatize as P } from '@ember/-internals/container'; import type { BootEnvironment, OutletState, OutletView } from '@ember/-internals/glimmer'; +import { getOwner as getInternalOwner } from '@ember/-internals/owner'; import { computed, get, set } from '@ember/object'; import type { default as Owner, FactoryManager } from '@ember/owner'; import { getOwner } from '@ember/owner'; @@ -34,6 +35,7 @@ import { getRenderState, hasDefaultSerialize, } from '@ember/routing/route'; +import { getRouteManager } from '@ember/-internals/routing/route-managers/utils'; import type { InternalRouteInfo, ModelFor, @@ -359,6 +361,14 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { } route._setRouteName(routeName); + // Look up the route manager and create a bucket. + // This activates the manager-driven code paths in router_js. + let manager = getRouteManager(route.constructor); + if (manager) { + let bucket = manager.createRoute(route, { name: routeName }); + route.manager = manager as any; + route.bucket = bucket; + } if (engineInfo && !hasDefaultSerialize(route)) { throw new Error( @@ -394,6 +404,10 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { router.didTransition === defaultDidTransition ); router.didTransition(infos); + + // With route managers, [RENDER]() is skipped so we need to trigger + // _setOutlets after the transition completes to build the outlet tree. + scheduleOnce('afterRender', router, router._setOutlets); } // TODO: merge into routeWillChange @@ -607,6 +621,25 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { let route = routeInfo.route!; let render = getRenderState(route); + // When a route manager is present, [RENDER]() is skipped so + // getRenderState() returns undefined. Build the RenderState from + // the bucket and route info instead. + if (!render && route.manager) { + let bucket = route.bucket as { invokable?: object; context?: unknown } | undefined; + let template = routeInfo.invokable ?? bucket?.invokable; + if (template) { + let routeOwner = getInternalOwner(route); + assert('Route is unexpectedly missing an owner', routeOwner); + render = { + owner: routeOwner, + name: route.routeName, + controller: route.controller, + model: route.currentModel, + template, + }; + } + } + if (render) { let state: OutletState = { render, diff --git a/packages/router_js/index.ts b/packages/router_js/index.ts index 11f8f8bb438..4b866450c55 100644 --- a/packages/router_js/index.ts +++ b/packages/router_js/index.ts @@ -13,6 +13,7 @@ export { default as TransitionState, TransitionError } from './lib/transition-st export { default as InternalRouteInfo, type Route, + type RouteManagerLike, type RouteInfo, type RouteInfoWithAttributes, type ModelFor, diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index ecd4875b1a8..2f139a31ef1 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -15,7 +15,31 @@ export type IModel = {} & { export type ModelFor = T extends Route ? V : never; +/** + Minimal interface describing what router_js needs to know about a route manager. + The full `RouteManager` interface in Ember extends/satisfies this. + Defined locally to avoid a circular dependency from router_js into Ember. +*/ +export interface RouteManagerLike { + capabilities: { classicInterop: boolean }; + + willEnter(bucket: Bucket, args: unknown): void; + enter(bucket: Bucket, args: unknown): Promise; + didEnter(bucket: Bucket, args: unknown): void; + + willExit(bucket: Bucket, args: unknown): void; + exit(bucket: Bucket, args: unknown): void; + didExit(bucket: Bucket, args: unknown): void; + + getInvokable(bucket: Bucket): Promise; +} + export interface Route { + // --- Route Manager fields (optional during migration) --- + manager?: RouteManagerLike; + bucket?: unknown; + + // --- Classic Route fields (removed once all call sites migrate) --- inaccessibleByURL?: boolean; routeName: string; _internalName: string; @@ -224,6 +248,7 @@ export default class InternalRouteInfo { declare queryParams?: Dict; declare context?: ModelFor | PromiseLike> | undefined; isResolved = false; + invokable?: object = undefined; constructor(router: Router, name: string, paramNames: string[], route?: R) { this.name = name; @@ -248,7 +273,71 @@ export default class InternalRouteInfo { throwIfAborted(transition); return route; }) - .then(() => this.runBeforeModelHook(transition)) + .then((route) => { + // If the route has a manager, use the manager-driven resolve path. + // Otherwise fall through to the classic beforeModel → model → afterModel chain. + if (route.manager && route.bucket !== undefined) { + return this.resolveViaManager(route.manager, route.bucket, transition); + } + + return this.resolveViaClassicHooks(transition); + }); + } + + /** + Manager-driven resolve path. For managers with classicInterop, runs the + classic beforeModel → model → afterModel chain (since those hooks need + access to the transition and route-info internals) but also calls + getInvokable() concurrently. For non-classic managers, calls enter() and + getInvokable() concurrently. + */ + private resolveViaManager( + manager: RouteManagerLike, + bucket: unknown, + transition: InternalTransition + ): Promise> { + if (manager.capabilities.classicInterop) { + // Classic interop: run the classic hooks for model resolution, + // but also call getInvokable() concurrently for template lookup. + let classicResolve = this.runBeforeModelHook(transition) + .then(() => throwIfAborted(transition)) + .then(() => this.getModel(transition)) + .then((resolvedModel) => { + throwIfAborted(transition); + return resolvedModel; + }) + .then((resolvedModel) => this.runAfterModelHook(transition, resolvedModel)); + + return Promise.all([classicResolve, manager.getInvokable(bucket)]).then( + ([resolvedModel, invokable]) => { + throwIfAborted(transition); + this.invokable = invokable; + // Store the resolved model on the bucket so didEnter() can access it + (bucket as any).context = resolvedModel; + return this.becomeResolved(transition, resolvedModel); + } + ); + } + + // Non-classic manager: enter() is the sole async entry point + // TODO: Build proper NavigationState & NavigationActions args from the transition + let args = {}; + + return Promise.all([manager.enter(bucket, args), manager.getInvokable(bucket)]).then( + ([resolvedModel, invokable]) => { + throwIfAborted(transition); + this.invokable = invokable; + return this.becomeResolved(transition, resolvedModel as ModelFor); + } + ); + } + + /** + Classic resolve path. Chains beforeModel → model → afterModel → becomeResolved. + This path is used when no RouteManager is present on the route object. + */ + private resolveViaClassicHooks(transition: InternalTransition): Promise> { + return this.runBeforeModelHook(transition) .then(() => throwIfAborted(transition)) .then(() => this.getModel(transition)) .then((resolvedModel) => { @@ -289,6 +378,9 @@ export default class InternalRouteInfo { context ); + // Carry the invokable (from getInvokable() during resolve) to the resolved info + resolved.invokable = this.invokable; + if (cached !== undefined) { // SAFETY: This is potentially a bit risker, but for what we're doing, it should be ok. ROUTE_INFOS.set(resolved as unknown as InternalRouteInfo, cached); diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index 0dd1300ae8c..be716db05ab 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -434,15 +434,23 @@ export default abstract class Router { for (i = 0, l = partition.exited.length; i < l; i++) { route = partition.exited[i]!.route; - delete route!.context; if (route !== undefined) { - if (route._internalReset !== undefined) { - route._internalReset(true, transition); - } + if (route.manager && route.bucket !== undefined) { + let args = { transition, isExiting: true }; + route.manager.willExit(route.bucket, args); + route.manager.exit(route.bucket, args); + } else { + // No manager (migration period fallback) — call classic hooks directly + delete route.context; - if (route.exit !== undefined) { - route.exit(transition); + if (route._internalReset !== undefined) { + route._internalReset(true, transition); + } + + if (route.exit !== undefined) { + route.exit(transition); + } } } } @@ -455,7 +463,11 @@ export default abstract class Router { for (i = 0, l = partition.reset.length; i < l; i++) { route = partition.reset[i]!.route; if (route !== undefined) { - if (route._internalReset !== undefined) { + if (route.manager && route.bucket !== undefined) { + // Reset (not exiting) — willExit with isExiting: false semantics + let args = { transition, isExiting: false }; + route.manager.willExit(route.bucket, args); + } else if (route._internalReset !== undefined) { route._internalReset(false, transition); } } @@ -522,6 +534,18 @@ export default abstract class Router { let route = routeInfo.route, context = routeInfo.context; + function _routeEnteredOrUpdatedViaManager(route: R) { + // Pass transition and enter flag through args so the ClassicRouteManager + // can forward them to route.enter() and route.setup() + let args = { transition, enter }; + route.manager!.didEnter(route.bucket, args); + + throwIfAborted(transition); + + currentRouteInfos.push(routeInfo); + return route; + } + function _routeEnteredOrUpdated(route: R) { if (enter) { if (route.enter !== undefined) { @@ -549,9 +573,18 @@ export default abstract class Router { // If the route doesn't exist, it means we haven't resolved the route promise yet if (route === undefined) { - routeInfo.routePromise = routeInfo.routePromise.then(_routeEnteredOrUpdated); + routeInfo.routePromise = routeInfo.routePromise.then((route) => { + if (route.manager && route.bucket !== undefined) { + return _routeEnteredOrUpdatedViaManager(route); + } + return _routeEnteredOrUpdated(route); + }); } else { - _routeEnteredOrUpdated(route); + if (route.manager && route.bucket !== undefined) { + _routeEnteredOrUpdatedViaManager(route); + } else { + _routeEnteredOrUpdated(route); + } } return true; diff --git a/packages/router_js/lib/transition-state.ts b/packages/router_js/lib/transition-state.ts index e10c7c7361c..bc11a7883af 100644 --- a/packages/router_js/lib/transition-state.ts +++ b/packages/router_js/lib/transition-state.ts @@ -68,7 +68,14 @@ function proceed( // already-resolved route. let { route } = resolvedRouteInfo; if (route !== undefined) { - if (route.redirect) { + if (route.manager) { + // redirect is a classic-only hook, not part of the RouteManager interface. + // Non-classic managers redirect during enter() via the transition/cancel API. + if (route.manager.capabilities.classicInterop && route.redirect) { + route.redirect(resolvedRouteInfo.context, transition); + } + } else if (route.redirect) { + // No manager yet (migration period fallback) — call redirect directly route.redirect(resolvedRouteInfo.context, transition); } } From 2e36033bb41dcb8022f5410a35225ef06c5f9365 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Thu, 2 Apr 2026 17:57:33 +0100 Subject: [PATCH 02/20] tests now pass but we still have work to do to use getInvokable instead of [RENDER]() for classic route manager. --- .../route-managers/classic-route-manager.ts | 23 ++------ packages/@ember/routing/route.ts | 7 +-- packages/@ember/routing/router.ts | 7 +-- packages/router_js/index.ts | 1 - packages/router_js/lib/route-info.ts | 57 +++++++------------ packages/router_js/lib/router.ts | 10 ++++ 6 files changed, 38 insertions(+), 67 deletions(-) diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index 1176e36b0e1..2ba410a9958 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -62,25 +62,10 @@ export class ClassicRouteManager implements RouteManager { bucket: ClassicRouteBucket, _args: NavigationState & NavigationActions & AsyncNavigationState ): Promise { - let route = bucket.route; - - // The classic resolve chain: beforeModel -> model -> afterModel - // These are called from the router_js resolve phase via resolveViaClassicHooks(). - // - // For the ClassicRouteManager, `enter()` is called from the manager-driven - // resolve path in route-info.ts. It runs the three classic hooks in sequence, - // checking for abort between each step. - // - // NOTE: In the current additive migration, the classic hooks are still called - // via resolveViaClassicHooks() in route-info.ts (the fallback path). Once all - // routes go through the manager, this method will be the sole entry point for - // the classic beforeModel/model/afterModel chain. - // - // For now, this returns the resolved model (context) and stores it on the bucket. - // The actual hook calls are handled by the resolveViaClassicHooks path since - // they need access to the transition and route-info internals. - - // Return the already-resolved context from the bucket (set by resolveViaClassicHooks) + // Just returning the context here for now + // will want to handle the beforeModel -> model -> afterModel chain here once + // we have the getInvokable timing worked out, since those hooks need to run + // before we can resolve the invokable (template or component) for the route return Promise.resolve(bucket.context); } diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 56fc95ea6ac..7f00a49c22c 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -958,9 +958,9 @@ class Route extends EmberObject.extend(ActionHandler, Evented) this.setupController(controller, context, transition); - // When a route manager is present, rendering is driven by the router via - // getInvokable() — skip the classic [RENDER]() path. - if (this._environment.options.shouldRender && !getRouteManager(this.constructor)) { + // TODO: For non-classic managers, skip [RENDER]() and let the router + // handle rendering via getInvokable(). For now, always render. + if (this._environment.options.shouldRender) { this[RENDER](); } @@ -2238,6 +2238,5 @@ export default Route; import { setRouteManager } from '@ember/-internals/routing/route-managers/utils'; import { ClassicRouteManager } from '@ember/-internals/routing/route-managers/classic-route-manager'; -import { getRouteManager } from '@ember/-internals/routing'; setRouteManager(() => new ClassicRouteManager(), Route); diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 957707b1a7f..4b731c33a57 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -360,7 +360,6 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { } } - route._setRouteName(routeName); // Look up the route manager and create a bucket. // This activates the manager-driven code paths in router_js. let manager = getRouteManager(route.constructor); @@ -368,6 +367,8 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { let bucket = manager.createRoute(route, { name: routeName }); route.manager = manager as any; route.bucket = bucket; + } else { + route._setRouteName(routeName); } if (engineInfo && !hasDefaultSerialize(route)) { @@ -404,10 +405,6 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { router.didTransition === defaultDidTransition ); router.didTransition(infos); - - // With route managers, [RENDER]() is skipped so we need to trigger - // _setOutlets after the transition completes to build the outlet tree. - scheduleOnce('afterRender', router, router._setOutlets); } // TODO: merge into routeWillChange diff --git a/packages/router_js/index.ts b/packages/router_js/index.ts index 4b866450c55..11f8f8bb438 100644 --- a/packages/router_js/index.ts +++ b/packages/router_js/index.ts @@ -13,7 +13,6 @@ export { default as TransitionState, TransitionError } from './lib/transition-st export { default as InternalRouteInfo, type Route, - type RouteManagerLike, type RouteInfo, type RouteInfoWithAttributes, type ModelFor, diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 2f139a31ef1..7a4a50a60d3 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -8,6 +8,7 @@ import type InternalTransition from './transition'; import { isTransition, PARAMS_SYMBOL, prepareResult, QUERY_PARAMS_SYMBOL } from './transition'; import { isParam, isPromise, merge } from './utils'; import { throwIfAborted } from './transition-aborted-error'; +import type { RouteManager } from '@ember/-internals/routing'; export type IModel = {} & { id?: string | number; @@ -15,28 +16,9 @@ export type IModel = {} & { export type ModelFor = T extends Route ? V : never; -/** - Minimal interface describing what router_js needs to know about a route manager. - The full `RouteManager` interface in Ember extends/satisfies this. - Defined locally to avoid a circular dependency from router_js into Ember. -*/ -export interface RouteManagerLike { - capabilities: { classicInterop: boolean }; - - willEnter(bucket: Bucket, args: unknown): void; - enter(bucket: Bucket, args: unknown): Promise; - didEnter(bucket: Bucket, args: unknown): void; - - willExit(bucket: Bucket, args: unknown): void; - exit(bucket: Bucket, args: unknown): void; - didExit(bucket: Bucket, args: unknown): void; - - getInvokable(bucket: Bucket): Promise; -} - export interface Route { // --- Route Manager fields (optional during migration) --- - manager?: RouteManagerLike; + manager?: RouteManager; bucket?: unknown; // --- Classic Route fields (removed once all call sites migrate) --- @@ -274,49 +256,46 @@ export default class InternalRouteInfo { return route; }) .then((route) => { - // If the route has a manager, use the manager-driven resolve path. - // Otherwise fall through to the classic beforeModel → model → afterModel chain. if (route.manager && route.bucket !== undefined) { return this.resolveViaManager(route.manager, route.bucket, transition); } + // Fallback to the classic resolve path, makes the lazy-loaded + // route test pass for now until we can stabilize the manager-driven path and remove this return this.resolveViaClassicHooks(transition); }); } /** Manager-driven resolve path. For managers with classicInterop, runs the - classic beforeModel → model → afterModel chain (since those hooks need + classic beforeModel -> model -> afterModel chain (since those hooks need access to the transition and route-info internals) but also calls getInvokable() concurrently. For non-classic managers, calls enter() and getInvokable() concurrently. */ private resolveViaManager( - manager: RouteManagerLike, + manager: RouteManager, bucket: unknown, transition: InternalTransition ): Promise> { if (manager.capabilities.classicInterop) { - // Classic interop: run the classic hooks for model resolution, - // but also call getInvokable() concurrently for template lookup. - let classicResolve = this.runBeforeModelHook(transition) + // Classic interop: run the classic hooks chain with the exact same + // microtask structure as resolveViaClassicHooks (same number of .then() + // calls) to preserve transition timing. We want to use getInvokable here + // but was causing a bunch of microtask timing headaches, so we are letting + // [RENDER]() run for now + return this.runBeforeModelHook(transition) .then(() => throwIfAborted(transition)) .then(() => this.getModel(transition)) .then((resolvedModel) => { throwIfAborted(transition); return resolvedModel; }) - .then((resolvedModel) => this.runAfterModelHook(transition, resolvedModel)); - - return Promise.all([classicResolve, manager.getInvokable(bucket)]).then( - ([resolvedModel, invokable]) => { - throwIfAborted(transition); - this.invokable = invokable; - // Store the resolved model on the bucket so didEnter() can access it + .then((resolvedModel) => this.runAfterModelHook(transition, resolvedModel)) + .then((resolvedModel) => { (bucket as any).context = resolvedModel; return this.becomeResolved(transition, resolvedModel); - } - ); + }); } // Non-classic manager: enter() is the sole async entry point @@ -333,8 +312,10 @@ export default class InternalRouteInfo { } /** - Classic resolve path. Chains beforeModel → model → afterModel → becomeResolved. - This path is used when no RouteManager is present on the route object. + Classic resolve path. Chains beforeModel -> model -> afterModel -> becomeResolved. + This path is not used now, keeping it here just in case I need to fall back to it while + ironing out getInvokable() timing issues, but the plan is to remove this once the route manager + is proven. */ private resolveViaClassicHooks(transition: InternalTransition): Promise> { return this.runBeforeModelHook(transition) diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index be716db05ab..53d29ed4bf5 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -442,6 +442,7 @@ export default abstract class Router { route.manager.exit(route.bucket, args); } else { // No manager (migration period fallback) — call classic hooks directly + // I think some routes are created in tests that have no manager delete route.context; if (route._internalReset !== undefined) { @@ -535,6 +536,15 @@ export default abstract class Router { context = routeInfo.context; function _routeEnteredOrUpdatedViaManager(route: R) { + // Sync routeInfo.context -> bucket.context. This is necessary because + // intermediate transitions (error/loading substates) skip the resolve + // phase entirely — they go straight to setupContexts via + // intermediateTransitionTo -> applyToState(isIntermediate=true) -> + // becomeResolved() -> setupContexts. Since resolveViaManager() never + // runs, bucket.context is never set during resolve. + if (route.bucket !== undefined) { + (route.bucket as any).context = context; + } // Pass transition and enter flag through args so the ClassicRouteManager // can forward them to route.enter() and route.setup() let args = { transition, enter }; From bf4a1315a1a389244f6878b07b613e799a95c9cd Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Tue, 7 Apr 2026 11:59:35 +0100 Subject: [PATCH 03/20] move classic route hooks (beforeModel, model, afterModel) into class-route-manager --- .gitignore | 1 + .../route-managers/classic-route-manager.ts | 117 ++++++++++++++++-- packages/@ember/routing/route.ts | 6 - packages/router_js/lib/route-info.ts | 50 +++----- packages/router_js/lib/router.ts | 13 ++ 5 files changed, 136 insertions(+), 51 deletions(-) diff --git a/.gitignore b/.gitignore index 9bf32df156f..5c750072c1d 100644 --- a/.gitignore +++ b/.gitignore @@ -55,3 +55,4 @@ npm-debug.log # couple of the files. Once it is, we can switch this over to just ignoring # `types/stable` entirely. types/stable +_notes diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index 2ba410a9958..60b3d93b12d 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -14,6 +14,7 @@ import type { } from './route-manager'; import { routeCapabilities } from './route-manager'; import type { RouteStateBucket } from './utils'; +import { once } from '@ember/runloop'; // --- Bucket --- @@ -23,6 +24,18 @@ export interface ClassicRouteBucket extends RouteStateBucket { invokable: object | undefined; } +// --- Classic interop args --- + +/** + * Extra args provided to manager methods when classicInterop is enabled. + * These give the ClassicRouteManager access to router_js internals needed + * to replicate the classic hook behaviour. + */ +export interface ClassicInteropArgs { + transition: any; + routeInfo: any; +} + // --- ClassicRouteManager --- /** @@ -54,19 +67,100 @@ export class ClassicRouteManager implements RouteManager { // --- Enter lifecycle --- + private _runBeforeModel(route: Route, transition: any): Promise { + let result: unknown; + if (route.beforeModel !== undefined) { + result = route.beforeModel(transition); + } + + if (this._isTransition(result)) { + result = null; + } + + return Promise.resolve(result); + } + + private _getModel(route: Route, routeInfo: any, transition: any): Promise { + let fullParams = routeInfo.params || {}; + if (transition.__QPS__) { + fullParams = { ...fullParams, queryParams: transition.__QPS__ }; + } + + let result: unknown; + if (route.deserialize) { + result = route.deserialize(fullParams, transition); + } else if (route.model) { + result = route.model(fullParams, transition); + } + + if (this._isTransition(result)) { + result = null; + } + + return Promise.resolve(result).then((resolvedModel) => { + let name = routeInfo.name; + transition.resolvedModels = transition.resolvedModels || {}; + transition.resolvedModels[name] = resolvedModel; + return resolvedModel; + }); + } + + private _runAfterModel(route: Route, resolvedModel: unknown, transition: any): Promise { + let name = route.routeName || route._internalName; + transition.resolvedModels = transition.resolvedModels || {}; + transition.resolvedModels[name] = resolvedModel; + + let result: unknown; + if (route.afterModel !== undefined) { + result = route.afterModel(resolvedModel, transition); + } + + result = this._isTransition(result) ? null : result; + + return Promise.resolve(result).then(() => { + return transition.resolvedModels[name]; + }); + } + + private _isTransition(obj: unknown): boolean { + return typeof obj === 'object' && obj !== null && (obj as any).isTransition === true; + } + willEnter(_bucket: ClassicRouteBucket, _args: NavigationState & NavigationActions): void { // No-op for classic routes } enter( bucket: ClassicRouteBucket, - _args: NavigationState & NavigationActions & AsyncNavigationState + args: NavigationState & NavigationActions & AsyncNavigationState & ClassicInteropArgs ): Promise { - // Just returning the context here for now - // will want to handle the beforeModel -> model -> afterModel chain here once - // we have the getInvokable timing worked out, since those hooks need to run - // before we can resolve the invokable (template or component) for the route - return Promise.resolve(bucket.context); + let route = bucket.route; + let transition = args.transition; + let routeInfo = args.routeInfo; + + if (transition.trigger) { + transition.trigger(true, 'willResolveModel', transition, route); + } + + return this._runBeforeModel(route, transition) + .then(() => { + if (transition.isAborted) { + throw transition.error; + } + }) + .then(() => this._getModel(route, routeInfo, transition)) + .then((resolvedModel) => { + if (transition.isAborted) { + throw transition.error; + } + return resolvedModel; + }) + .then((resolvedModel) => this._runAfterModel(route, resolvedModel, transition)) + .then((resolvedModel) => { + (bucket as any).context = resolvedModel; + + return resolvedModel; + }); } didEnter(bucket: ClassicRouteBucket, _args: NavigationState): void { @@ -83,10 +177,15 @@ export class ClassicRouteManager implements RouteManager { // Set the resolved context on the route object route.context = context; - route.contextDidChange(); + if (route.contextDidChange !== undefined) { + route.contextDidChange(); + } + + if (route.setup !== undefined) { + route.setup(context, transition!); + } - // Run setup (controller wiring, QP handling, rendering) - route.setup(context, transition); + once(route._router, '_setOutlets'); } // --- Exit lifecycle --- diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 7f00a49c22c..3968aebe4f4 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -958,12 +958,6 @@ class Route extends EmberObject.extend(ActionHandler, Evented) this.setupController(controller, context, transition); - // TODO: For non-classic managers, skip [RENDER]() and let the router - // handle rendering via getInvokable(). For now, always render. - if (this._environment.options.shouldRender) { - this[RENDER](); - } - // Setup can cause changes to QPs which need to be propogated immediately in // some situations. Eventually, we should work on making these async somehow. flushAsyncObservers(false); diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 7a4a50a60d3..e2b4bfd3ef1 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -267,48 +267,26 @@ export default class InternalRouteInfo { } /** - Manager-driven resolve path. For managers with classicInterop, runs the - classic beforeModel -> model -> afterModel chain (since those hooks need - access to the transition and route-info internals) but also calls - getInvokable() concurrently. For non-classic managers, calls enter() and - getInvokable() concurrently. + Manager-driven resolve path. Calls enter() and getInvokable() concurrently. + For classicInterop managers, enter() runs the beforeModel → model → afterModel chain. + For non-classic managers, enter() is the sole async entry point. */ private resolveViaManager( manager: RouteManager, bucket: unknown, transition: InternalTransition ): Promise> { - if (manager.capabilities.classicInterop) { - // Classic interop: run the classic hooks chain with the exact same - // microtask structure as resolveViaClassicHooks (same number of .then() - // calls) to preserve transition timing. We want to use getInvokable here - // but was causing a bunch of microtask timing headaches, so we are letting - // [RENDER]() run for now - return this.runBeforeModelHook(transition) - .then(() => throwIfAborted(transition)) - .then(() => this.getModel(transition)) - .then((resolvedModel) => { - throwIfAborted(transition); - return resolvedModel; - }) - .then((resolvedModel) => this.runAfterModelHook(transition, resolvedModel)) - .then((resolvedModel) => { - (bucket as any).context = resolvedModel; - return this.becomeResolved(transition, resolvedModel); - }); - } - - // Non-classic manager: enter() is the sole async entry point - // TODO: Build proper NavigationState & NavigationActions args from the transition - let args = {}; - - return Promise.all([manager.enter(bucket, args), manager.getInvokable(bucket)]).then( - ([resolvedModel, invokable]) => { - throwIfAborted(transition); - this.invokable = invokable; - return this.becomeResolved(transition, resolvedModel as ModelFor); - } - ); + return Promise.all([ + manager.enter(bucket, { + transition, + routeInfo: this, + }), + manager.getInvokable(bucket), + ]).then(([resolvedModel, invokable]) => { + throwIfAborted(transition); + this.invokable = invokable; + return this.becomeResolved(transition, resolvedModel as ModelFor); + }); } /** diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index 53d29ed4bf5..b212f4c011f 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -545,6 +545,19 @@ export default abstract class Router { if (route.bucket !== undefined) { (route.bucket as any).context = context; } + // For intermediate transitions, getInvokable() was never called during + // resolve. Call it now so the template can be found for rendering. + // ClassicRouteManager.getInvokable() returns a synchronously resolved + // promise, so the invokable is available by the time didEnter runs setup() + // which schedules _setOutlets. + if (routeInfo.invokable === undefined && route.manager) { + route.manager.getInvokable(route.bucket).then((invokable: any) => { + routeInfo.invokable = invokable; + if (route.bucket) { + (route.bucket as any).invokable = invokable; + } + }); + } // Pass transition and enter flag through args so the ClassicRouteManager // can forward them to route.enter() and route.setup() let args = { transition, enter }; From de7b5c6058fc9a40376370433b8993ee5719c34e Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Tue, 7 Apr 2026 14:03:28 +0100 Subject: [PATCH 04/20] use RSVP for class-route-manager promises --- .../-internals/routing/route-managers/classic-route-manager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index 60b3d93b12d..2ffdd03fe79 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -15,6 +15,7 @@ import type { import { routeCapabilities } from './route-manager'; import type { RouteStateBucket } from './utils'; import { once } from '@ember/runloop'; +import { Promise } from 'rsvp'; // --- Bucket --- From 127485032ada44d931a72ad1b20d1c4c861ff990 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 8 Apr 2026 14:49:29 +0100 Subject: [PATCH 05/20] Remove classic-hook fallback paths now that all routes use a RouteManager - Exclude router_js unit tests from the main test suite via import.meta.glob negation (they test the now-deleted classic paths directly) - Drop resolveViaClassicHooks, runBeforeModelHook, runAfterModelHook from route-info.ts; resolve() now calls resolveViaManager unconditionally - Remove the no-manager else branches in setupContexts (exit and reset loops) in router_js/router.ts - Remove the _routeEnteredOrUpdated classic inner function and its fallback call sites in routeEnteredOrUpdated - Drop the else branch in getRoute that called route._setRouteName directly (ClassicRouteManager.createRoute() handles that now) - Simplify _setOutlets to build RenderState from manager/bucket unconditionally, removing the getRenderState() fallback and the conditional break - Fix _getModel to delegate to routeInfo.getModel() for polymorphic dispatch (handles both URL-param and model-object transitions correctly) - Guard _setOutlets call on shouldRender !== false; add LOG_VIEW_LOOKUPS logging --- index.html | 9 +- .../route-managers/classic-route-manager.ts | 80 ++++++------ packages/@ember/routing/route.ts | 118 +----------------- packages/@ember/routing/router.ts | 80 +++++------- packages/router_js/lib/route-info.ts | 75 +---------- packages/router_js/lib/router.ts | 20 +-- 6 files changed, 86 insertions(+), 296 deletions(-) diff --git a/index.html b/index.html index 77ad682afac..70b646751d4 100644 --- a/index.html +++ b/index.html @@ -67,9 +67,12 @@ }); // tests in non-scoped packages - import.meta.glob('./packages/*/tests/**/*.{js,ts,gjs,gts}', { - eager: true, - }); + import.meta.glob( + ['./packages/*/tests/**/*.{js,ts,gjs,gts}', '!./packages/router_js/tests/**'], + { + eager: true, + } + ); // packages originally authored in glimmer-vm repo have a different test // naming convention diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index 2ffdd03fe79..9274c556d5a 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -1,5 +1,6 @@ import { getOwner } from '@ember/-internals/owner'; -import { assert } from '@ember/debug'; +import { assert, info } from '@ember/debug'; +import { get } from '@ember/-internals/metal'; import { DEBUG } from '@glimmer/env'; import { hasInternalComponentManager } from '@glimmer/manager'; import type { Destroyable, TemplateFactory } from '@glimmer/interfaces'; @@ -81,24 +82,12 @@ export class ClassicRouteManager implements RouteManager { return Promise.resolve(result); } - private _getModel(route: Route, routeInfo: any, transition: any): Promise { - let fullParams = routeInfo.params || {}; - if (transition.__QPS__) { - fullParams = { ...fullParams, queryParams: transition.__QPS__ }; - } - - let result: unknown; - if (route.deserialize) { - result = route.deserialize(fullParams, transition); - } else if (route.model) { - result = route.model(fullParams, transition); - } - - if (this._isTransition(result)) { - result = null; - } - - return Promise.resolve(result).then((resolvedModel) => { + private _getModel(routeInfo: any, transition: any): Promise { + // Delegate to routeInfo.getModel() for polymorphic dispatch: + // - UnresolvedRouteInfoByParam calls route.deserialize/model with URL params + // - UnresolvedRouteInfoByObject returns the pre-provided model object directly + // This is critical when transitionTo() is called with a model object rather than URL params. + return Promise.resolve(routeInfo.getModel(transition)).then((resolvedModel: unknown) => { let name = routeInfo.name; transition.resolvedModels = transition.resolvedModels || {}; transition.resolvedModels[name] = resolvedModel; @@ -143,25 +132,28 @@ export class ClassicRouteManager implements RouteManager { transition.trigger(true, 'willResolveModel', transition, route); } - return this._runBeforeModel(route, transition) - .then(() => { - if (transition.isAborted) { - throw transition.error; - } - }) - .then(() => this._getModel(route, routeInfo, transition)) - .then((resolvedModel) => { - if (transition.isAborted) { - throw transition.error; - } - return resolvedModel; - }) - .then((resolvedModel) => this._runAfterModel(route, resolvedModel, transition)) - .then((resolvedModel) => { - (bucket as any).context = resolvedModel; - - return resolvedModel; - }); + return ( + this._runBeforeModel(route, transition) + .then(() => { + if (transition.isAborted) { + throw transition.error; + } + }) + .then(() => this._getModel(routeInfo, transition)) + // _getModel uses routeInfo.getModel() for polymorphic dispatch + .then((resolvedModel) => { + if (transition.isAborted) { + throw transition.error; + } + return resolvedModel; + }) + .then((resolvedModel) => this._runAfterModel(route, resolvedModel, transition)) + .then((resolvedModel) => { + (bucket as any).context = resolvedModel; + + return resolvedModel; + }) + ); } didEnter(bucket: ClassicRouteBucket, _args: NavigationState): void { @@ -186,7 +178,9 @@ export class ClassicRouteManager implements RouteManager { route.setup(context, transition!); } - once(route._router, '_setOutlets'); + if (route._environment?.options?.shouldRender !== false) { + once(route._router, '_setOutlets'); + } } // --- Exit lifecycle --- @@ -252,6 +246,14 @@ export class ClassicRouteManager implements RouteManager { template = (templateFactoryOrComponent as TemplateFactory)(owner); } } else { + if (DEBUG) { + let LOG_VIEW_LOOKUPS = get(route._router, 'namespace.LOG_VIEW_LOOKUPS'); + if (LOG_VIEW_LOOKUPS) { + info(`Could not find "${name}" template. Nothing will be rendered`, { + fullName: `template:${name}`, + }); + } + } // Default {{outlet}} template template = route._topLevelViewTemplate(owner); } diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 3968aebe4f4..230db562974 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -17,14 +17,10 @@ import { isProxy, lookupDescriptor } from '@ember/-internals/utils'; import type { AnyFn } from '@ember/-internals/utility-types'; import Controller from '@ember/controller'; import type { ControllerQueryParamType } from '@ember/controller'; -import { assert, info, isTesting } from '@ember/debug'; +import { assert, isTesting } from '@ember/debug'; import EngineInstance from '@ember/engine/instance'; import { dependentKeyCompat } from '@ember/object/compat'; import { once } from '@ember/runloop'; -import { DEBUG } from '@glimmer/env'; -import { hasInternalComponentManager } from '@glimmer/manager'; -import type { RenderState } from '@ember/-internals/glimmer'; -import type { TemplateFactory } from '@glimmer/interfaces'; import type { InternalRouteInfo, Route as IRoute, Transition, TransitionState } from 'router_js'; import { PARAMS_SYMBOL, STATE_SYMBOL } from 'router_js'; import type { QueryParam, default as EmberRouter } from '@ember/routing/router'; @@ -60,9 +56,6 @@ type RouteTransitionState = TransitionState & { type MaybeParameters = T extends AnyFn ? Parameters : unknown[]; type MaybeReturnType = T extends AnyFn ? ReturnType : unknown; -const RENDER = Symbol('render'); -const RENDER_STATE = Symbol('render-state'); - /** @module @ember/routing/route */ @@ -793,7 +786,6 @@ class Route extends EmberObject.extend(ActionHandler, Evented) @method enter */ enter(transition: Transition) { - this[RENDER_STATE] = undefined; this.activate(transition); this.trigger('activate', transition); } @@ -1456,18 +1448,6 @@ class Route extends EmberObject.extend(ActionHandler, Evented) return route?.currentModel; } - [RENDER_STATE]: RenderState | undefined = undefined; - - /** - `this[RENDER]` is used to set up the rendering option for the outlet state. - @method this[RENDER] - @private - */ - [RENDER]() { - this[RENDER_STATE] = buildRenderState(this); - once(this._router, '_setOutlets'); - } - willDestroy() { this.teardownViews(); } @@ -1478,10 +1458,7 @@ class Route extends EmberObject.extend(ActionHandler, Evented) @method teardownViews */ teardownViews() { - if (this[RENDER_STATE]) { - this[RENDER_STATE] = undefined; - once(this._router, '_setOutlets'); - } + once(this._router, '_setOutlets'); } /** @@ -1782,97 +1759,6 @@ class Route extends EmberObject.extend(ActionHandler, Evented) >; } -export function getRenderState(route: Route): RenderState | undefined { - return route[RENDER_STATE]; -} - -function buildRenderState(route: Route): RenderState { - let owner = getOwner(route); - assert('Route is unexpectedly missing an owner', owner); - - let name = route.routeName; - - let controller = owner.lookup(`controller:${route.controllerName || name}`); - assert('Expected an instance of controller', controller instanceof Controller); - - let model = route.currentModel; - - let templateFactoryOrComponent = owner.lookup(`template:${route.templateName || name}`) as - | TemplateFactory - | object // This is meant to be a component - | undefined; - - // Now we support either a component or a template to be returned by this - // resolver call, but if it's a `TemplateFactory`, we need to instantiate - // it into a `Template`, since that's what `RenderState` wants. We can't - // easily change it, it's intimate API used by @ember/test-helpers and the - // like. We could compatibly allow `Template` | `TemplateFactory`, and that's - // what it used to do but we _just_ went through deprecations to get that - // removed. It's also not ideal since once you mix the two types, they are - // not exactly easy to tell apart. - // - // It may also be tempting to just normalize `Template` into `RouteTemplate` - // here, and we could. However, this is not the only entrypoint where this - // `RenderState` is made – @ember/test-helpers punches through an impressive - // amount of private API to set it directly, and this feature would also be - // useful for them. So, even if we had normalized here, we'd still have to - // check and do that again during render anyway. - let template: object; - - if (templateFactoryOrComponent) { - if (hasInternalComponentManager(templateFactoryOrComponent)) { - template = templateFactoryOrComponent; - } else { - if (DEBUG && typeof templateFactoryOrComponent !== 'function') { - let label: string; - - try { - label = `\`${String(templateFactoryOrComponent)}\``; - } catch { - label = 'an unknown object'; - } - - assert( - `Failed to render the ${name} route, expected ` + - `\`template:${route.templateName || name}\` to resolve into ` + - `a component or a \`TemplateFactory\`, got: ${label}. ` + - `Most likely an improperly defined class or an invalid module export.` - ); - } - - template = (templateFactoryOrComponent as TemplateFactory)(owner); - } - } else { - // default `{{outlet}}` - template = route._topLevelViewTemplate(owner); - } - - let render: RenderState = { - owner, - name, - controller, - model, - template, - }; - - if (DEBUG) { - let LOG_VIEW_LOOKUPS = get(route._router, 'namespace.LOG_VIEW_LOOKUPS'); - // This is covered by tests and the existing code was deliberately - // targeting the value prior to normalization, but is this message actually - // accurate? It seems like we will always default the `{{outlet}}` template - // so I'm not sure about "Nothing will be rendered?" (who consumes these - // logs anyway? as lookups happen more infrequently now I doubt this is all - // that useful) - if (LOG_VIEW_LOOKUPS && !templateFactoryOrComponent) { - info(`Could not find "${name}" template. Nothing will be rendered`, { - fullName: `template:${name}`, - }); - } - } - - return render; -} - export function getFullQueryParams(router: EmberRouter, state: RouteTransitionState) { if (state.fullQueryParams) { return state.fullQueryParams; diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 4b731c33a57..ddc59dc987c 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -32,7 +32,6 @@ import { type default as Route, defaultSerialize, getFullQueryParams, - getRenderState, hasDefaultSerialize, } from '@ember/routing/route'; import { getRouteManager } from '@ember/-internals/routing/route-managers/utils'; @@ -363,13 +362,9 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Look up the route manager and create a bucket. // This activates the manager-driven code paths in router_js. let manager = getRouteManager(route.constructor); - if (manager) { - let bucket = manager.createRoute(route, { name: routeName }); - route.manager = manager as any; - route.bucket = bucket; - } else { - route._setRouteName(routeName); - } + let bucket = manager.createRoute(route, { name: routeName }); + route.manager = manager as any; + route.bucket = bucket; if (engineInfo && !hasDefaultSerialize(route)) { throw new Error( @@ -615,51 +610,40 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { let parent: OutletState | null = null; for (let routeInfo of routeInfos) { - let route = routeInfo.route!; - let render = getRenderState(route); - - // When a route manager is present, [RENDER]() is skipped so - // getRenderState() returns undefined. Build the RenderState from - // the bucket and route info instead. - if (!render && route.manager) { - let bucket = route.bucket as { invokable?: object; context?: unknown } | undefined; - let template = routeInfo.invokable ?? bucket?.invokable; - if (template) { - let routeOwner = getInternalOwner(route); - assert('Route is unexpectedly missing an owner', routeOwner); - render = { - owner: routeOwner, - name: route.routeName, - controller: route.controller, - model: route.currentModel, - template, - }; - } - } + let route = routeInfo.route; + assert('RouteInfo should have route', route); - if (render) { - let state: OutletState = { - render, - outlets: { - main: undefined, - }, - }; + let bucket = route.bucket as { invokable?: object; context?: unknown } | undefined; + let template = routeInfo.invokable ?? bucket?.invokable; - if (parent) { - parent.outlets.main = state; - } else { - root = state; - } + assert('Expected route to have an invokable template or component', template !== undefined); + + let routeOwner = getInternalOwner(route); + + assert('Route is unexpectedly missing an owner', routeOwner); - parent = state; + let render = { + owner: routeOwner, + name: route.routeName, + controller: route.controller, + model: route.currentModel, + template, + }; + + let state: OutletState = { + render, + outlets: { + main: undefined, + }, + }; + + if (parent) { + parent.outlets.main = state; } else { - // It used to be that we would create a stub entry and keep traversing, - // but I don't think that is necessary anymore – if a parent route did - // not render, then the child routes have nowhere to render into these - // days. That wasn't always the case since in the past any route can - // render into any other route's outlets. - break; + root = state; } + + parent = state; } // when a transitionTo happens after the validation phase diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index e2b4bfd3ef1..06bd679e5ba 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -5,7 +5,7 @@ import type { SerializerFunc } from './router'; import type Router from './router'; import type { PublicTransition as Transition } from './transition'; import type InternalTransition from './transition'; -import { isTransition, PARAMS_SYMBOL, prepareResult, QUERY_PARAMS_SYMBOL } from './transition'; +import { isTransition, PARAMS_SYMBOL, QUERY_PARAMS_SYMBOL } from './transition'; import { isParam, isPromise, merge } from './utils'; import { throwIfAborted } from './transition-aborted-error'; import type { RouteManager } from '@ember/-internals/routing'; @@ -256,13 +256,7 @@ export default class InternalRouteInfo { return route; }) .then((route) => { - if (route.manager && route.bucket !== undefined) { - return this.resolveViaManager(route.manager, route.bucket, transition); - } - - // Fallback to the classic resolve path, makes the lazy-loaded - // route test pass for now until we can stabilize the manager-driven path and remove this - return this.resolveViaClassicHooks(transition); + return this.resolveViaManager(route.manager, route.bucket, transition); }); } @@ -289,24 +283,6 @@ export default class InternalRouteInfo { }); } - /** - Classic resolve path. Chains beforeModel -> model -> afterModel -> becomeResolved. - This path is not used now, keeping it here just in case I need to fall back to it while - ironing out getInvokable() timing issues, but the plan is to remove this once the route manager - is proven. - */ - private resolveViaClassicHooks(transition: InternalTransition): Promise> { - return this.runBeforeModelHook(transition) - .then(() => throwIfAborted(transition)) - .then(() => this.getModel(transition)) - .then((resolvedModel) => { - throwIfAborted(transition); - return resolvedModel; - }) - .then((resolvedModel) => this.runAfterModelHook(transition, resolvedModel)) - .then((resolvedModel) => this.becomeResolved(transition, resolvedModel)); - } - becomeResolved( transition: InternalTransition | null, resolvedContext: ModelFor | undefined @@ -406,53 +382,6 @@ export default class InternalRouteInfo { return (this.route = route); } - private runBeforeModelHook(transition: InternalTransition) { - if (transition.trigger) { - transition.trigger(true, 'willResolveModel', transition, this.route); - } - - let result; - if (this.route) { - if (this.route.beforeModel !== undefined) { - result = this.route.beforeModel(transition); - } - } - - if (isTransition(result)) { - result = null; - } - - return Promise.resolve(result); - } - - private runAfterModelHook( - transition: InternalTransition, - resolvedModel?: ModelFor | null - ): Promise> { - // Stash the resolved model on the payload. - // This makes it possible for users to swap out - // the resolved model in afterModel. - let name = this.name; - this.stashResolvedModel(transition, resolvedModel!); - - let result; - if (this.route !== undefined) { - if (this.route.afterModel !== undefined) { - result = this.route.afterModel(resolvedModel!, transition); - } - } - - result = prepareResult(result); - - return Promise.resolve(result).then(() => { - // Ignore the fulfilled value returned from afterModel. - // Return the value stashed in resolvedModels, which - // might have been swapped out in afterModel. - // SAFTEY: We expect this to be of type T, though typing it as such is challenging. - return transition.resolvedModels[name]! as unknown as ModelFor; - }); - } - private stashResolvedModel( transition: InternalTransition, resolvedModel: ModelFor | undefined diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index b212f4c011f..8e850b3a319 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -436,23 +436,9 @@ export default abstract class Router { route = partition.exited[i]!.route; if (route !== undefined) { - if (route.manager && route.bucket !== undefined) { - let args = { transition, isExiting: true }; - route.manager.willExit(route.bucket, args); - route.manager.exit(route.bucket, args); - } else { - // No manager (migration period fallback) — call classic hooks directly - // I think some routes are created in tests that have no manager - delete route.context; - - if (route._internalReset !== undefined) { - route._internalReset(true, transition); - } - - if (route.exit !== undefined) { - route.exit(transition); - } - } + let args = { transition, isExiting: true }; + route.manager.willExit(route.bucket, args); + route.manager.exit(route.bucket, args); } } From 5fef2dbaf464bea4ae02bd792e809876ac41d1a8 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 8 Apr 2026 16:34:16 +0100 Subject: [PATCH 06/20] Add null check for _router in teardownViews method --- packages/@ember/routing/route.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 230db562974..ff9ed39e390 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -1458,7 +1458,9 @@ class Route extends EmberObject.extend(ActionHandler, Evented) @method teardownViews */ teardownViews() { - once(this._router, '_setOutlets'); + if (this._router) { + once(this._router, '_setOutlets'); + } } /** From 6b3b4722dc265c442a85763924da07a7ad295396 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Thu, 9 Apr 2026 11:42:04 +0100 Subject: [PATCH 07/20] clean up some types and shapes of objects --- packages/@ember/-internals/routing/index.ts | 1 + .../route-managers/classic-route-manager.ts | 12 ++++++---- .../routing/route-managers/route-manager.ts | 9 +++++-- packages/@ember/routing/route.ts | 4 ++-- packages/@ember/routing/router.ts | 8 +++++-- packages/router_js/lib/route-info.ts | 24 ++++++++++++++----- packages/router_js/lib/transition.ts | 1 + packages/router_js/package.json | 6 ++--- pnpm-lock.yaml | 9 ++++--- 9 files changed, 52 insertions(+), 22 deletions(-) diff --git a/packages/@ember/-internals/routing/index.ts b/packages/@ember/-internals/routing/index.ts index ad95753d3e2..6c0e0ce9972 100644 --- a/packages/@ember/-internals/routing/index.ts +++ b/packages/@ember/-internals/routing/index.ts @@ -6,6 +6,7 @@ export { } from '@ember/routing/-internals'; export { setRouteManager, getRouteManager } from './route-managers/utils'; +export type { RouteStateBucket } from './route-managers/utils'; export { ClassicRouteManager } from './route-managers/classic-route-manager'; export type { ClassicRouteBucket } from './route-managers/classic-route-manager'; export type { diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index 9274c556d5a..b774eb22fb1 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -12,6 +12,7 @@ import type { NavigationActions, AsyncNavigationState, CreateRouteArgs, + NavigationStateWithTransition, } from './route-manager'; import { routeCapabilities } from './route-manager'; import type { RouteStateBucket } from './utils'; @@ -122,11 +123,11 @@ export class ClassicRouteManager implements RouteManager { enter( bucket: ClassicRouteBucket, - args: NavigationState & NavigationActions & AsyncNavigationState & ClassicInteropArgs + args: NavigationState & NavigationActions & AsyncNavigationState & NavigationStateWithTransition ): Promise { let route = bucket.route; let transition = args.transition; - let routeInfo = args.routeInfo; + let routeInfo = args.to; if (transition.trigger) { transition.trigger(true, 'willResolveModel', transition, route); @@ -165,7 +166,8 @@ export class ClassicRouteManager implements RouteManager { // Classic enter: activate + trigger 'activate' (only on fresh enter, not updates) if (enter) { - route.enter(transition); + route.activate(transition); + route.trigger('activate', transition); } // Set the resolved context on the route object @@ -198,7 +200,9 @@ export class ClassicRouteManager implements RouteManager { let route = bucket.route; let transition = (_args as any).transition; delete route.context; - route.exit(transition); + route.deactivate(transition); + route.trigger('deactivate', transition); + route.teardownViews(); } didExit(_bucket: ClassicRouteBucket, _args: NavigationState): void { diff --git a/packages/@ember/-internals/routing/route-managers/route-manager.ts b/packages/@ember/-internals/routing/route-managers/route-manager.ts index 4f4186e7d87..cdbcd3659c4 100644 --- a/packages/@ember/-internals/routing/route-managers/route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/route-manager.ts @@ -1,6 +1,7 @@ import type { Capabilities, Destroyable } from '@glimmer/interfaces'; import type { RouteStateBucket } from './utils'; import type { RouteInfo } from '@ember/routing/-internals'; +import type Transition from '../../../routing/transition'; // --- Capabilities --- @@ -43,12 +44,16 @@ export interface NavigationActions { export interface AsyncNavigationState { /** Signal for the current navigation */ - signal: AbortSignal; + signal?: AbortSignal; /** Retrieve the ancestor promise for an ancestor route, to await async ancestor behaviour. */ getAncestorPromise(routeInfo: RouteInfo): ReturnType['enter']>; } +export interface NavigationStateWithTransition extends NavigationState { + transition: Transition; +} + export interface CreateRouteArgs { name: string; } @@ -64,7 +69,7 @@ export interface RouteManager { willEnter(bucket: R, args: NavigationState & NavigationActions): void; enter( bucket: R, - args: NavigationState & NavigationActions & AsyncNavigationState + args: NavigationState & NavigationActions & AsyncNavigationState & NavigationStateWithTransition ): Promise; didEnter(bucket: R, args: NavigationState): void; diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index ff9ed39e390..4d3ad13de75 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -2118,7 +2118,7 @@ export default Route; // --- Wire the ClassicRouteManager on the Route base class --- // Every Route subclass inherits this via the prototype-walking lookup in getRouteManager. -import { setRouteManager } from '@ember/-internals/routing/route-managers/utils'; -import { ClassicRouteManager } from '@ember/-internals/routing/route-managers/classic-route-manager'; +import { setRouteManager } from '@ember/-internals/routing'; +import { ClassicRouteManager } from '@ember/-internals/routing'; setRouteManager(() => new ClassicRouteManager(), Route); diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index ddc59dc987c..1e288941aa4 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -34,7 +34,7 @@ import { getFullQueryParams, hasDefaultSerialize, } from '@ember/routing/route'; -import { getRouteManager } from '@ember/-internals/routing/route-managers/utils'; +import { getRouteManager } from '@ember/-internals/routing'; import type { InternalRouteInfo, ModelFor, @@ -362,8 +362,12 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Look up the route manager and create a bucket. // This activates the manager-driven code paths in router_js. let manager = getRouteManager(route.constructor); + assert(`Route ${fullRouteName} does not have a manager`, manager); + let bucket = manager.createRoute(route, { name: routeName }); - route.manager = manager as any; + route.manager = manager; + + assert('Route manager should return a bucket', bucket); route.bucket = bucket; if (engineInfo && !hasDefaultSerialize(route)) { diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 06bd679e5ba..87e47ac2c1a 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -8,7 +8,7 @@ import type InternalTransition from './transition'; import { isTransition, PARAMS_SYMBOL, QUERY_PARAMS_SYMBOL } from './transition'; import { isParam, isPromise, merge } from './utils'; import { throwIfAborted } from './transition-aborted-error'; -import type { RouteManager } from '@ember/-internals/routing'; +import type { RouteManager, RouteStateBucket } from '@ember/-internals/routing'; export type IModel = {} & { id?: string | number; @@ -18,8 +18,8 @@ export type ModelFor = T extends Route ? V : never; export interface Route { // --- Route Manager fields (optional during migration) --- - manager?: RouteManager; - bucket?: unknown; + manager: RouteManager; + bucket: RouteStateBucket; // --- Classic Route fields (removed once all call sites migrate) --- inaccessibleByURL?: boolean; @@ -266,14 +266,26 @@ export default class InternalRouteInfo { For non-classic managers, enter() is the sole async entry point. */ private resolveViaManager( - manager: RouteManager, - bucket: unknown, + manager: RouteManager, + bucket: RouteStateBucket, transition: InternalTransition ): Promise> { return Promise.all([ manager.enter(bucket, { transition, - routeInfo: this, + to: this as unknown as RouteInfo, // will need to rethink this + cancel() { + transition.abort(); + }, + signal: transition.isTransition ? transition.signal : undefined, + getAncestorPromise: (routeInfo) => { + let index = transition.routeInfos.indexOf(routeInfo as unknown as InternalRouteInfo); + if (index === -1) { + return Promise.reject(new Error('RouteInfo not found in transition')); + } + let ancestorRouteInfo = transition.routeInfos[index]; + return Promise.resolve(ancestorRouteInfo?.routePromise); + }, }), manager.getInvokable(bucket), ]).then(([resolvedModel, invokable]) => { diff --git a/packages/router_js/lib/transition.ts b/packages/router_js/lib/transition.ts index b4c0e26634c..18f48ccaaac 100644 --- a/packages/router_js/lib/transition.ts +++ b/packages/router_js/lib/transition.ts @@ -71,6 +71,7 @@ export default class Transition implements Partial> _visibleQueryParams: Dict = {}; isIntermediate = false; [REDIRECT_DESTINATION_SYMBOL]?: Transition; + signal?: AbortSignal; /** In non-production builds, this function will return the stack that this Transition was diff --git a/packages/router_js/package.json b/packages/router_js/package.json index 68628f0ac24..b5c1ad6541d 100644 --- a/packages/router_js/package.json +++ b/packages/router_js/package.json @@ -8,8 +8,7 @@ "exports": { ".": "./index.ts" }, - "scripts": { - }, + "scripts": {}, "dependencies": { "@glimmer/env": "workspace:*" }, @@ -20,7 +19,8 @@ "loader.js": "^4.7.0", "qunit": "^2.11.3", "route-recognizer": "^0.3.4", - "rsvp": "^4.8.5" + "rsvp": "^4.8.5", + "@ember/-internals": "workspace:*" }, "peerDependencies": { "route-recognizer": "^0.3.4", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 76c7702f212..48a91dd0289 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -155,7 +155,7 @@ importers: version: 2.1.0 ember-cli-dependency-checker: specifier: ^3.3.1 - version: 3.3.3(ember-cli@6.11.2(@babel/core@7.29.0)(@types/node@22.19.15)) + version: 3.3.3(ember-cli@6.11.2(@babel/core@7.29.0)(@types/node@22.19.15)(ejs@3.1.10)(handlebars@4.7.9)(underscore@1.13.8)) ember-cli-yuidoc: specifier: ^0.9.1 version: 0.9.1 @@ -2723,6 +2723,9 @@ importers: specifier: workspace:* version: link:../@glimmer/env devDependencies: + '@ember/-internals': + specifier: workspace:* + version: link:../@ember/-internals '@types/qunit': specifier: ^2.9.6 version: 2.19.13 @@ -2797,7 +2800,7 @@ importers: version: 3.0.0 ember-cli-dependency-checker: specifier: ^3.3.3 - version: 3.3.3(ember-cli@6.11.2(@babel/core@7.29.0)(@types/node@22.19.15)) + version: 3.3.3(ember-cli@6.11.2(@babel/core@7.29.0)(@types/node@22.19.15)(ejs@3.1.10)(handlebars@4.7.9)(underscore@1.13.8)) ember-cli-deprecation-workflow: specifier: ^3.4.0 version: 3.4.0(ember-source@) @@ -17512,7 +17515,7 @@ snapshots: transitivePeerDependencies: - supports-color - ember-cli-dependency-checker@3.3.3(ember-cli@6.11.2(@babel/core@7.29.0)(@types/node@22.19.15)): + ember-cli-dependency-checker@3.3.3(ember-cli@6.11.2(@babel/core@7.29.0)(@types/node@22.19.15)(ejs@3.1.10)(handlebars@4.7.9)(underscore@1.13.8)): dependencies: chalk: 2.4.2 ember-cli: 6.11.2(@babel/core@7.29.0)(@types/node@22.19.15)(ejs@3.1.10)(handlebars@4.7.9)(underscore@1.13.8) From 8689563a023866d0989302a323bfa8bc601c35cd Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Mon, 13 Apr 2026 11:31:21 +0100 Subject: [PATCH 08/20] clean up fallback path for `routeEnteredOrUpdated` --- packages/router_js/lib/router.ts | 36 ++------------------------------ 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index 8e850b3a319..97647e10c07 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -555,45 +555,13 @@ export default abstract class Router { return route; } - function _routeEnteredOrUpdated(route: R) { - if (enter) { - if (route.enter !== undefined) { - route.enter(transition!); - } - } - - throwIfAborted(transition); - - route.context = context as Awaited; - - if (route.contextDidChange !== undefined) { - route.contextDidChange(); - } - - if (route.setup !== undefined) { - route.setup(context!, transition!); - } - - throwIfAborted(transition); - - currentRouteInfos.push(routeInfo); - return route; - } - // If the route doesn't exist, it means we haven't resolved the route promise yet if (route === undefined) { routeInfo.routePromise = routeInfo.routePromise.then((route) => { - if (route.manager && route.bucket !== undefined) { - return _routeEnteredOrUpdatedViaManager(route); - } - return _routeEnteredOrUpdated(route); + return _routeEnteredOrUpdatedViaManager(route); }); } else { - if (route.manager && route.bucket !== undefined) { - _routeEnteredOrUpdatedViaManager(route); - } else { - _routeEnteredOrUpdated(route); - } + _routeEnteredOrUpdatedViaManager(route); } return true; From 00b42fd13c5bdedfb377a05dc7a2a9e3bddb8baf Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Tue, 14 Apr 2026 13:34:42 +0100 Subject: [PATCH 09/20] Handle setting up controller and model in the classic route manager. This make outlets and renderer unaware of controllers and models. The Route Manager now curries the invokable with @controller and @model arguments. Model is @tracked so it rerenders when it changes. --- .../glimmer/lib/component-managers/outlet.ts | 6 +- .../@ember/-internals/glimmer/lib/renderer.ts | 14 +-- .../-internals/glimmer/lib/syntax/outlet.ts | 53 ++--------- .../-internals/glimmer/lib/utils/outlet.ts | 15 +--- .../-internals/glimmer/lib/views/outlet.ts | 3 - .../application/debug-render-tree-test.ts | 2 +- packages/@ember/-internals/package.json | 4 + .../route-managers/classic-route-manager.ts | 89 ++++++++++++++----- packages/@ember/routing/route.ts | 4 +- packages/@ember/routing/router.ts | 4 +- packages/@glimmer/runtime/index.ts | 2 +- packages/router_js/lib/route-info.ts | 3 +- 12 files changed, 93 insertions(+), 106 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts b/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts index e39b60250a3..9c932797fa2 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts @@ -42,7 +42,6 @@ export interface OutletDefinitionState { ref: Reference; name: string; template: object; - controller: unknown; } const CAPABILITIES: InternalComponentCapabilities = { @@ -172,10 +171,7 @@ class OutletComponentManager const OUTLET_MANAGER = new OutletComponentManager(); -const OUTLET_COMPONENT_TEMPLATE = precompileTemplate( - '<@Component @controller={{@controller}} @model={{@model}} />', - { strictMode: true } -); +const OUTLET_COMPONENT_TEMPLATE = precompileTemplate('<@Component />', { strictMode: true }); export class OutletComponent implements ComponentDefinition< OutletDefinitionState, diff --git a/packages/@ember/-internals/glimmer/lib/renderer.ts b/packages/@ember/-internals/glimmer/lib/renderer.ts index cfc74e0e423..6ef56c4c5aa 100644 --- a/packages/@ember/-internals/glimmer/lib/renderer.ts +++ b/packages/@ember/-internals/glimmer/lib/renderer.ts @@ -861,7 +861,7 @@ export class Renderer extends BaseRenderer { // we can refactor this to do something more direct/less convoluted // and with less setup, but get it working first let outlet = createRootOutlet(view); - let { name, /* controller, */ template } = view.state; + let { name, template } = view.state; let named = dict(); @@ -870,18 +870,6 @@ export class Renderer extends BaseRenderer { '@Component' ); - // TODO: is this guaranteed to be undefined? It seems to be the - // case in the `OutletView` class. Investigate how much that class - // exists as an internal implementation detail only, or if it was - // used outside of core. As far as I can tell, test-helpers uses - // it but only for `setOutletState`. - // named['controller'] = createConstRef(controller, '@controller'); - // Update: at least according to the debug render tree tests, we - // appear to always expect this to be undefined. Not a definitive - // source by any means, but is useful evidence - named['controller'] = UNDEFINED_REFERENCE; - named['model'] = UNDEFINED_REFERENCE; - let args = createCapturedArgs(named, EMPTY_POSITIONAL); this._appendDefinition( diff --git a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts index c4f6909a579..287eaffaed7 100644 --- a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts @@ -8,15 +8,9 @@ import type { Template, } from '@glimmer/interfaces'; import type { Reference } from '@glimmer/reference'; -import { - childRefFromParts, - createComputeRef, - createConstRef, - createDebugAliasRef, - valueForRef, -} from '@glimmer/reference'; +import { createComputeRef, createConstRef, valueForRef } from '@glimmer/reference'; import type { CurriedValue } from '@glimmer/runtime'; -import { createCapturedArgs, curry, EMPTY_POSITIONAL } from '@glimmer/runtime'; +import { createCapturedArgs, curry, EMPTY_POSITIONAL, isCurriedValue } from '@glimmer/runtime'; import { dict } from '@glimmer/util'; import { hasInternalComponentManager } from '@glimmer/manager'; import { OutletComponent, type OutletDefinitionState } from '../component-managers/outlet'; @@ -88,16 +82,14 @@ export const outletHelper = internalHelper( let named = dict(); - // Here we either have a raw template that needs to be normalized, - // or a component that we can render as-is. `RouteTemplate` upgrades - // the template into a component so we can have a unified code path. - // We still store the original `template` value, because we rely on - // its identity for the stability check, and the `RouteTemplate` - // wrapper doesn't dedup for us. + // When the route manager provides the invokable, state.template is + // already a curried component with @controller and @model baked in. + // For backward compatibility (e.g. setOutletState from test helpers), + // raw templates and components are wrapped with makeRouteTemplate. let template = state.template; let component: object; - if (hasInternalComponentManager(template)) { + if (isCurriedValue(template) || hasInternalComponentManager(template)) { component = template; } else { if (DEBUG) { @@ -137,36 +129,8 @@ export const outletHelper = internalHelper( component = makeRouteTemplate(outletOwner, state.name, template as Template); } - // Component is stable for the lifetime of the outlet named['Component'] = createConstRef(component, '@Component'); - // Controller is stable for the lifetime of the outlet - named['controller'] = createConstRef(state.controller, '@controller'); - - // Create a ref for the model - let modelRef = childRefFromParts(outletRef, ['render', 'model']); - - // Store the value of the model - let model = valueForRef(modelRef); - - // Create a compute ref which we pass in as the `{{@model}}` reference - // for the outlet. This ref will update and return the value of the - // model _until_ the outlet itself changes. Once the outlet changes, - // dynamic scope also changes, and so the original model ref would not - // provide the correct updated value. So we stop updating and return - // the _last_ model value for that outlet. - named['model'] = createComputeRef(() => { - if (lastState === state) { - model = valueForRef(modelRef); - } - - return model; - }); - - if (DEBUG) { - named['model'] = createDebugAliasRef!('@model', named['model']); - } - let args = createCapturedArgs(named, EMPTY_POSITIONAL); // Package up everything @@ -204,7 +168,6 @@ function stateFor( ref, name: render.name, template, - controller: render.controller, }; } @@ -216,5 +179,5 @@ function isStable( return false; } - return state.template === lastState.template && state.controller === lastState.controller; + return state.template === lastState.template; } diff --git a/packages/@ember/-internals/glimmer/lib/utils/outlet.ts b/packages/@ember/-internals/glimmer/lib/utils/outlet.ts index 61cb1ead256..7299e52a96a 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/outlet.ts @@ -15,18 +15,9 @@ export interface RenderState { name: string; /** - * The controller (the self of the outlet component) - */ - controller: unknown; - - /** - * The model (the resolved value of the model hook) - */ - model: unknown; - - /** - * The route's template – this is either a Template or a component, and it - * gets normalized during the render process. + * The route's invokable – a component with @controller and @model + * already applied by the route manager. This is either a CurriedValue + * (for classic routes) or a raw component/template for backward compatibility. */ template: Template | object | undefined; } diff --git a/packages/@ember/-internals/glimmer/lib/views/outlet.ts b/packages/@ember/-internals/glimmer/lib/views/outlet.ts index 7cc9be26e73..3e9488e94a4 100644 --- a/packages/@ember/-internals/glimmer/lib/views/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/views/outlet.ts @@ -68,8 +68,6 @@ export default class OutletView { render: { owner: owner, name: TOP_LEVEL_NAME, - controller: undefined, - model: undefined, template, }, }; @@ -89,7 +87,6 @@ export default class OutletView { ref, name: TOP_LEVEL_NAME, template, - controller: undefined, }; } diff --git a/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts b/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts index b255af4502c..9d86e4f9378 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts @@ -1686,7 +1686,7 @@ if (ENV._DEBUG_RENDER_TREE) { this.outlet({ type: 'route-template', name: '-top-level', - args: { positional: [], named: { controller: undefined, model: undefined } }, + args: { positional: [], named: {} }, instance: undefined, template: outlet, bounds: this.elementBounds(this.element!), diff --git a/packages/@ember/-internals/package.json b/packages/@ember/-internals/package.json index cc947bbb6b5..1cfa293c5b8 100644 --- a/packages/@ember/-internals/package.json +++ b/packages/@ember/-internals/package.json @@ -12,10 +12,14 @@ "./error-handling": "./error-handling/index.ts", "./glimmer": "./glimmer/index.ts", "./glimmer/lib/components/internal": "./glimmer/lib/components/internal.ts", + "./glimmer/lib/component-managers/route-template": "./glimmer/lib/component-managers/route-template.ts", "./meta": "./meta/index.ts", "./metal": "./metal/index.ts", "./owner": "./owner/index.ts", "./routing": "./routing/index.ts", + "./routing/route-managers/utils": "./routing/route-managers/utils.ts", + "./routing/route-managers/classic-route-manager": "./routing/route-managers/classic-route-manager.ts", + "./routing/route-managers/route-manager": "./routing/route-managers/route-manager.ts", "./runtime": "./runtime/index.ts", "./string": "./string/index.ts", "./utility-types": "./utility-types/index.ts", diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index b774eb22fb1..7fb9aedfa5f 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -3,7 +3,13 @@ import { assert, info } from '@ember/debug'; import { get } from '@ember/-internals/metal'; import { DEBUG } from '@glimmer/env'; import { hasInternalComponentManager } from '@glimmer/manager'; -import type { Destroyable, TemplateFactory } from '@glimmer/interfaces'; +import type { CurriedComponent, Destroyable, Template, TemplateFactory } from '@glimmer/interfaces'; +import type { Reference } from '@glimmer/reference'; +import { createComputeRef } from '@glimmer/reference'; +import { createCapturedArgs, curry, EMPTY_POSITIONAL } from '@glimmer/runtime'; +import { dict } from '@glimmer/util'; +import { tracked } from '@glimmer/tracking'; +import { makeRouteTemplate } from '@ember/-internals/glimmer/lib/component-managers/route-template'; import type Route from '@ember/routing/route'; import type { RouteManager, @@ -21,10 +27,22 @@ import { Promise } from 'rsvp'; // --- Bucket --- -export interface ClassicRouteBucket extends RouteStateBucket { +export class ClassicRouteBucket implements RouteStateBucket { route: Route; - context: unknown; + @tracked context: unknown; + controller: unknown; invokable: object | undefined; + instance: object; + args: CreateRouteArgs; + + constructor(route: Route, args: CreateRouteArgs) { + this.route = route; + this.context = undefined; + this.controller = undefined; + this.invokable = undefined; + this.instance = route; + this.args = args; + } } // --- Classic interop args --- @@ -55,13 +73,7 @@ export class ClassicRouteManager implements RouteManager { let route = routeInstance as Route; route._setRouteName(args.name); - return { - route, - context: undefined, - invokable: undefined, - instance: route, - args, - }; + return new ClassicRouteBucket(route, args); } getDestroyable(bucket: ClassicRouteBucket): Destroyable | null { @@ -150,7 +162,7 @@ export class ClassicRouteManager implements RouteManager { }) .then((resolvedModel) => this._runAfterModel(route, resolvedModel, transition)) .then((resolvedModel) => { - (bucket as any).context = resolvedModel; + bucket.context = resolvedModel; return resolvedModel; }) @@ -180,6 +192,10 @@ export class ClassicRouteManager implements RouteManager { route.setup(context, transition!); } + // Sync the controller onto the bucket so that the + // the invokable (see getInvokable) picks it up. + bucket.controller = route.controller; + if (route._environment?.options?.shouldRender !== false) { once(route._router, '_setOutlets'); } @@ -212,6 +228,11 @@ export class ClassicRouteManager implements RouteManager { // --- Template/Component lookup --- getInvokable(bucket: ClassicRouteBucket): Promise { + // Return cached invokable if already built + if (bucket.invokable !== undefined) { + return Promise.resolve(bucket.invokable); + } + let route = bucket.route; let owner = getOwner(route); assert('Route is unexpectedly missing an owner', owner); @@ -222,12 +243,17 @@ export class ClassicRouteManager implements RouteManager { | object | undefined; - let template: object; + let component: object; + // Track whether the component is already a resolved definition (CurriedValue + // from makeRouteTemplate) vs a raw component class that needs VM resolution. + let isResolved = true; if (templateFactoryOrComponent) { if (hasInternalComponentManager(templateFactoryOrComponent)) { - // ComponentLike - pass through directly - template = templateFactoryOrComponent; + // ComponentLike - use directly, will be curried below. + // Not resolved yet — the VM needs to look up its definition. + component = templateFactoryOrComponent; + isResolved = false; } else { if (DEBUG && typeof templateFactoryOrComponent !== 'function') { let label: string; @@ -246,8 +272,9 @@ export class ClassicRouteManager implements RouteManager { ); } - // TemplateFactory -> Template - template = (templateFactoryOrComponent as TemplateFactory)(owner); + // TemplateFactory -> Template -> RouteTemplate (curried component with no args) + let template = (templateFactoryOrComponent as TemplateFactory)(owner); + component = makeRouteTemplate(owner, name, template as Template); } } else { if (DEBUG) { @@ -258,11 +285,33 @@ export class ClassicRouteManager implements RouteManager { }); } } - // Default {{outlet}} template - template = route._topLevelViewTemplate(owner); + // Default {{outlet}} template -> RouteTemplate + let template = route._topLevelViewTemplate(owner); + component = makeRouteTemplate(owner, name, template as Template); } - bucket.invokable = template; - return Promise.resolve(template); + // Create refs that read from the bucket. The bucket fields are populated + // later (context during enter(), controller during didEnter()), so these + // refs start as undefined and resolve lazily. + // + // Controller: a tag-free compute ref. It's read lazily (first read happens + // during rendering, after didEnter has set bucket.controller). + // + // Model: bucket.context is @tracked, so this compute ref auto-tracks it + // and re-evaluates when the model changes (e.g. navigating to the same + // route with different dynamic segments). + let named = dict(); + named['controller'] = createComputeRef(() => bucket.controller); + named['model'] = createComputeRef(() => bucket.context); + + let args = createCapturedArgs(named, EMPTY_POSITIONAL); + + // Curry @controller and @model onto the component so the invokable is + // self-contained. The outlet rendering pipeline no longer needs to know + // about model or controller. + let invokable = curry(0 as CurriedComponent, component, owner, args, isResolved); + + bucket.invokable = invokable; + return Promise.resolve(invokable); } } diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 4d3ad13de75..ff9ed39e390 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -2118,7 +2118,7 @@ export default Route; // --- Wire the ClassicRouteManager on the Route base class --- // Every Route subclass inherits this via the prototype-walking lookup in getRouteManager. -import { setRouteManager } from '@ember/-internals/routing'; -import { ClassicRouteManager } from '@ember/-internals/routing'; +import { setRouteManager } from '@ember/-internals/routing/route-managers/utils'; +import { ClassicRouteManager } from '@ember/-internals/routing/route-managers/classic-route-manager'; setRouteManager(() => new ClassicRouteManager(), Route); diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 1e288941aa4..b3a0da44a3d 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -34,7 +34,7 @@ import { getFullQueryParams, hasDefaultSerialize, } from '@ember/routing/route'; -import { getRouteManager } from '@ember/-internals/routing'; +import { getRouteManager } from '@ember/-internals/routing/route-managers/utils'; import type { InternalRouteInfo, ModelFor, @@ -629,8 +629,6 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { let render = { owner: routeOwner, name: route.routeName, - controller: route.controller, - model: route.currentModel, template, }; diff --git a/packages/@glimmer/runtime/index.ts b/packages/@glimmer/runtime/index.ts index 9ec4eb2b603..d076a24c2e0 100644 --- a/packages/@glimmer/runtime/index.ts +++ b/packages/@glimmer/runtime/index.ts @@ -17,7 +17,7 @@ export { templateOnlyComponent, TemplateOnlyComponentManager, } from './lib/component/template-only'; -export { CurriedValue, curry } from './lib/curried-value'; +export { CurriedValue, curry, isCurriedValue } from './lib/curried-value'; export { DOMChanges, DOMTreeConstruction, diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 87e47ac2c1a..04840882945 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -8,7 +8,8 @@ import type InternalTransition from './transition'; import { isTransition, PARAMS_SYMBOL, QUERY_PARAMS_SYMBOL } from './transition'; import { isParam, isPromise, merge } from './utils'; import { throwIfAborted } from './transition-aborted-error'; -import type { RouteManager, RouteStateBucket } from '@ember/-internals/routing'; +import type { RouteManager } from '@ember/-internals/routing/route-managers/route-manager'; +import type { RouteStateBucket } from '@ember/-internals/routing/route-managers/utils'; export type IModel = {} & { id?: string | number; From 188c4fe2e1ca916ed725ac36d3f94b94a6febee1 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Mon, 20 Apr 2026 13:39:40 +0100 Subject: [PATCH 10/20] don't use resolver to instantiate route classes --- .../-internals/container/lib/container.ts | 20 ++- .../route-managers/classic-route-manager.ts | 15 +- .../routing/route-managers/utils.ts | 51 +++++-- packages/@ember/routing/index.ts | 8 + packages/@ember/routing/route.ts | 2 +- packages/@ember/routing/router.ts | 142 ++++++++++++------ 6 files changed, 171 insertions(+), 67 deletions(-) diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 0705d2ee2de..86165240ad3 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -6,12 +6,13 @@ import type { FactoryManager, FullName, } from '@ember/-internals/owner'; -import { setOwner } from '@ember/-internals/owner'; +import { getOwner, setOwner } from '@ember/-internals/owner'; import { dictionary } from '@ember/-internals/utils'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import type { DebugRegistry } from './registry'; import type Registry from './registry'; +import type EmberRouter from '@ember/routing/router'; interface LeakTracking { hasContainers(): boolean; @@ -282,6 +283,23 @@ function lookup( ): InternalFactory | object | undefined { let normalizedName = fullName; + if (normalizedName.startsWith('route:') && container.owner) { + let router = container.owner.lookup('router:main') as EmberRouter; + let routeName = normalizedName.split('route:')[1]; + if (routeName !== undefined) { + // For the main app container, pass the name directly — EmberRouter.getRoute + // resolves engine vs app routes internally via _engineInfoByRoute. + // For engine containers (owner !== router's owner), pass the engine owner + // explicitly so EmberRouter.getRoute uses it directly without a name lookup. + const isEngineContainer = getOwner(router) !== container.owner; + const engineOwner = isEngineContainer ? container.owner : undefined; + return router.getRoute(routeName, engineOwner) as + | InternalFactory + | object + | undefined; + } + } + if ( options.singleton === true || (options.singleton === undefined && isSingleton(container, fullName)) diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index 7fb9aedfa5f..b0317127ad0 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -1,4 +1,4 @@ -import { getOwner } from '@ember/-internals/owner'; +import { getOwner, setOwner } from '@ember/-internals/owner'; import { assert, info } from '@ember/debug'; import { get } from '@ember/-internals/metal'; import { DEBUG } from '@glimmer/env'; @@ -10,6 +10,7 @@ import { createCapturedArgs, curry, EMPTY_POSITIONAL } from '@glimmer/runtime'; import { dict } from '@glimmer/util'; import { tracked } from '@glimmer/tracking'; import { makeRouteTemplate } from '@ember/-internals/glimmer/lib/component-managers/route-template'; +import type Owner from '@ember/owner'; import type Route from '@ember/routing/route'; import type { RouteManager, @@ -69,8 +70,16 @@ export interface ClassicInteropArgs { export class ClassicRouteManager implements RouteManager { capabilities: RouteCapabilities = routeCapabilities('1.0', { classicInterop: true }); - createRoute(routeInstance: object, args: CreateRouteArgs): ClassicRouteBucket { - let route = routeInstance as Route; + #owner: Owner; + + constructor(owner: Owner) { + this.#owner = owner; + } + + createRoute(RouteClass: typeof Route, args: CreateRouteArgs): ClassicRouteBucket { + let props = {}; + setOwner(props, this.#owner); + let route = RouteClass.create(props); route._setRouteName(args.name); return new ClassicRouteBucket(route, args); diff --git a/packages/@ember/-internals/routing/route-managers/utils.ts b/packages/@ember/-internals/routing/route-managers/utils.ts index eb8204520b3..bf0b9bac230 100644 --- a/packages/@ember/-internals/routing/route-managers/utils.ts +++ b/packages/@ember/-internals/routing/route-managers/utils.ts @@ -1,12 +1,14 @@ -import type { Owner } from '@glimmer/interfaces'; import type { ManagerFactory, RouteManager } from './route-manager'; +import { DEBUG } from '@glimmer/env'; +import { debugAssert } from '@glimmer/global-context'; export interface RouteStateBucket { + invokable?: object; instance?: unknown; args: object; } -const ROUTE_MANAGERS = new WeakMap>(); +const ROUTE_MANAGERS = new WeakMap>>(); /** * There is also Reflect.getPrototypeOf, @@ -22,14 +24,31 @@ function setManager( manager: object, obj: Def ): Def { + if (DEBUG) { + debugAssert( + obj !== null && (typeof obj === 'object' || typeof obj === 'function'), + + `Attempted to set a manager on a non-object value. Managers can only be associated with objects or functions. Value was ${ + obj + }` + ); + + debugAssert( + !map.has(obj), + `Attempted to set the same type of manager multiple times on a value. You can only associate one manager of each type with a given value. Value was ${ + obj + }` + ); + } + map.set(obj, manager); return obj; } -function getManager>( +function getManager>>( map: WeakMap, obj: object -): M | undefined { +): ManagerFactory> | undefined { let pointer: object | null = obj; while (pointer !== null) { const manager = map.get(pointer); @@ -44,22 +63,30 @@ function getManager>( return undefined; } -export function setRouteManager( - factory: ManagerFactory>, +export function setRouteManager( + factory: ManagerFactory>, definition: T ): T { - let manager = factory(undefined); - setManager(ROUTE_MANAGERS, manager, definition); + setManager(ROUTE_MANAGERS, factory, definition); return definition; } export function getRouteManager( definition: T -): RouteManager | undefined { - if (!definition) { - return undefined; +): ManagerFactory> | undefined { + let factory = getManager(ROUTE_MANAGERS, definition); + + if (factory === undefined) { + if (DEBUG) { + debugAssert( + false, + `Attempted to load a route, but there wasn't a route manager associated with the definition. The definition was: ${ + definition + }` + ); + } } - return getManager(ROUTE_MANAGERS, definition); + return factory; } diff --git a/packages/@ember/routing/index.ts b/packages/@ember/routing/index.ts index 474d8b90c17..d79ffdc3951 100644 --- a/packages/@ember/routing/index.ts +++ b/packages/@ember/routing/index.ts @@ -1 +1,9 @@ export { LinkTo } from '@ember/-internals/glimmer'; +export { setRouteManager } from '@ember/-internals/routing/route-managers/utils'; +export { ClassicRouteManager } from '@ember/-internals/routing/route-managers/classic-route-manager'; +export { routeCapabilities } from '@ember/-internals/routing/route-managers/route-manager'; + +import type { RouteStateBucket } from '@ember/-internals/routing'; +import type { RouteManager as InternalRouteManager } from '@ember/-internals/routing/route-managers/route-manager'; + +export type RouteManager = InternalRouteManager; diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index ff9ed39e390..89e457338db 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -2121,4 +2121,4 @@ export default Route; import { setRouteManager } from '@ember/-internals/routing/route-managers/utils'; import { ClassicRouteManager } from '@ember/-internals/routing/route-managers/classic-route-manager'; -setRouteManager(() => new ClassicRouteManager(), Route); +setRouteManager((owner) => new ClassicRouteManager(owner), Route); diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index b3a0da44a3d..1e56c6e1ce6 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -34,7 +34,7 @@ import { getFullQueryParams, hasDefaultSerialize, } from '@ember/routing/route'; -import { getRouteManager } from '@ember/-internals/routing/route-managers/utils'; +import { getRouteManager, RouteStateBucket } from '@ember/-internals/routing/route-managers/utils'; import type { InternalRouteInfo, ModelFor, @@ -51,6 +51,7 @@ import type { QueryParams } from 'route-recognizer'; import type { AnyFn, MethodNamesOf, OmitFirst } from '@ember/-internals/utility-types'; import type { Template } from '@glimmer/interfaces'; import type ApplicationInstance from '@ember/application/instance'; +import { RouteManager } from '@ember/-internals/routing/route-managers/route-manager'; /** @module @ember/routing/router @@ -312,71 +313,112 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { this._routerService = routerService; } - _initRouterJs(): void { - let location = get(this, 'location') as EmberLocation; - let router = this; - const owner = getOwner(this); - assert('Router is unexpectedly missing an owner', owner); - let seen = Object.create(null); + // Both routes and manager instances are keyed first by owner so that engine routes + // (which share local names like 'application' with the main app) are isolated. + #routes = new WeakMap>(); + #routeManagerInstances = new WeakMap>>(); - class PrivateRouter extends Router { - getRoute(name: string): Route { - let routeName = name; - let routeOwner = owner; - let engineInfo = router._engineInfoByRoute[routeName]; + getRoute(name: string, engineOwner?: Owner) { + if (name === 'undefined') { + return; + } - if (engineInfo) { - let engineInstance = router._getEngineInstance(engineInfo); + assert('Name should not start with "route:"', !name.startsWith('route:')); - routeOwner = engineInstance; - routeName = engineInfo.localFullName; - } + // Resolve the owner and local route name. + // If engineOwner is provided (from the container bypass for an engine container), + // use it directly — the name is already the local route name within that engine. + // Otherwise, check _engineInfoByRoute to resolve engine routes by fully-qualified name. + let routeName = name; + let routeOwner: Owner; - let fullRouteName = `route:${routeName}` as const; - - assert('Route is unexpectedly missing an owner', routeOwner); + if (engineOwner) { + routeOwner = engineOwner; + } else { + routeOwner = getOwner(this)!; + assert('BUG: Missing owner', routeOwner); - let route = routeOwner.lookup(fullRouteName) as Route | undefined; + const engineInfo = this._engineInfoByRoute[routeName]; + if (engineInfo) { + routeOwner = this._getEngineInstance(engineInfo); + routeName = engineInfo.localFullName; + } + } - if (seen[name]) { - assert('seen routes should exist', route); - return route; - } + // Per-owner route cache. + let ownerRoutes = this.#routes.get(routeOwner); + if (!ownerRoutes) { + ownerRoutes = new Map(); + this.#routes.set(routeOwner, ownerRoutes); + } - seen[name] = true; + if (!ownerRoutes.has(routeName)) { + let fullRouteName = `route:${routeName}` as const; + let factoryManager = routeOwner.factoryFor(fullRouteName); - if (!route) { - // SAFETY: this is configured in `commonSetupRegistry` in the - // `@ember/application/lib` package. - let DefaultRoute: any = routeOwner.factoryFor('route:basic')!.class; - routeOwner.register(fullRouteName, class extends DefaultRoute {}); - route = routeOwner.lookup(fullRouteName) as Route; + // Auto-generate a default route if none is registered. + if (!factoryManager) { + let DefaultRoute: any = routeOwner.factoryFor('route:basic')!.class; + routeOwner.register(fullRouteName, class extends DefaultRoute {}); + factoryManager = routeOwner.factoryFor(fullRouteName); - if (DEBUG) { - if (router.namespace.LOG_ACTIVE_GENERATION) { - info(`generated -> ${fullRouteName}`, { fullName: fullRouteName }); - } + if (DEBUG) { + if (this.namespace.LOG_ACTIVE_GENERATION) { + info(`generated -> ${fullRouteName}`, { fullName: fullRouteName }); } } + } - // Look up the route manager and create a bucket. - // This activates the manager-driven code paths in router_js. - let manager = getRouteManager(route.constructor); - assert(`Route ${fullRouteName} does not have a manager`, manager); + assert('BUG: Missing factory for route', factoryManager); + let RouteClass = factoryManager.class; - let bucket = manager.createRoute(route, { name: routeName }); - route.manager = manager; + // Engine routes must not define a custom serialize method. + const isEngineRoute = routeOwner !== getOwner(this); + if (isEngineRoute && !hasDefaultSerialize(RouteClass.prototype as any)) { + throw new Error( + 'Defining a custom serialize method on an Engine route is not supported.' + ); + } - assert('Route manager should return a bucket', bucket); - route.bucket = bucket; + // Look up the manager factory function via prototype walk on the class. + let managerFactory = getRouteManager(RouteClass); + assert('Route manager needs to be defined', managerFactory !== undefined); + + // Instantiate the manager once per owner. Keyed first by owner so engine routes + // get their own manager instances, then by factory function so all subclasses + // sharing the same factory share one manager instance within that owner. + let ownerManagerInstances = this.#routeManagerInstances.get(routeOwner); + if (!ownerManagerInstances) { + ownerManagerInstances = new WeakMap(); + this.#routeManagerInstances.set(routeOwner, ownerManagerInstances); + } + if (!ownerManagerInstances.has(managerFactory)) { + ownerManagerInstances.set(managerFactory, managerFactory(routeOwner)); + } + let routeManagerInstance = ownerManagerInstances.get(managerFactory)!; - if (engineInfo && !hasDefaultSerialize(route)) { - throw new Error( - 'Defining a custom serialize method on an Engine route is not supported.' - ); - } + // Pass the concrete class to createRoute — the manager is responsible for instantiation. + let routeStateBucket = routeManagerInstance.createRoute(RouteClass, { name: routeName }); + + assert('Failed to create Route instance', routeStateBucket.instance); + (routeStateBucket.instance as any).bucket = routeStateBucket; + (routeStateBucket.instance as any).manager = routeManagerInstance; + ownerRoutes.set(routeName, routeStateBucket as RouteStateBucket); + } - return route; + return ownerRoutes.get(routeName)!.instance; + } + + _initRouterJs(): void { + let location = get(this, 'location') as EmberLocation; + let router = this; + const owner = getOwner(this); + assert('Router is unexpectedly missing an owner', owner); + class PrivateRouter extends Router { + getRoute(name: string): Route { + // All route instantiation and caching is owned by EmberRouter.getRoute, + // which resolves engine vs main-app owner internally. + return router.getRoute(name) as Route; } getSerializer(name: string) { From 55e3f59c96240e987fb1a5be5b370312f66c0626 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Mon, 20 Apr 2026 14:53:28 +0100 Subject: [PATCH 11/20] rename ownerManagerInstances to ownerRouteManagerInstances --- packages/@ember/routing/router.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 1e56c6e1ce6..bea89b9b705 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -375,9 +375,7 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Engine routes must not define a custom serialize method. const isEngineRoute = routeOwner !== getOwner(this); if (isEngineRoute && !hasDefaultSerialize(RouteClass.prototype as any)) { - throw new Error( - 'Defining a custom serialize method on an Engine route is not supported.' - ); + throw new Error('Defining a custom serialize method on an Engine route is not supported.'); } // Look up the manager factory function via prototype walk on the class. @@ -387,15 +385,15 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Instantiate the manager once per owner. Keyed first by owner so engine routes // get their own manager instances, then by factory function so all subclasses // sharing the same factory share one manager instance within that owner. - let ownerManagerInstances = this.#routeManagerInstances.get(routeOwner); - if (!ownerManagerInstances) { - ownerManagerInstances = new WeakMap(); - this.#routeManagerInstances.set(routeOwner, ownerManagerInstances); + let ownerRouteManagerInstances = this.#routeManagerInstances.get(routeOwner); + if (!ownerRouteManagerInstances) { + ownerRouteManagerInstances = new WeakMap(); + this.#routeManagerInstances.set(routeOwner, ownerRouteManagerInstances); } - if (!ownerManagerInstances.has(managerFactory)) { - ownerManagerInstances.set(managerFactory, managerFactory(routeOwner)); + if (!ownerRouteManagerInstances.has(managerFactory)) { + ownerRouteManagerInstances.set(managerFactory, managerFactory(routeOwner)); } - let routeManagerInstance = ownerManagerInstances.get(managerFactory)!; + let routeManagerInstance = ownerRouteManagerInstances.get(managerFactory)!; // Pass the concrete class to createRoute — the manager is responsible for instantiation. let routeStateBucket = routeManagerInstance.createRoute(RouteClass, { name: routeName }); From 83e20f961d699ce73d1fe9c8d4b2b881f050593c Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Tue, 21 Apr 2026 12:57:57 +0100 Subject: [PATCH 12/20] assert container has an owner when looking up routes and call manager.willEnter in InternalRouteInfo --- packages/@ember/-internals/container/lib/container.ts | 4 +++- packages/router_js/lib/route-info.ts | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 86165240ad3..4997f821103 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -283,7 +283,9 @@ function lookup( ): InternalFactory | object | undefined { let normalizedName = fullName; - if (normalizedName.startsWith('route:') && container.owner) { + if (normalizedName.startsWith('route:')) { + assert('Expected container to have an owner when looking up a route', container.owner); + let router = container.owner.lookup('router:main') as EmberRouter; let routeName = normalizedName.split('route:')[1]; if (routeName !== undefined) { diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 04840882945..1f8052548c9 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -271,6 +271,7 @@ export default class InternalRouteInfo { bucket: RouteStateBucket, transition: InternalTransition ): Promise> { + manager.willEnter(bucket, { transition }); return Promise.all([ manager.enter(bucket, { transition, From 6fbdc2c788aafe5ae017c47c5d35410cd1924677 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 22 Apr 2026 10:24:55 +0100 Subject: [PATCH 13/20] cleaner route bucket types --- .../routing/route-managers/classic-route-manager.ts | 8 +++++--- .../@ember/-internals/routing/route-managers/utils.ts | 3 ++- packages/@ember/routing/router.ts | 8 +++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index b0317127ad0..f164d751e1b 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -33,7 +33,6 @@ export class ClassicRouteBucket implements RouteStateBucket { @tracked context: unknown; controller: unknown; invokable: object | undefined; - instance: object; args: CreateRouteArgs; constructor(route: Route, args: CreateRouteArgs) { @@ -41,7 +40,6 @@ export class ClassicRouteBucket implements RouteStateBucket { this.context = undefined; this.controller = undefined; this.invokable = undefined; - this.instance = route; this.args = args; } } @@ -82,7 +80,11 @@ export class ClassicRouteManager implements RouteManager { let route = RouteClass.create(props); route._setRouteName(args.name); - return new ClassicRouteBucket(route, args); + const bucket = new ClassicRouteBucket(route, args); + route.bucket = bucket; + route.manager = this; + + return bucket; } getDestroyable(bucket: ClassicRouteBucket): Destroyable | null { diff --git a/packages/@ember/-internals/routing/route-managers/utils.ts b/packages/@ember/-internals/routing/route-managers/utils.ts index bf0b9bac230..361ee3b6fbb 100644 --- a/packages/@ember/-internals/routing/route-managers/utils.ts +++ b/packages/@ember/-internals/routing/route-managers/utils.ts @@ -4,7 +4,8 @@ import { debugAssert } from '@glimmer/global-context'; export interface RouteStateBucket { invokable?: object; - instance?: unknown; + route?: object & { manager: RouteManager }; + context?: unknown; args: object; } diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index bea89b9b705..969a29f868d 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -398,13 +398,11 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Pass the concrete class to createRoute — the manager is responsible for instantiation. let routeStateBucket = routeManagerInstance.createRoute(RouteClass, { name: routeName }); - assert('Failed to create Route instance', routeStateBucket.instance); - (routeStateBucket.instance as any).bucket = routeStateBucket; - (routeStateBucket.instance as any).manager = routeManagerInstance; - ownerRoutes.set(routeName, routeStateBucket as RouteStateBucket); + assert('Failed to create Route instance', routeStateBucket.route); + ownerRoutes.set(routeName, routeStateBucket); } - return ownerRoutes.get(routeName)!.instance; + return ownerRoutes.get(routeName)!.route; } _initRouterJs(): void { From 08cc3cbe28360a017adec5435e919260c0fc3424 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 22 Apr 2026 10:29:32 +0100 Subject: [PATCH 14/20] update engine route serialization check to use Object.getPrototypeOf --- packages/@ember/routing/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 969a29f868d..23403709ac7 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -374,7 +374,7 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Engine routes must not define a custom serialize method. const isEngineRoute = routeOwner !== getOwner(this); - if (isEngineRoute && !hasDefaultSerialize(RouteClass.prototype as any)) { + if (isEngineRoute && !hasDefaultSerialize(Object.getPrototypeOf(RouteClass))) { throw new Error('Defining a custom serialize method on an Engine route is not supported.'); } From 3934d4d421e643add055feb4ff70a50c11062ba8 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 22 Apr 2026 11:29:58 +0100 Subject: [PATCH 15/20] tidying up a bit --- .../routing/route-managers/route-manager.ts | 16 +++++++++++----- packages/@ember/routing/router.ts | 2 +- packages/router_js/lib/route-info.ts | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/@ember/-internals/routing/route-managers/route-manager.ts b/packages/@ember/-internals/routing/route-managers/route-manager.ts index cdbcd3659c4..3ac93d75fbe 100644 --- a/packages/@ember/-internals/routing/route-managers/route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/route-manager.ts @@ -66,16 +66,22 @@ export interface RouteManager { createRoute(definition: object, args: CreateRouteArgs): R; getDestroyable(bucket: R): Destroyable | null; - willEnter(bucket: R, args: NavigationState & NavigationActions): void; + willEnter( + bucket: R, + args: NavigationState & NavigationActions & NavigationStateWithTransition + ): void; enter( bucket: R, args: NavigationState & NavigationActions & AsyncNavigationState & NavigationStateWithTransition ): Promise; - didEnter(bucket: R, args: NavigationState): void; + didEnter(bucket: R, args: NavigationState & NavigationStateWithTransition): void; - willExit(bucket: R, args: NavigationState & NavigationActions): void; - exit(bucket: R, args: NavigationState): void; - didExit(bucket: R, args: NavigationState): void; + willExit( + bucket: R, + args: NavigationState & NavigationActions & NavigationStateWithTransition + ): void; + exit(bucket: R, args: NavigationState & NavigationStateWithTransition): void; + didExit(bucket: R, args: NavigationState & NavigationStateWithTransition): void; getInvokable(bucket: R): Promise; } diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 23403709ac7..7f57aa56d21 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -655,7 +655,7 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { let route = routeInfo.route; assert('RouteInfo should have route', route); - let bucket = route.bucket as { invokable?: object; context?: unknown } | undefined; + let bucket = route.bucket; let template = routeInfo.invokable ?? bucket?.invokable; assert('Expected route to have an invokable template or component', template !== undefined); diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 1f8052548c9..a9e1b102140 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -272,6 +272,7 @@ export default class InternalRouteInfo { transition: InternalTransition ): Promise> { manager.willEnter(bucket, { transition }); + return Promise.all([ manager.enter(bucket, { transition, From 7b3101b74f73a350eefd9fec0ddd37ca11dadd0a Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 29 Apr 2026 09:07:05 +0100 Subject: [PATCH 16/20] working on moving sub state, ancestor promises etc into route manager --- packages/@ember/-internals/glimmer/index.ts | 1 + .../route-managers/classic-route-manager.ts | 204 +++++++-- .../routing/route-managers/route-manager.ts | 7 +- .../routing/route-managers/utils.ts | 7 +- packages/@ember/routing/route.ts | 23 + packages/@ember/routing/router.ts | 392 ++++++++++++++---- packages/router_js/lib/route-info.ts | 123 ++++-- packages/router_js/lib/router.ts | 294 +++---------- packages/router_js/lib/transition-state.ts | 54 ++- 9 files changed, 702 insertions(+), 403 deletions(-) diff --git a/packages/@ember/-internals/glimmer/index.ts b/packages/@ember/-internals/glimmer/index.ts index 4cf8bd70367..060dcf4d504 100644 --- a/packages/@ember/-internals/glimmer/index.ts +++ b/packages/@ember/-internals/glimmer/index.ts @@ -498,3 +498,4 @@ export { } from './lib/utils/managers'; export { isSerializationFirstNode } from './lib/utils/serialization-first-node-helpers'; export { uniqueId } from './lib/helpers/unique-id'; +export { makeRouteTemplate } from './lib/component-managers/route-template'; diff --git a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts index f164d751e1b..ec3bef5c89e 100644 --- a/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic-route-manager.ts @@ -5,7 +5,7 @@ import { DEBUG } from '@glimmer/env'; import { hasInternalComponentManager } from '@glimmer/manager'; import type { CurriedComponent, Destroyable, Template, TemplateFactory } from '@glimmer/interfaces'; import type { Reference } from '@glimmer/reference'; -import { createComputeRef } from '@glimmer/reference'; +import { createComputeRef, createConstRef } from '@glimmer/reference'; import { createCapturedArgs, curry, EMPTY_POSITIONAL } from '@glimmer/runtime'; import { dict } from '@glimmer/util'; import { tracked } from '@glimmer/tracking'; @@ -23,23 +23,31 @@ import type { } from './route-manager'; import { routeCapabilities } from './route-manager'; import type { RouteStateBucket } from './utils'; -import { once } from '@ember/runloop'; +import { cancel, scheduleOnce } from '@ember/runloop'; import { Promise } from 'rsvp'; +import type { InternalRouteInfo } from 'router_js'; +import { STATE_SYMBOL } from 'router_js'; // --- Bucket --- export class ClassicRouteBucket implements RouteStateBucket { route: Route; - @tracked context: unknown; - controller: unknown; - invokable: object | undefined; args: CreateRouteArgs; + @tracked context: unknown = undefined; + controller: unknown = undefined; + invokable: object | undefined = undefined; + // Populated by the router immediately after calling manager.enter(), so that + // child routes can await the parent's data resolution via getAncestorPromise. + enterPromise: Promise | undefined = undefined; + // Timer handle for any pending loading substate transition scheduled during + // willEnter. Stored per-bucket so that concurrent routes each track their + // own timer and didEnter can cancel the right one without clobbering another + // route's timer. + loadingSubstateTimer: unknown = null; + constructor(route: Route, args: CreateRouteArgs) { this.route = route; - this.context = undefined; - this.controller = undefined; - this.invokable = undefined; this.args = args; } } @@ -140,8 +148,118 @@ export class ClassicRouteManager implements RouteManager { return typeof obj === 'object' && obj !== null && (obj as any).isTransition === true; } - willEnter(_bucket: ClassicRouteBucket, _args: NavigationState & NavigationActions): void { - // No-op for classic routes + willEnter( + bucket: ClassicRouteBucket, + args: NavigationState & NavigationActions & NavigationStateWithTransition + ): void { + const transition = args.transition; + + if (!transition.isActive) { + return; + } + + // Schedule the loading substate detection on the 'routerTransitions' queue. + // We do not look up the substate name here. Looking up factoryFor a missing + // template (e.g. on a fast initial visit where no loading template is added) + // poisons the registry's _failSet, so subsequent factoryFor calls return + // undefined even after the template is registered. Deferring the lookup until + // the timer fires mirrors the original _scheduleLoadingEvent timing and lets + // any addTemplate calls in the test setup land before we look anything up. + bucket.loadingSubstateTimer = scheduleOnce( + 'routerTransitions', + this, + this._enterLoadingSubstate, + bucket, + transition + ); + } + + private _enterLoadingSubstate(bucket: ClassicRouteBucket, transition: any): void { + if (!bucket.loadingSubstateTimer || !transition.isActive) { + return; + } + bucket.loadingSubstateTimer = null; + + // Walk up the route tree starting from this bucket's route to find a loading + // substate. Mirrors the original defaultActionHandlers.loading + forEachRouteAbove + // which received routeInfos sliced to the currently resolving route, not the + // full transition. + // - For the slow route itself, only check substate form (foo_loading). The + // state form (foo.loading) is its own child, technically below where we are. + // - For ancestor routes, check both state form (foo.loading) and substate + // form (foo_loading). + // - Stop at the pivot route, do not bubble higher. + const routeInfos = transition[STATE_SYMBOL]?.routeInfos ?? []; + const pivotHandler = transition.pivotHandler; + + let slowRouteInfo: InternalRouteInfo | undefined; + for (const candidate of routeInfos) { + if (candidate?.route === bucket.route) { + slowRouteInfo = candidate; + break; + } + } + const slowIndex = slowRouteInfo ? routeInfos.indexOf(slowRouteInfo) : -1; + const startIndex = slowIndex >= 0 ? slowIndex : routeInfos.length - 1; + + let loadingSubstateName = ''; + for (let i = startIndex; i >= 0; i--) { + const ancestorRouteInfo = routeInfos[i]; + const ancestorRoute = ancestorRouteInfo?.route; + if (!ancestorRoute) continue; + + const ancestorOwner = getOwner(ancestorRoute); + assert('Route is unexpectedly missing an owner', ancestorOwner); + + // Skip the state route check (foo.loading) for the slow route itself, + // since that would be a child route and below where we are. + if (ancestorRouteInfo !== slowRouteInfo) { + loadingSubstateName = this._findRouteStateName(ancestorRoute, ancestorOwner, 'loading'); + if (loadingSubstateName) break; + } + + loadingSubstateName = this._findRouteSubstateName(ancestorRoute, ancestorOwner, 'loading'); + if (loadingSubstateName) break; + + if (pivotHandler === ancestorRoute) break; + } + + if (!loadingSubstateName) { + return; + } + + bucket.route._router.intermediateTransitionTo(loadingSubstateName); + } + + // Checks whether a substate route of the form `routeName_state` exists. + private _findRouteSubstateName(route: Route, owner: Owner, state: string): string { + const { routeName, fullRouteName, _router: router } = route; + const substateName = `${routeName}_${state}`; + const substateNameFull = `${fullRouteName}_${state}`; + return this._routeHasBeenDefined(owner, router, substateName, substateNameFull) + ? substateNameFull + : ''; + } + + // Checks whether a state route of the form `routeName.state` exists. + private _findRouteStateName(route: Route, owner: Owner, state: string): string { + const { routeName, fullRouteName, _router: router } = route; + const stateName = routeName === 'application' ? state : `${routeName}.${state}`; + const stateNameFull = fullRouteName === 'application' ? state : `${fullRouteName}.${state}`; + return this._routeHasBeenDefined(owner, router, stateName, stateNameFull) ? stateNameFull : ''; + } + + // A route is defined if the router has the route AND the owner has a template or route class. + private _routeHasBeenDefined( + owner: Owner, + router: any, + localName: string, + fullName: string + ): boolean { + const routerHasRoute = router.hasRoute(fullName); + const ownerHasRoute = + owner.factoryFor(`template:${localName}`) || owner.factoryFor(`route:${localName}`); + return Boolean(routerHasRoute && ownerHasRoute); } enter( @@ -181,9 +299,15 @@ export class ClassicRouteManager implements RouteManager { } didEnter(bucket: ClassicRouteBucket, _args: NavigationState): void { + // Cancel the pending loading substate if enter() resolved before it fired. + if (bucket.loadingSubstateTimer) { + cancel(bucket.loadingSubstateTimer as any); + bucket.loadingSubstateTimer = null; + } + let route = bucket.route; let context = bucket.context; - // Extract transition and enter flag from args (passed by router_js) + // Extract transition and enter flag from args (passed by EmberRouter.onTransitionSettled) let transition = (_args as any).transition; let enter = (_args as any).enter; @@ -203,13 +327,14 @@ export class ClassicRouteManager implements RouteManager { route.setup(context, transition!); } - // Sync the controller onto the bucket so that the - // the invokable (see getInvokable) picks it up. + // Re-sync controller in case setup() did anything unusual. Normally the + // controller is the same instance _initController set during getInvokable. bucket.controller = route.controller; - if (route._environment?.options?.shouldRender !== false) { - once(route._router, '_setOutlets'); - } + // _setOutlets is no longer scheduled here. Rendering is driven by + // onRouteInvokableReady in EmberRouter, which fires as soon as getInvokable() + // resolves, before enter() finishes. By the time didEnter runs, the outlet is + // already rendering the route template with the reactive @model and @controller refs. } // --- Exit lifecycle --- @@ -239,9 +364,23 @@ export class ClassicRouteManager implements RouteManager { // --- Template/Component lookup --- getInvokable(bucket: ClassicRouteBucket): Promise { - // Return cached invokable if already built + // Build the invokable synchronously, then gate on enterPromise. Awaiting + // enterPromise ensures onRouteInvokableReady does not fire until data is + // loaded, whether this is a fresh entry or a re-entry where the model hook + // runs again. During the wait, the deferred scheduleOnce in willEnter fires + // and enters the loading substate. Once enter() resolves, the invokable is + // returned and the real route renders, replacing the loading substate. + const invokable = this._buildInvokable(bucket); + return (bucket.enterPromise || Promise.resolve()).then(() => invokable); + } + + // Synchronous invokable construction. Used by getInvokable above and directly + // by EmberRouter.onIntermediateTransition for substate routes which have no + // async work and need their didEnter (and therefore activate) to fire before + // routeWillChange events. + _buildInvokable(bucket: ClassicRouteBucket): object { if (bucket.invokable !== undefined) { - return Promise.resolve(bucket.invokable); + return bucket.invokable; } let route = bucket.route; @@ -262,7 +401,7 @@ export class ClassicRouteManager implements RouteManager { if (templateFactoryOrComponent) { if (hasInternalComponentManager(templateFactoryOrComponent)) { // ComponentLike - use directly, will be curried below. - // Not resolved yet — the VM needs to look up its definition. + // Not resolved yet, the VM needs to look up its definition. component = templateFactoryOrComponent; isResolved = false; } else { @@ -301,18 +440,22 @@ export class ClassicRouteManager implements RouteManager { component = makeRouteTemplate(owner, name, template as Template); } - // Create refs that read from the bucket. The bucket fields are populated - // later (context during enter(), controller during didEnter()), so these - // refs start as undefined and resolve lazily. - // - // Controller: a tag-free compute ref. It's read lazily (first read happens - // during rendering, after didEnter has set bucket.controller). - // - // Model: bucket.context is @tracked, so this compute ref auto-tracks it - // and re-evaluates when the model changes (e.g. navigating to the same + // Resolve the controller eagerly (looking up or generating it) so that + // bucket.controller is set before the route template renders. This is + // required because LinkTo (and other internal components) assert the + // caller reference is const, which forces us to curry @controller as a + // const ref. setup() called later in didEnter is idempotent and will skip + // the assignment when this.controller is already set. + let controller = (route as any)._initController(); + bucket.controller = controller; + + // Create curry args. Controller is a const ref because the controller + // is a singleton per owner and never changes for the route's lifetime. + // Model is a compute ref over bucket.context (which is @tracked) so the + // template re-renders when the model changes (e.g. navigating to the same // route with different dynamic segments). let named = dict(); - named['controller'] = createComputeRef(() => bucket.controller); + named['controller'] = createConstRef(controller, 'controller'); named['model'] = createComputeRef(() => bucket.context); let args = createCapturedArgs(named, EMPTY_POSITIONAL); @@ -323,6 +466,7 @@ export class ClassicRouteManager implements RouteManager { let invokable = curry(0 as CurriedComponent, component, owner, args, isResolved); bucket.invokable = invokable; - return Promise.resolve(invokable); + + return invokable; } } diff --git a/packages/@ember/-internals/routing/route-managers/route-manager.ts b/packages/@ember/-internals/routing/route-managers/route-manager.ts index 3ac93d75fbe..f8ddeef61ca 100644 --- a/packages/@ember/-internals/routing/route-managers/route-manager.ts +++ b/packages/@ember/-internals/routing/route-managers/route-manager.ts @@ -46,8 +46,11 @@ export interface AsyncNavigationState { /** Signal for the current navigation */ signal?: AbortSignal; - /** Retrieve the ancestor promise for an ancestor route, to await async ancestor behaviour. */ - getAncestorPromise(routeInfo: RouteInfo): ReturnType['enter']>; + /** + * Retrieve the ancestor promise for an ancestor route, to await async ancestor behaviour. + * If no routeInfo is passed, returns the immediate parent route's promise. + */ + getAncestorPromise(routeInfo?: RouteInfo): ReturnType['enter']>; } export interface NavigationStateWithTransition extends NavigationState { diff --git a/packages/@ember/-internals/routing/route-managers/utils.ts b/packages/@ember/-internals/routing/route-managers/utils.ts index 361ee3b6fbb..ea1ea3d06af 100644 --- a/packages/@ember/-internals/routing/route-managers/utils.ts +++ b/packages/@ember/-internals/routing/route-managers/utils.ts @@ -4,9 +4,12 @@ import { debugAssert } from '@glimmer/global-context'; export interface RouteStateBucket { invokable?: object; - route?: object & { manager: RouteManager }; - context?: unknown; + instance?: unknown; args: object; + // Set by the router synchronously after calling manager.enter(), before awaiting it. + // This makes the promise available to getAncestorPromise on child routes, which need + // to await their parent's data loading before starting their own. + enterPromise?: Promise; } const ROUTE_MANAGERS = new WeakMap>>(); diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 89e457338db..b16723ca4a9 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -780,6 +780,29 @@ class Route extends EmberObject.extend(ActionHandler, Evented) this.resetController(controller, isExiting, transition); } + /** + Resolve and assign this route's controller eagerly. Used by the route + manager during getInvokable so that the controller is available when the + route template renders, before didEnter/setup runs. Idempotent, calling + setup() later will see this.controller is set and skip the assignment. + + @private + @method _initController + */ + _initController() { + if (this.controller) { + return this.controller; + } + let controllerName = this.controllerName || this.routeName; + let definedController = this.controllerFor(controllerName, true); + let controller = definedController ?? this.generateController(controllerName); + let queryParams = get(this, '_qp') as Route['_qp']; + let propNames = queryParams.propertyNames; + addQueryParamsObservers(controller, propNames); + this.controller = controller; + return controller; + } + /** @private diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 7f57aa56d21..ff697e71c54 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -25,7 +25,8 @@ import { A as emberA } from '@ember/array'; import { typeOf } from '@ember/utils'; import Evented from '@ember/object/evented'; import { assert, info } from '@ember/debug'; -import { cancel, once, run, scheduleOnce } from '@ember/runloop'; +import { once, run } from '@ember/runloop'; +import { Promise as RSVPPromise } from 'rsvp'; import { DEBUG } from '@glimmer/env'; import { type QueryParamMeta, @@ -44,8 +45,8 @@ import type { TransitionError, TransitionState, } from 'router_js'; +import type { InternalTransition } from 'router_js'; import Router, { logAbort, STATE_SYMBOL } from 'router_js'; -import type { Timer } from 'backburner.js'; import EngineInstance from '@ember/engine/instance'; import type { QueryParams } from 'route-recognizer'; import type { AnyFn, MethodNamesOf, OmitFirst } from '@ember/-internals/utility-types'; @@ -60,8 +61,6 @@ import { RouteManager } from '@ember/-internals/routing/route-managers/route-man function defaultDidTransition(this: EmberRouter, infos: InternalRouteInfo[]) { updatePaths(this); - this._cancelSlowTransitionTimer(); - this.notifyPropertyChange('url'); this.set('currentState', this.targetState); @@ -192,8 +191,6 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { _engineInfoByRoute = Object.create(null); _routerService: RouterService; - _slowTransitionTimer: Timer | null = null; - private namespace: any; // Begin Evented @@ -374,7 +371,7 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Engine routes must not define a custom serialize method. const isEngineRoute = routeOwner !== getOwner(this); - if (isEngineRoute && !hasDefaultSerialize(Object.getPrototypeOf(RouteClass))) { + if (isEngineRoute && !hasDefaultSerialize((RouteClass as any).prototype)) { throw new Error('Defining a custom serialize method on an Engine route is not supported.'); } @@ -398,11 +395,11 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // Pass the concrete class to createRoute — the manager is responsible for instantiation. let routeStateBucket = routeManagerInstance.createRoute(RouteClass, { name: routeName }); - assert('Failed to create Route instance', routeStateBucket.route); + assert('Failed to create Route instance', (routeStateBucket as any).route); ownerRoutes.set(routeName, routeStateBucket); } - return ownerRoutes.get(routeName)!.route; + return (ownerRoutes.get(routeName)! as any).route; } _initRouterJs(): void { @@ -526,6 +523,28 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { this.updateURL(url); } } + + onRouteInvokableReady( + routeInfo: InternalRouteInfo, + transition: InternalTransition, + routeIndex: number + ): void { + router.onRouteInvokableReady(routeInfo, transition, routeIndex); + } + + onIntermediateTransition( + newState: TransitionState, + transition: InternalTransition + ): void { + router.onIntermediateTransition(newState, transition); + } + + onTransitionSettled( + transition: InternalTransition, + newState: TransitionState + ): Promise { + return router.onTransitionSettled(transition, newState); + } } let routerMicrolib = (this._routerMicrolib = new PrivateRouter()); @@ -635,6 +654,275 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { return true; } + onRouteInvokableReady( + routeInfo: InternalRouteInfo, + _transition: InternalTransition, + routeIndex: number + ): void { + // A route's getInvokable() has resolved. Write it into currentRouteInfos at + // routeIndex — the exact position of this route in the transition hierarchy. + // Do not truncate beyond this index: an intermediate transition may have placed + // a loading substate at a higher index that should remain until its own slot + // is overwritten by the real route at that position. + const currentRouteInfos = this._routerMicrolib.currentRouteInfos ?? []; + + currentRouteInfos[routeIndex] = routeInfo; + + this._routerMicrolib.currentRouteInfos = currentRouteInfos; + once(this as any, '_setOutlets'); + } + + onIntermediateTransition( + newState: TransitionState, + transition: InternalTransition + ): void { + // An intermediate transition (e.g. a loading or error substate entered via + // intermediateTransitionTo) has fired. We splice the intermediate routes into + // currentRouteInfos without disturbing the ancestor routes already rendering. + // For each newly entered route: build the invokable, then fire didEnter so + // classic lifecycle hooks (activate, setup, etc.) run just as they do for + // normal transitions. Without didEnter, activate() never fires on error/loading + // substate routes and transitions awaiting those hooks hang indefinitely. + // + // Substate routes are always synchronously available (no async engine boundary, + // no model hook), so we run getInvokable + didEnter synchronously. This matches + // the original setupContexts ordering where activate fires before + // routeWillChange (router_js calls onIntermediateTransition then routeWillChange + // synchronously in transitionByIntent). + const microlib = this._routerMicrolib; + const partition = microlib.partitionRoutes(microlib.state!, newState); + + const currentRouteInfos = [...partition.unchanged, ...partition.entered]; + + microlib.currentRouteInfos = currentRouteInfos; + // Update microlib.state to reflect the intermediate state. Without this, when + // the real transition completes and onTransitionSettled partitions against + // microlib.state, it would see ancestor routes (e.g. application) as still + // entering, firing didEnter and setupController twice for them. + microlib.oldState = microlib.state; + microlib.state = newState; + + for (const routeInfo of partition.entered) { + const route = routeInfo.route; + if (!route?.manager) { + continue; + } + + // Build the invokable synchronously. Substate routes don't have async work + // (no model hook, no engine loading), so getInvokable resolves on the same + // tick. We unwrap by treating it as a plain value rather than awaiting. + if (routeInfo.invokable === undefined && route.manager.capabilities.classicInterop) { + const invokable = (route.manager as any)._buildInvokable + ? (route.manager as any)._buildInvokable(route.bucket) + : undefined; + if (invokable !== undefined) { + routeInfo.invokable = invokable; + if (route.bucket) { + route.bucket.invokable = invokable; + } + } + } + + // Sync routeInfo.context to bucket before didEnter so classic setup() + // and activate() see the correct model. For error substates this is the + // error object passed via intermediateTransitionTo(name, error). + if ( + route.manager.capabilities.classicInterop && + routeInfo.context !== undefined && + route.bucket + ) { + (route.bucket as any).context = routeInfo.context; + } + + route.manager.didEnter(route.bucket, { + transition, + enter: true, + } as any); + } + + once(this as any, '_setOutlets'); + } + + onTransitionSettled( + activeTransition: InternalTransition, + newState: TransitionState + ): Promise { + // All getInvokable() calls in the transition have resolved and the outlet tree + // has already been updated incrementally via onRouteInvokableReady. The enter() + // promises are still in flight on each bucket. This method runs the remaining + // lifecycle: exit hooks, URL update, waiting for data, then didEnter/didExit. + + const microlib = this._routerMicrolib; + const partition = microlib.partitionRoutes(microlib.state!, newState); + + // Update router state now that we know which routes are settled. + microlib.oldState = microlib.state; + microlib.state = newState; + + // Fire willExit and exit on routes leaving the hierarchy, leaf-first. + for (const exitingRouteInfo of partition.exited) { + const route = exitingRouteInfo.route; + if (route !== undefined) { + route.manager.willExit(route.bucket, { transition: activeTransition } as any); + route.manager.exit(route.bucket, { transition: activeTransition } as any); + } + } + + // Remove exited routes from currentRouteInfos by filtering them out. + // We cannot simply truncate to unchanged.length because onRouteInvokableReady + // may have already written entering routes at higher indices. Instead, filter + // to only keep entries whose route object is not in the exited set. + const exitedRouteObjects = new Set( + partition.exited.map((exitingRouteInfo) => exitingRouteInfo.route) + ); + if (microlib.currentRouteInfos) { + microlib.currentRouteInfos = microlib.currentRouteInfos.filter( + (currentRouteInfo) => !exitedRouteObjects.has(currentRouteInfo.route) + ); + } + + // Reset routes whose context changed but which are not truly exiting. + for (const resetRouteInfo of partition.reset) { + const route = resetRouteInfo.route; + if (route !== undefined && route.manager && route.bucket !== undefined) { + route.manager.willExit(route.bucket, { + transition: activeTransition, + isExiting: false, + } as any); + } + } + + activeTransition.isActive = false; + microlib.activeTransition = undefined; + + // Collect the enter() promises from all entering and context-updating routes. + // These were fired during resolveViaManager and stored on each bucket. + // Swallow rejections with .catch: a rejected enterPromise means the transition + // was already aborted and handled via transitionDidError. If we let it propagate + // here it fires the global RSVP unhandled rejection handler on subsequent + // transitions that include the same route in their partition.entered. + const enteringRouteInfos = [...partition.entered, ...partition.updatedContext]; + const enterPromises = enteringRouteInfos.map((routeInfo) => { + const p = routeInfo.route?.bucket?.enterPromise ?? RSVPPromise.resolve(undefined); + return p.catch(() => undefined); + }); + + // Once all data loading is done, fire the post-data lifecycle hooks. + // The returned promise becomes part of the transition's promise chain + // (see Router.transitionByIntent in router_js), so an error here rejects + // the transition and propagates out to e.g. application.visit's caller. + return RSVPPromise.all(enterPromises).then(() => { + // Guard against the app being destroyed between when the transition + // settled and when this async callback fires (e.g. during test teardown). + if (this.isDestroying || this.isDestroyed) { + return; + } + if (activeTransition.isAborted) { + return; + } + + try { + // Fire didEnter on routes that newly entered, in parent-to-child order. + for (const enteredRouteInfo of partition.entered) { + const route = enteredRouteInfo.route; + if (route !== undefined) { + // Only sync routeInfo.context to the bucket for classic interop routes. + // Classic routes store context on the routeInfo via becomeResolved and + // need it pushed to the bucket before setup() runs in didEnter. + // Non-classic managers own their bucket.context entirely, they set it + // inside enter() and we must not overwrite it here. + if ( + route.manager.capabilities.classicInterop && + enteredRouteInfo.context !== undefined && + route.bucket + ) { + (route.bucket as any).context = enteredRouteInfo.context; + } + route.manager.didEnter(route.bucket, { + transition: activeTransition, + enter: true, + } as any); + } + } + + // Fire didEnter on routes whose context changed (same route, new model). + for (const updatedRouteInfo of partition.updatedContext) { + const route = updatedRouteInfo.route; + if (route !== undefined) { + if ( + route.manager.capabilities.classicInterop && + updatedRouteInfo.context !== undefined && + route.bucket + ) { + (route.bucket as any).context = updatedRouteInfo.context; + } + route.manager.didEnter(route.bucket, { + transition: activeTransition, + enter: false, + } as any); + } + } + } catch (error) { + // setup()/activate()/setupController() inside didEnter can throw. Route + // through the existing transitionDidError path so the error bubbles to + // route action handlers (and default error handler that searches for an + // error substate) the same way as errors from the resolution chain. + // Use a synthetic TransitionError shape with wasAborted=false so the + // error route lookup runs. The returned value is what should reject + // the transition's promise. Throwing it here makes onTransitionSettled's + // promise reject and propagates through application.visit's caller. + const errorRoute = newState.routeInfos[newState.routeInfos.length - 1]?.route; + const reason = microlib.transitionDidError( + { error, route: errorRoute, wasAborted: false } as any, + activeTransition as any + ); + throw reason; + } + + // Fire didExit on routes that left the hierarchy. + for (const exitedRouteInfo of partition.exited) { + const route = exitedRouteInfo.route; + if (route !== undefined) { + route.manager.didExit(route.bucket, { transition: activeTransition } as any); + } + } + + // The transition is fully settled. Replace currentRouteInfos with the + // real final route list from newState. During the transition, currentRouteInfos + // was updated incrementally (onRouteInvokableReady, onIntermediateTransition) + // and may still contain loading/error substate routes. Replacing it here + // ensures _setOutlets renders the real routes, not stale intermediates. + // This must happen before finalizeQueryParamChange so that QP finalization + // and _setOutlets both operate on the correct, authoritative route list. + // We copy the array (not alias) because subsequent transitions mutate + // currentRouteInfos via onRouteInvokableReady, and aliasing would mutate + // microlib.state.routeInfos (the settled state) too, corrupting the + // baseline used by partitionRoutes for the next transition. + microlib.currentRouteInfos = newState.routeInfos.slice(); + + // Finalize query params after didEnter so that controllers exist. + // Route.finalizeQueryParamChange accesses route.controller, which is + // created inside didEnter (via setup/setupController). Calling this + // before didEnter causes "cannot set _qpDelegate on undefined". + microlib.state!.queryParams = microlib.finalizeQueryParamChange( + microlib.currentRouteInfos!, + newState.queryParams, + activeTransition + ); + + // Update the URL now that query params are finalized. + microlib._updateURL(activeTransition, newState); + + // Re-render now that currentRouteInfos is authoritative. + once(this as any, '_setOutlets'); + + microlib.triggerEvent(microlib.currentRouteInfos!, true, 'didTransition', []); + microlib.didTransition(microlib.currentRouteInfos!); + microlib.toInfos(activeTransition, newState.routeInfos, true); + microlib.routeDidChange(activeTransition); + }); + } + _setOutlets() { // This is triggered async during Route#willDestroy. // If the router is also being destroyed we do not want to @@ -643,6 +931,19 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { return; } + // Skip rendering when the application instance was booted with + // shouldRender: false. The instance still goes through the full transition + // lifecycle (routes resolve, controllers initialise, models load) but no + // DOM is created. Used by application.visit() in non-rendering scenarios + // such as fastboot prefetching or unit-testing route logic. + let owner = getOwner(this); + let environment = owner?.lookup('-environment:main') as + | { options?: { shouldRender?: boolean } } + | undefined; + if (environment?.options?.shouldRender === false) { + return; + } + let routeInfos = this._routerMicrolib.currentRouteInfos; if (!routeInfos) { return; @@ -1365,43 +1666,9 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { } } - _scheduleLoadingEvent(transition: Transition, originRoute: Route) { - this._cancelSlowTransitionTimer(); - this._slowTransitionTimer = scheduleOnce( - 'routerTransitions', - this, - this._handleSlowTransition, - transition, - originRoute - ); - } - currentState: null | RouterState = null; targetState: null | RouterState = null; - _handleSlowTransition(transition: Transition, originRoute: Route) { - if (!this._routerMicrolib.activeTransition) { - // Don't fire an event if we've since moved on from - // the transition that put us in a loading state. - return; - } - let targetState = new RouterState( - this, - this._routerMicrolib, - this._routerMicrolib.activeTransition[STATE_SYMBOL]! - ); - this.set('targetState', targetState); - - transition.trigger(true, 'loading', transition, originRoute); - } - - _cancelSlowTransitionTimer() { - if (this._slowTransitionTimer) { - cancel(this._slowTransitionTimer); - } - this._slowTransitionTimer = null; - } - // These three helper functions are used to ensure errors aren't // re-raised if they're handled in a route's error action. _markErrorAsHandled(error: Error) { @@ -1567,15 +1834,6 @@ function forEachRouteAbove( // These get invoked when an action bubbles above ApplicationRoute // and are not meant to be overridable. let defaultActionHandlers = { - willResolveModel( - this: EmberRouter, - _routeInfos: InternalRouteInfo[], - transition: Transition, - originRoute: R - ) { - this._scheduleLoadingEvent(transition, originRoute); - }, - // Attempt to find an appropriate error route or substate to enter. error( this: EmberRouter, @@ -1613,36 +1871,6 @@ let defaultActionHandlers = { logError(error, `Error while processing route: ${transition.targetName}`); }, - - // Attempt to find an appropriate loading route or substate to enter. - loading(this: EmberRouter, routeInfos: InternalRouteInfo[], transition: Transition) { - let router = this; - - let routeInfoWithSlowLoading = routeInfos[routeInfos.length - 1]; - - forEachRouteAbove(routeInfos, (route, routeInfo) => { - // We don't check the leaf most routeInfos since that would - // technically be below where we're at in the route hierarchy. - if (routeInfo !== routeInfoWithSlowLoading) { - // Check for the existence of a 'loading' route. - let loadingRouteName = findRouteStateName(route, 'loading'); - if (loadingRouteName) { - router.intermediateTransitionTo(loadingRouteName); - return false; - } - } - - // Check for loading substate - let loadingSubstateName = findRouteSubstateName(route, 'loading'); - if (loadingSubstateName) { - router.intermediateTransitionTo(loadingSubstateName); - return false; - } - - // Don't bubble above pivot route. - return (transition.pivotHandler as any) !== route; - }); - }, }; function logError(_error: any, initialMessage: string) { diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index a9e1b102140..4e1615e2bdb 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -5,7 +5,7 @@ import type { SerializerFunc } from './router'; import type Router from './router'; import type { PublicTransition as Transition } from './transition'; import type InternalTransition from './transition'; -import { isTransition, PARAMS_SYMBOL, QUERY_PARAMS_SYMBOL } from './transition'; +import { isTransition, PARAMS_SYMBOL, QUERY_PARAMS_SYMBOL, STATE_SYMBOL } from './transition'; import { isParam, isPromise, merge } from './utils'; import { throwIfAborted } from './transition-aborted-error'; import type { RouteManager } from '@ember/-internals/routing/route-managers/route-manager'; @@ -250,7 +250,7 @@ export default class InternalRouteInfo { return this.params || {}; } - resolve(transition: InternalTransition): Promise> { + resolve(transition: InternalTransition): Promise { return Promise.resolve(this.routePromise) .then((route: Route) => { throwIfAborted(transition); @@ -262,40 +262,107 @@ export default class InternalRouteInfo { } /** - Manager-driven resolve path. Calls enter() and getInvokable() concurrently. - For classicInterop managers, enter() runs the beforeModel → model → afterModel chain. - For non-classic managers, enter() is the sole async entry point. + Manager-driven resolve path. Calls willEnter and enter() first, then getInvokable() + independently. enter() is fired and its promise stored on the bucket immediately, + but we do not await it here. resolve() settles as soon as getInvokable() resolves, + which allows the router to update currentRouteInfos and trigger rendering before + the route's data loading has finished. The enter() promise is awaited later by + EmberRouter after all getInvokable() calls in the transition have resolved. */ private resolveViaManager( manager: RouteManager, bucket: RouteStateBucket, transition: InternalTransition - ): Promise> { - manager.willEnter(bucket, { transition }); - - return Promise.all([ - manager.enter(bucket, { - transition, - to: this as unknown as RouteInfo, // will need to rethink this - cancel() { - transition.abort(); - }, - signal: transition.isTransition ? transition.signal : undefined, - getAncestorPromise: (routeInfo) => { - let index = transition.routeInfos.indexOf(routeInfo as unknown as InternalRouteInfo); - if (index === -1) { - return Promise.reject(new Error('RouteInfo not found in transition')); + ): Promise { + // Build the navigation args object once and share it between willEnter and enter. + // getAncestorPromise looks up the matching routeInfo on the transition by name + // and resolves with that ancestor's bucket.context once its enterPromise has + // settled. Sequential resolution guarantees that any ancestor's enterPromise + // is already set by the time a descendant's enter() runs. + // When called without an argument, it defaults to the immediate parent of the + // route currently being resolved. + const self = this as unknown as InternalRouteInfo; + const navigationArgs = { + transition, + to: this as unknown as RouteInfo, // will need to rethink this + cancel() { + transition.abort(); + }, + signal: transition.isTransition ? transition.signal : undefined, + getAncestorPromise: (ancestorRouteInfo?: RouteInfo) => { + const allRouteInfos = transition[STATE_SYMBOL]?.routeInfos ?? []; + let matched: InternalRouteInfo | undefined; + if (ancestorRouteInfo === undefined) { + // Default: parent of the current route. Find self by reference and + // step back one slot. If we are the root route there is no parent + // so we fall through to the no-match branch below. + const selfIndex = allRouteInfos.indexOf(self); + if (selfIndex > 0) { + matched = allRouteInfos[selfIndex - 1]; } - let ancestorRouteInfo = transition.routeInfos[index]; - return Promise.resolve(ancestorRouteInfo?.routePromise); - }, - }), - manager.getInvokable(bucket), - ]).then(([resolvedModel, invokable]) => { + } else { + matched = allRouteInfos.find((ri) => ri?.name === ancestorRouteInfo.name); + } + if (!matched) return Promise.resolve(undefined); + const ancestorBucket = matched.route?.bucket; + const ancestorEnterPromise = ancestorBucket?.enterPromise ?? Promise.resolve(undefined); + // Read from bucket.context (the live value set inside enter()) rather + // than matched.context. Managers whose getInvokable resolves before + // enter() (e.g. to render a loading state immediately) will have the + // routeInfo's context still undefined at the time enter() finishes; + // the bucket is the authoritative source. + return ancestorEnterPromise.then( + () => matched!.route?.bucket?.context ?? matched!.context + ); + }, + }; + + // willEnter fires synchronously. For the classic manager this is where loading + // substate detection happens, so it must run before getInvokable() is called + // and before any rendering occurs. + manager.willEnter(bucket, navigationArgs); + + // Fire enter() and immediately store the returned promise on the bucket. + // We do not await it here. Storing it now (before getInvokable() runs) ensures + // that by the time a child route's enter() is called, this ancestor bucket's + // enterPromise is already set and can be retrieved via getAncestorPromise. + const enterPromise = manager.enter(bucket, navigationArgs); + + bucket.enterPromise = enterPromise; + + // getInvokable() resolves as soon as the component definition is available. + // For classic routes it awaits bucket.enterPromise internally, so by the time + // this .then callback runs, enter() has resolved and bucket.context is set. + // We mirror bucket.context onto this.context so router_js's partitionRoutes + // can detect context changes between transitions (oldRouteInfo.context vs + // newRouteInfo.context) and emit updatedContext events for re-entered routes. + return manager.getInvokable(bucket).then((invokable) => { throwIfAborted(transition); this.invokable = invokable; - return this.becomeResolved(transition, resolvedModel as ModelFor); - }); + this.isResolved = true; + this.context = bucket.context as ModelFor | undefined; + + // Compute and stash params for URL generation. Mirrors becomeResolved. + // Without this, _updateURL fails for routes with dynamic segments because + // the recognizer cannot find the param values in routeInfo.params. + const resolvedContext = bucket.context as ModelFor | undefined; + this.params = this.serialize(resolvedContext); + this.stashResolvedModel(transition, resolvedContext); + transition[PARAMS_SYMBOL] = transition[PARAMS_SYMBOL] || {}; + transition[PARAMS_SYMBOL][this.name] = this.params; + + // Patch resolve() on this instance so subsequent transitions skip resolveViaManager. + // When named-transition-intent reuses this routeInfo as oldHandlerInfo, the patched + // resolve() returns immediately without re-running willEnter or enter(). This avoids + // the need for a separate ResolvedRouteInfo class while preserving the same behaviour. + this.resolve = (t: InternalTransition) => { + if (t && t.resolvedModels) { + t.resolvedModels[this.name] = this.context as ModelFor | undefined; + } + return Promise.resolve(this); + }; + return this; + }) as unknown as Promise; } becomeResolved( diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index 97647e10c07..8e953245014 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -64,6 +64,41 @@ export default abstract class Router { abstract routeDidChange(transition: Transition): void; abstract transitionDidError(error: TransitionError, transition: Transition): Transition | Error; + // Called once all getInvokable() calls in the transition have resolved, meaning the + // outlet tree has already been updated incrementally and enter() promises are in flight + // on each bucket. EmberRouter overrides this to run exit/enter lifecycle hooks, update + // the URL, await all enter() promises, then fire didEnter/didExit/routeDidChange. + // The default is a no-op so that test routers that do not care about rendering work + // without changes. + onTransitionSettled( + _transition: InternalTransition, + _newState: TransitionState + ): void | Promise {} + + // Called once per route, in parent-to-child order, as each getInvokable() resolves + // during a normal transition. EmberRouter overrides this to push the route into + // currentRouteInfos and schedule _setOutlets so rendering happens incrementally + // rather than waiting for the entire transition chain to finish. + // The routeIndex is the position of this route in the transition's resolved hierarchy, + // allowing EmberRouter to write directly to the correct slot in currentRouteInfos + // and replace any intermediate (loading/error substate) entry that was placed there. + // Default is a no-op so test routers work without changes. + onRouteInvokableReady( + _routeInfo: InternalRouteInfo, + _transition: InternalTransition, + _routeIndex: number + ): void {} + + // Called when intermediateTransitionTo fires, for example from ClassicRouteManager + // willEnter when it detects a loading substate. EmberRouter overrides this to splice + // the intermediate route into currentRouteInfos and schedule _setOutlets, without + // disturbing the ancestor routes that are already rendering correctly. + // Default is a no-op so test routers work without changes. + onIntermediateTransition( + _newState: TransitionState, + _transition: InternalTransition + ): void {} + /** The main entry point into the router. The API is essentially the same as the `map` method in `route-recognizer`. @@ -224,12 +259,12 @@ export default abstract class Router { } if (isIntermediate) { - let transition = new InternalTransition(this, undefined, newState); - transition.isIntermediate = true; - this.toReadOnlyInfos(transition, newState); - this.setupContexts(newState, transition); + let intermediateTransition = new InternalTransition(this, undefined, newState); + intermediateTransition.isIntermediate = true; + this.toReadOnlyInfos(intermediateTransition, newState); + this.onIntermediateTransition(newState, intermediateTransition); - this.routeWillChange(transition); + this.routeWillChange(intermediateTransition); return this.activeTransition!; } @@ -255,15 +290,18 @@ export default abstract class Router { } this.activeTransition = newTransition; - // Transition promises by default resolve with resolved state. - // For our purposes, swap out the promise to resolve - // after the transition has been finalized. + // Once all getInvokable() calls have resolved (the TransitionState promise settles), + // hand off to EmberRouter to run exit/enter lifecycle, update the URL, and await + // all enter() promises before firing didEnter/didExit/routeDidChange. Returning + // the promise here so an error inside onTransitionSettled (e.g. setup() throwing + // inside didEnter) rejects the transition's outer promise, which application.visit + // and handleURL callers chain off of. newTransition.promise = newTransition.promise!.then( - (result: TransitionState) => { - return this.finalizeTransition(newTransition, result); + (newState: TransitionState) => { + return this.onTransitionSettled(newTransition, newState); }, null, - promiseLabel('Settle transition promise when transition is finalized') + promiseLabel('Settle transition promise when all getInvokable calls have resolved') ); if (!wasTransitioning) { @@ -336,158 +374,6 @@ export default abstract class Router { /** @private - Updates the URL (if necessary) and calls `setupContexts` - to update the router's array of `currentRouteInfos`. - */ - private finalizeTransition( - transition: InternalTransition, - newState: TransitionState - ): R | Promise { - try { - log( - transition.router, - transition.sequence, - 'Resolved all models on destination route; finalizing transition.' - ); - - let routeInfos = newState.routeInfos; - - // Run all the necessary enter/setup/exit hooks - this.setupContexts(newState, transition); - - // Check if a redirect occurred in enter/setup - if (transition.isAborted) { - // TODO: cleaner way? distinguish b/w targetRouteInfos? - this.state!.routeInfos = this.currentRouteInfos!; - return Promise.reject(logAbort(transition)); - } - - this._updateURL(transition, newState); - - transition.isActive = false; - this.activeTransition = undefined; - - this.triggerEvent(this.currentRouteInfos!, true, 'didTransition', []); - this.didTransition(this.currentRouteInfos!); - this.toInfos(transition, newState.routeInfos, true); - this.routeDidChange(transition); - - log(this, transition.sequence, 'TRANSITION COMPLETE.'); - - // Resolve with the final route. - return routeInfos[routeInfos.length - 1]!.route!; - } catch (e) { - if (!isTransitionAborted(e)) { - let infos = transition[STATE_SYMBOL]!.routeInfos; - transition.trigger(true, 'error', e, transition, infos[infos.length - 1]!.route); - transition.abort(); - } - - throw e; - } - } - - /** - @private - - Takes an Array of `RouteInfo`s, figures out which ones are - exiting, entering, or changing contexts, and calls the - proper route hooks. - - For example, consider the following tree of routes. Each route is - followed by the URL segment it handles. - - ``` - |~index ("/") - | |~posts ("/posts") - | | |-showPost ("/:id") - | | |-newPost ("/new") - | | |-editPost ("/edit") - | |~about ("/about/:id") - ``` - - Consider the following transitions: - - 1. A URL transition to `/posts/1`. - 1. Triggers the `*model` callbacks on the - `index`, `posts`, and `showPost` routes - 2. Triggers the `enter` callback on the same - 3. Triggers the `setup` callback on the same - 2. A direct transition to `newPost` - 1. Triggers the `exit` callback on `showPost` - 2. Triggers the `enter` callback on `newPost` - 3. Triggers the `setup` callback on `newPost` - 3. A direct transition to `about` with a specified - context object - 1. Triggers the `exit` callback on `newPost` - and `posts` - 2. Triggers the `serialize` callback on `about` - 3. Triggers the `enter` callback on `about` - 4. Triggers the `setup` callback on `about` - - @param {Router} transition - @param {TransitionState} newState -*/ - private setupContexts(newState: TransitionState, transition?: InternalTransition) { - let partition = this.partitionRoutes(this.state!, newState); - let i, l, route; - - for (i = 0, l = partition.exited.length; i < l; i++) { - route = partition.exited[i]!.route; - - if (route !== undefined) { - let args = { transition, isExiting: true }; - route.manager.willExit(route.bucket, args); - route.manager.exit(route.bucket, args); - } - } - - let oldState = (this.oldState = this.state); - this.state = newState; - let currentRouteInfos = (this.currentRouteInfos = partition.unchanged.slice()); - - try { - for (i = 0, l = partition.reset.length; i < l; i++) { - route = partition.reset[i]!.route; - if (route !== undefined) { - if (route.manager && route.bucket !== undefined) { - // Reset (not exiting) — willExit with isExiting: false semantics - let args = { transition, isExiting: false }; - route.manager.willExit(route.bucket, args); - } else if (route._internalReset !== undefined) { - route._internalReset(false, transition); - } - } - } - - for (i = 0, l = partition.updatedContext.length; i < l; i++) { - this.routeEnteredOrUpdated( - currentRouteInfos, - partition.updatedContext[i]!, - false, - transition! - ); - } - - for (i = 0, l = partition.entered.length; i < l; i++) { - this.routeEnteredOrUpdated(currentRouteInfos, partition.entered[i]!, true, transition!); - } - } catch (e) { - this.state = oldState; - this.currentRouteInfos = oldState!.routeInfos; - throw e; - } - - this.state.queryParams = this.finalizeQueryParamChange( - currentRouteInfos, - newState.queryParams, - transition! - ); - } - - /** - @private - Fires queryParamsDidChange event */ private fireQueryParamDidChange(newState: TransitionState, queryParamChangelist: ChangeList) { @@ -509,67 +395,6 @@ export default abstract class Router { /** @private - Helper method used by setupContexts. Handles errors or redirects - that may happen in enter/setup. -*/ - private routeEnteredOrUpdated( - currentRouteInfos: InternalRouteInfo[], - routeInfo: InternalRouteInfo, - enter: boolean, - transition?: InternalTransition - ) { - let route = routeInfo.route, - context = routeInfo.context; - - function _routeEnteredOrUpdatedViaManager(route: R) { - // Sync routeInfo.context -> bucket.context. This is necessary because - // intermediate transitions (error/loading substates) skip the resolve - // phase entirely — they go straight to setupContexts via - // intermediateTransitionTo -> applyToState(isIntermediate=true) -> - // becomeResolved() -> setupContexts. Since resolveViaManager() never - // runs, bucket.context is never set during resolve. - if (route.bucket !== undefined) { - (route.bucket as any).context = context; - } - // For intermediate transitions, getInvokable() was never called during - // resolve. Call it now so the template can be found for rendering. - // ClassicRouteManager.getInvokable() returns a synchronously resolved - // promise, so the invokable is available by the time didEnter runs setup() - // which schedules _setOutlets. - if (routeInfo.invokable === undefined && route.manager) { - route.manager.getInvokable(route.bucket).then((invokable: any) => { - routeInfo.invokable = invokable; - if (route.bucket) { - (route.bucket as any).invokable = invokable; - } - }); - } - // Pass transition and enter flag through args so the ClassicRouteManager - // can forward them to route.enter() and route.setup() - let args = { transition, enter }; - route.manager!.didEnter(route.bucket, args); - - throwIfAborted(transition); - - currentRouteInfos.push(routeInfo); - return route; - } - - // If the route doesn't exist, it means we haven't resolved the route promise yet - if (route === undefined) { - routeInfo.routePromise = routeInfo.routePromise.then((route) => { - return _routeEnteredOrUpdatedViaManager(route); - }); - } else { - _routeEnteredOrUpdatedViaManager(route); - } - - return true; - } - - /** - @private - This function is called when transitioning from one URL to another to determine which routes are no longer active, which routes are newly active, and which routes remain @@ -608,7 +433,9 @@ export default abstract class Router { @return {Partition} */ - private partitionRoutes(oldState: TransitionState, newState: TransitionState) { + // Public so EmberRouter can call it via _routerMicrolib when orchestrating + // onRouteInvokableReady and onIntermediateTransition. Previously private. + partitionRoutes(oldState: TransitionState, newState: TransitionState) { let oldRouteInfos = oldState.routeInfos; let newRouteInfos = newState.routeInfos; @@ -656,7 +483,8 @@ export default abstract class Router { return routes; } - private _updateURL(transition: OpaqueTransition, state: TransitionState) { + // Public so EmberRouter can call it via _routerMicrolib after awaiting enter() promises. + _updateURL(transition: OpaqueTransition, state: TransitionState) { let urlMethod: string | null = transition.urlMethod; if (!urlMethod) { @@ -719,7 +547,8 @@ export default abstract class Router { } } - private finalizeQueryParamChange( + // Public so EmberRouter can call it via _routerMicrolib as part of transition settlement. + finalizeQueryParamChange( resolvedHandlers: InternalRouteInfo[], newQueryParams: Dict, transition: OpaqueTransition @@ -950,9 +779,16 @@ export default abstract class Router { queryParams = partitionedArgs[1]; // Construct a TransitionIntent with the provided params - // and apply it to the present state of the router. + // and apply it to the present state of the router. During an active + // transition (e.g. while a loading substate is rendering), the active + // transition's state holds the partially resolved route hierarchy and + // is the correct baseline for URL generation. Without this, link-to + // helpers rendered inside a loading substate fail to compute their href + // because microlib.state has not been updated yet (state assignment only + // happens in onTransitionSettled). + let baseState = (this.activeTransition && this.activeTransition[STATE_SYMBOL]) || this.state!; let intent = new NamedTransitionIntent(this, routeName, undefined, suppliedParams); - let state = intent.applyToState(this.state!, false); + let state = intent.applyToState(baseState, false); let params: Params = {}; for (let i = 0, len = state.routeInfos.length; i < len; ++i) { diff --git a/packages/router_js/lib/transition-state.ts b/packages/router_js/lib/transition-state.ts index bc11a7883af..ee79474c1e2 100644 --- a/packages/router_js/lib/transition-state.ts +++ b/packages/router_js/lib/transition-state.ts @@ -1,6 +1,6 @@ import { Promise } from 'rsvp'; import type { Dict } from './core'; -import type { Route, ResolvedRouteInfo } from './route-info'; +import type { Route } from './route-info'; import type InternalRouteInfo from './route-info'; import type Transition from './transition'; import { forEach, promiseLabel } from './utils'; @@ -36,15 +36,15 @@ function resolveOneRouteInfo( transition: Transition ): void | Promise { if (transition.resolveIndex === currentState.routeInfos.length) { - // This is is the only possible - // fulfill value of TransitionState#resolve + // All routes in this transition have had their getInvokable() resolved. + // This is the only fulfill value of TransitionState#resolve. return; } let routeInfo = currentState.routeInfos[transition.resolveIndex]!; let callback = proceed.bind(null, currentState, transition) as ( - resolvedRouteInfo: ResolvedRouteInfo + readyRouteInfo: InternalRouteInfo ) => void | Promise; return routeInfo.resolve(transition).then(callback, null, currentState.promiseLabel('Proceed')); @@ -53,36 +53,30 @@ function resolveOneRouteInfo( function proceed( currentState: TransitionState, transition: Transition, - resolvedRouteInfo: ResolvedRouteInfo + readyRouteInfo: InternalRouteInfo ): void | Promise { - let wasAlreadyResolved = currentState.routeInfos[transition.resolveIndex]!.isResolved; - - // Swap the previously unresolved routeInfo with - // the resolved routeInfo - currentState.routeInfos[transition.resolveIndex++] = resolvedRouteInfo; - - if (!wasAlreadyResolved) { - // Call the redirect hook. The reason we call it here - // vs. afterModel is so that redirects into child - // routes don't re-run the model hooks for this - // already-resolved route. - let { route } = resolvedRouteInfo; - if (route !== undefined) { - if (route.manager) { - // redirect is a classic-only hook, not part of the RouteManager interface. - // Non-classic managers redirect during enter() via the transition/cancel API. - if (route.manager.capabilities.classicInterop && route.redirect) { - route.redirect(resolvedRouteInfo.context, transition); - } - } else if (route.redirect) { - // No manager yet (migration period fallback) — call redirect directly - route.redirect(resolvedRouteInfo.context, transition); - } + const routeIndex = transition.resolveIndex; + currentState.routeInfos[transition.resolveIndex++] = readyRouteInfo; + + // Notify the router that this route's invokable is ready. EmberRouter uses + // this to place the route at routeIndex in currentRouteInfos, replacing any + // loading/error substate that was entered for this position, and schedules + // _setOutlets so the outlet tree is updated before enter() finishes. + transition.router.onRouteInvokableReady(readyRouteInfo, transition, routeIndex); + + // Skip redirect for intermediate transitions (loading/error substates). Their + // routeInfos come from applyToState already resolved and never run the model + // hook in this transition, so a redirect callback would be inappropriate. + if (!transition.isIntermediate) { + // Call the redirect hook now that the route's model has resolved. Calling + // it here, between resolution and the next route's resolution, lets a + // redirect into a child route skip re-running this route's model hook. + const route = readyRouteInfo.route; + if (route !== undefined && route.redirect) { + route.redirect(readyRouteInfo.context, transition); } } - // Proceed after ensuring that the redirect hook - // didn't abort this transition by transitioning elsewhere. throwIfAborted(transition); return resolveOneRouteInfo(currentState, transition); From 306c22e9af8cdd1496d3d3a9f4314d4b36e170a0 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Thu, 30 Apr 2026 10:45:04 +0100 Subject: [PATCH 17/20] simplify how resolve returns the fast path --- .../routing/route-managers/utils.ts | 1 + packages/router_js/lib/route-info.ts | 27 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/@ember/-internals/routing/route-managers/utils.ts b/packages/@ember/-internals/routing/route-managers/utils.ts index ea1ea3d06af..18a33cec417 100644 --- a/packages/@ember/-internals/routing/route-managers/utils.ts +++ b/packages/@ember/-internals/routing/route-managers/utils.ts @@ -6,6 +6,7 @@ export interface RouteStateBucket { invokable?: object; instance?: unknown; args: object; + context?: unknown; // Set by the router synchronously after calling manager.enter(), before awaiting it. // This makes the promise available to getAncestorPromise on child routes, which need // to await their parent's data loading before starting their own. diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 4e1615e2bdb..b68962d5981 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -251,6 +251,19 @@ export default class InternalRouteInfo { } resolve(transition: InternalTransition): Promise { + if (this.isResolved) { + // Fast path for re-entered routes. When named-transition-intent reuses this + // routeInfo as oldHandlerInfo on a subsequent transition, we skip the full + // resolveViaManager pass (no willEnter, no enter()) and just stash the + // already-resolved context onto the new transition so downstream consumers + // (e.g. modelFor()) see it. Mirrors what ResolvedRouteInfo.resolve does on + // becomeResolved-produced instances, without needing to build one. + if (transition && transition.resolvedModels) { + transition.resolvedModels[this.name] = this.context as ModelFor | undefined; + } + return Promise.resolve(this); + } + return Promise.resolve(this.routePromise) .then((route: Route) => { throwIfAborted(transition); @@ -311,9 +324,7 @@ export default class InternalRouteInfo { // enter() (e.g. to render a loading state immediately) will have the // routeInfo's context still undefined at the time enter() finishes; // the bucket is the authoritative source. - return ancestorEnterPromise.then( - () => matched!.route?.bucket?.context ?? matched!.context - ); + return ancestorEnterPromise.then(() => matched!.route?.bucket?.context ?? matched!.context); }, }; @@ -351,16 +362,6 @@ export default class InternalRouteInfo { transition[PARAMS_SYMBOL] = transition[PARAMS_SYMBOL] || {}; transition[PARAMS_SYMBOL][this.name] = this.params; - // Patch resolve() on this instance so subsequent transitions skip resolveViaManager. - // When named-transition-intent reuses this routeInfo as oldHandlerInfo, the patched - // resolve() returns immediately without re-running willEnter or enter(). This avoids - // the need for a separate ResolvedRouteInfo class while preserving the same behaviour. - this.resolve = (t: InternalTransition) => { - if (t && t.resolvedModels) { - t.resolvedModels[this.name] = this.context as ModelFor | undefined; - } - return Promise.resolve(this); - }; return this; }) as unknown as Promise; } From 4159248b0872f9cacc776b3eed6eabb187317d4f Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Thu, 30 Apr 2026 16:17:26 +0100 Subject: [PATCH 18/20] re-enable router_js tests --- index.html | 9 +- packages/router_js/lib/transition-state.ts | 22 +- packages/router_js/tests/route_info_test.ts | 22 +- packages/router_js/tests/router_test.ts | 15 +- packages/router_js/tests/test_helpers.ts | 401 +++++++++++++++++++- 5 files changed, 435 insertions(+), 34 deletions(-) diff --git a/index.html b/index.html index 70b646751d4..77ad682afac 100644 --- a/index.html +++ b/index.html @@ -67,12 +67,9 @@ }); // tests in non-scoped packages - import.meta.glob( - ['./packages/*/tests/**/*.{js,ts,gjs,gts}', '!./packages/router_js/tests/**'], - { - eager: true, - } - ); + import.meta.glob('./packages/*/tests/**/*.{js,ts,gjs,gts}', { + eager: true, + }); // packages originally authored in glimmer-vm repo have a different test // naming convention diff --git a/packages/router_js/lib/transition-state.ts b/packages/router_js/lib/transition-state.ts index ee79474c1e2..2ffae2a0f33 100644 --- a/packages/router_js/lib/transition-state.ts +++ b/packages/router_js/lib/transition-state.ts @@ -42,8 +42,13 @@ function resolveOneRouteInfo( } let routeInfo = currentState.routeInfos[transition.resolveIndex]!; + // Capture whether the routeInfo was already resolved BEFORE this resolve + // pass began. When named-transition-intent reuses an oldHandlerInfo, its + // isResolved is already true (set on a prior transition); we use this flag + // to decide whether redirect should fire in proceed. + let wasAlreadyResolved = routeInfo.isResolved; - let callback = proceed.bind(null, currentState, transition) as ( + let callback = proceed.bind(null, currentState, transition, wasAlreadyResolved) as ( readyRouteInfo: InternalRouteInfo ) => void | Promise; @@ -53,6 +58,7 @@ function resolveOneRouteInfo( function proceed( currentState: TransitionState, transition: Transition, + wasAlreadyResolved: boolean, readyRouteInfo: InternalRouteInfo ): void | Promise { const routeIndex = transition.resolveIndex; @@ -62,12 +68,14 @@ function proceed( // this to place the route at routeIndex in currentRouteInfos, replacing any // loading/error substate that was entered for this position, and schedules // _setOutlets so the outlet tree is updated before enter() finishes. - transition.router.onRouteInvokableReady(readyRouteInfo, transition, routeIndex); - - // Skip redirect for intermediate transitions (loading/error substates). Their - // routeInfos come from applyToState already resolved and never run the model - // hook in this transition, so a redirect callback would be inappropriate. - if (!transition.isIntermediate) { + // Guard against fake/synthetic transitions used in tests that don't have a + // router reference; those transitions don't drive any rendering anyway. + transition.router?.onRouteInvokableReady?.(readyRouteInfo, transition, routeIndex); + + // Skip redirect for intermediate transitions (loading/error substates), and + // for routes that were already resolved on a prior transition. Mirrors the + // original transition-state proceed which gated redirect on `!wasAlreadyResolved`. + if (!transition.isIntermediate && !wasAlreadyResolved) { // Call the redirect hook now that the route's model has resolved. Calling // it here, between resolution and the next route's resolution, lets a // redirect into a child route skip re-running this route's model hook. diff --git a/packages/router_js/tests/route_info_test.ts b/packages/router_js/tests/route_info_test.ts index 31ba500e7b7..a1be70f97d0 100644 --- a/packages/router_js/tests/route_info_test.ts +++ b/packages/router_js/tests/route_info_test.ts @@ -54,35 +54,33 @@ QUnit.test('RouteInfo can be aborted mid-resolve', function (assert) { }); }); -QUnit.test('RouteInfo#resolve resolves with a ResolvedRouteInfo', function (assert) { +QUnit.test('RouteInfo#resolve resolves isResolved on the RouteInfo', function (assert) { assert.expect(1); let routeInfo = createHandlerInfo('stub'); routeInfo.resolve({} as Transition).then((resolvedRouteInfo) => { - assert.ok(resolvedRouteInfo instanceof ResolvedRouteInfo); + assert.ok(resolvedRouteInfo.isResolved); }); }); -QUnit.test('RouteInfo#resolve runs beforeModel hook on handler', function (assert) { - assert.expect(1); - +QUnit.test('RouteInfo#resolve runs beforeModel hook on handler', async function (assert) { + assert.expect(2); let transition = {} as Transition; - + let beforeModelCalled = false; let routeInfo = createHandlerInfo('stub', { route: createHandler('stub', { beforeModel: function (currentTransition: Transition) { + beforeModelCalled = true; assert.equal( - transition, currentTransition, - 'beforeModel was called with the payload we passed to resolve()' + transition, + 'beforeModel was called with the transition passed to resolve()' ); }, }), }); - - routeInfo.resolve(transition).then(() => { - assert.ok(true, 'routeInfo resolved successfully'); - }); + await routeInfo.resolve(transition); + assert.true(beforeModelCalled, 'beforeModel was called'); }); QUnit.test('RouteInfo#resolve runs getModel hook', async function (assert) { diff --git a/packages/router_js/tests/router_test.ts b/packages/router_js/tests/router_test.ts index 2f8d320a751..77df5c18101 100644 --- a/packages/router_js/tests/router_test.ts +++ b/packages/router_js/tests/router_test.ts @@ -1428,14 +1428,16 @@ scenarios.forEach(function (scenario) { post: createHandler('post', { model() { redirected1 = true; - router.transitionTo('/foo/bar'); + // The redirect aborts this transition, swallow the rejection so it + // does not surface as an unhandled rejection in the full test suite. + ignoreTransitionError(router.transitionTo('/foo/bar')); }, }), foo: createHandler('foo', { model() { redirected1 = false; redirected2 = true; - router.transitionTo('/ok'); + ignoreTransitionError(router.transitionTo('/ok')); }, }), ok: createHandler('ok'), @@ -1517,7 +1519,9 @@ scenarios.forEach(function (scenario) { post: createHandler('post', { model() { redirected = true; - router.transitionTo('/foo/bar'); + // foo.model aborts this redirect transition, swallow the rejection + // so it does not surface as an unhandled rejection in the full suite. + ignoreTransitionError(router.transitionTo('/foo/bar')); }, }), foo: createHandler('foo', { @@ -1764,7 +1768,10 @@ scenarios.forEach(function (scenario) { post: createHandler('post', { model() { redirected = true; - router.transitionTo('/foo/bar'); + // foo.model throws, errorHandled branch in transitionDidError returns + // the transition itself which becomes the rejection value, swallow it + // so it does not surface as an unhandled rejection in the full suite. + ignoreTransitionError(router.transitionTo('/foo/bar')); }, }), foo: createHandler('foo', { diff --git a/packages/router_js/tests/test_helpers.ts b/packages/router_js/tests/test_helpers.ts index 38f0f938dca..d32b3736a88 100644 --- a/packages/router_js/tests/test_helpers.ts +++ b/packages/router_js/tests/test_helpers.ts @@ -7,7 +7,8 @@ import type { PublicTransition } from '../lib/transition'; import { logAbort } from '../lib/transition'; import type { TransitionError } from '../lib/transition-state'; import type { UnrecognizedURLError } from '../lib/unrecognized-url-error'; -import { isTransitionAborted } from '../lib/transition-aborted-error'; +import { isTransitionAborted, throwIfAborted } from '../lib/transition-aborted-error'; +import { Promise } from 'rsvp'; // A useful function to allow you to ignore transition errors in a testing context export async function ignoreTransitionError(transition: Transition) { @@ -65,11 +66,180 @@ export { assertAbort, }; +// Minimal structural copies of the route manager types. The real interfaces +// live in @ember/-internals/routing/route-managers/route-manager. We copy +// them here so router_js tests stay independent of the ember-source side. +interface RouteCapabilities { + classicInterop: boolean; +} + +interface NavigationArgs { + transition: any; + to: any; + cancel: () => void; + signal?: AbortSignal; + getAncestorPromise: (routeInfo?: any) => Promise; +} + +interface RouteManagerLike { + capabilities: RouteCapabilities; + createRoute(definition: any, args: { name: string }): TestRouteBucket; + getDestroyable(bucket: TestRouteBucket): unknown; + willEnter(bucket: TestRouteBucket, args: NavigationArgs): void; + enter(bucket: TestRouteBucket, args: NavigationArgs): Promise; + didEnter(bucket: TestRouteBucket, args: NavigationArgs & { enter?: boolean }): void; + willExit(bucket: TestRouteBucket, args: NavigationArgs & { isExiting?: boolean }): void; + exit(bucket: TestRouteBucket, args: NavigationArgs): void; + didExit(bucket: TestRouteBucket, args: NavigationArgs): void; + getInvokable(bucket: TestRouteBucket): Promise; +} + +// Bucket carries per-route state during a transition. Mirrors ClassicRouteBucket +// without ember-side decorators. +class TestRouteBucket { + route: Route; + args: { name: string }; + context: unknown = undefined; + enterPromise: Promise | undefined = undefined; + invokable: object | undefined = undefined; + + constructor(route: Route, args: { name: string }) { + this.route = route; + this.args = args; + } +} + +// In-test route manager that runs the classic beforeModel/model/afterModel +// chain without depending on EmberObject, owners, or the glimmer module graph. +// router_js tests expect plain object handlers with model/setup/etc. hooks, +// so this manager dispatches directly to those hooks. +class TestRouteManager implements RouteManagerLike { + capabilities: RouteCapabilities = { classicInterop: true }; + + createRoute(handler: Route, args: { name: string }): TestRouteBucket { + const bucket = new TestRouteBucket(handler, args); + handler.bucket = bucket as any; + handler.manager = this as any; + return bucket; + } + + getDestroyable(bucket: TestRouteBucket): unknown { + return bucket.route; + } + + willEnter(_bucket: TestRouteBucket, _args: NavigationArgs): void {} + + enter(bucket: TestRouteBucket, args: NavigationArgs): Promise { + const transition = args.transition; + const routeInfo = args.to; + // Source the route from routeInfo.route (the value the router resolved), + // not bucket.route. createHandlerInfo and friends sometimes attach a route + // to the routeInfo via prototype assignment that doesn't match the route + // the bucket was constructed with; routeInfo.route is the authoritative one. + const route = (routeInfo?.route ?? bucket.route) as Route; + + if (transition && typeof transition.trigger === 'function') { + transition.trigger(true, 'willResolveModel', transition, route); + } + + const name = routeInfo?.name ?? bucket.args.name; + // If a hook returns a Transition (e.g. router.transitionTo from inside + // beforeModel/afterModel), null it out so the chain doesn't pause waiting + // on a "transition" thenable. Mirrors classic-route-manager's _isTransition + // guards. UnresolvedRouteInfoByParam.getModel already handles this for + // route.model itself. + const isTransitionLike = (obj: unknown): boolean => + typeof obj === 'object' && obj !== null && (obj as any).isTransition === true; + + const beforeModelResult = route?.beforeModel?.(transition); + return Promise.resolve(isTransitionLike(beforeModelResult) ? null : beforeModelResult) + .then(() => routeInfo.getModel(transition)) + .then((model: unknown) => { + // Stash the model on transition.resolvedModels BEFORE afterModel so + // afterModel can swap it (test: "resolved models can be swapped out + // within afterModel"). Mirrors classic-route-manager._getModel + + // _runAfterModel order. + if (transition) { + transition.resolvedModels = transition.resolvedModels || {}; + transition.resolvedModels[name] = model; + } + const afterModelResult = route?.afterModel?.(model as any, transition); + const safeAfterModel = isTransitionLike(afterModelResult) ? null : afterModelResult; + return Promise.resolve(safeAfterModel).then(() => { + // afterModel may have swapped transition.resolvedModels[name]; pick + // up the (possibly new) value rather than the original model. + return transition?.resolvedModels?.[name] ?? model; + }); + }) + .then((model: unknown) => { + bucket.context = model; + return model; + }); + } + + didEnter(bucket: TestRouteBucket, args: NavigationArgs & { enter?: boolean }): void { + const transition = args.transition; + const routeInfo = args.to; + const route = (routeInfo?.route ?? bucket.route) as Route; + const context = bucket.context; + + if (route) { + // Fire route.enter() for fresh entries (not for context-updates), mirroring + // classic setupContexts which gated enter on the `enter` flag passed in. + if (args.enter !== false && route.enter) { + route.enter(transition); + } + // If enter triggered a redirect (router.transitionTo from inside enter), + // the transition is now aborted. Throw so the outer didEnter loop bails + // before calling setup or invoking later routes' enter, mirroring the + // throwIfAborted between enter and setup in main's setupContexts. + throwIfAborted(transition); + route.context = context as any; + if (route.setup) { + route.setup(context as any, transition); + } + // Same guard between setup and the next route, so a redirect from setup + // also stops further enters/setups in this loop iteration. + throwIfAborted(transition); + } + } + + willExit(_bucket: TestRouteBucket, _args: NavigationArgs & { isExiting?: boolean }): void {} + + exit(bucket: TestRouteBucket, args: NavigationArgs): void { + // Mirror classic setupContexts: fire route.exit and clear route.context + // when the route is leaving the active hierarchy. + const transition = args.transition; + const route = bucket.route; + if (route) { + delete route.context; + if (route.exit) { + route.exit(transition); + } + } + } + + didExit(_bucket: TestRouteBucket, _args: NavigationArgs): void {} + + // Gate on enterPromise so the resolution loop stays sequential, mirroring + // classic behaviour. Most existing tests assume a route's model resolves + // before its child route's model starts. + getInvokable(bucket: TestRouteBucket): Promise { + return (bucket.enterPromise ?? Promise.resolve()).then(() => undefined); + } +} + +const SHARED_TEST_MANAGER = new TestRouteManager(); + export function createHandler(name: string, options?: Dict): Route { - return Object.assign( + const handler = Object.assign( { name, routeName: name, context: {}, names: [], handler: name, _internalName: name }, options ) as unknown as Route; + // Attach a manager+bucket so resolveViaManager has something to work with. + // Without this, route.manager is undefined and the resolution loop blows up. + SHARED_TEST_MANAGER.createRoute(handler as unknown as Route, { name }); + return handler; } export class TestRouter extends Router { @@ -94,14 +264,231 @@ export class TestRouter extends Router { return error.error; } } - getRoute(_name: string): any { - return {}; + getRoute(name: string): any { + // Return a handler with a manager attached so resolveViaManager has + // something to work with. Bare {} would crash on route.manager access. + return createHandler(name); } getSerializer(_name: string): any { return () => {}; } + + // As each route's getInvokable resolves, write the resolved routeInfo into + // currentRouteInfos at its slot. Mirrors EmberRouter.onRouteInvokableReady. + onRouteInvokableReady( + routeInfo: InternalRouteInfoLike, + _transition: any, + routeIndex: number + ): void { + const current = (this.currentRouteInfos as any[]) ?? []; + current[routeIndex] = routeInfo; + (this as any).currentRouteInfos = current; + } + + // Mirrors EmberRouter.onIntermediateTransition. Substate routes have no + // model hook to await, so we synchronously fire didEnter on each entered + // route (after splicing currentRouteInfos), so route.enter/route.setup run. + // For async getRoute scenarios the route may not be resolved yet at this + // point (becomeResolved was called before fetchRoute); chain on routePromise + // so didEnter still fires once the handler is available, mirroring main's + // routeEnteredOrUpdated path that defers via routePromise.then. + onIntermediateTransition(newState: any, transition: any): void { + const partition = this.partitionRoutes(this.state!, newState); + + const currentRouteInfos = [...partition.unchanged, ...partition.entered]; + (this as any).currentRouteInfos = currentRouteInfos; + + this.oldState = this.state; + this.state = newState; + + const fireDidEnter = (routeInfo: any, route: any) => { + if (!route?.manager) return; + if ( + route.manager.capabilities?.classicInterop && + routeInfo.context !== undefined && + route.bucket + ) { + route.bucket.context = routeInfo.context; + } + route.manager.didEnter(route.bucket, { transition, enter: true } as any); + }; + + for (const routeInfo of partition.entered) { + const route = (routeInfo as any).route; + if (route) { + fireDidEnter(routeInfo, route); + } else { + // Async route: trigger fetchRoute via routePromise and defer didEnter + // until it resolves. Without this faq.setup never fires in scenarios + // where getRoute returns a Promise. + (routeInfo as any).routePromise.then((resolved: any) => { + fireDidEnter(routeInfo, resolved); + }); + } + } + } + + // Mirrors EmberRouter.onTransitionSettled, the orchestrator that fires + // willExit/exit/didEnter/didExit, awaits enterPromises, finalizes QPs, and + // updates the URL. Without this, classic route.enter/route.exit hooks never + // run for tests that use a manager-driven router. + onTransitionSettled(activeTransition: any, newState: any): Promise { + const partition = this.partitionRoutes(this.state!, newState); + + // Capture pre-transition state so we can revert on didEnter throws. + // Mirrors classic setupContexts: a throw inside enter/setup must roll + // back router state so the next transition sees the routes as still- + // unentered and re-fires their enter hooks. + const preTransitionState = this.state; + this.oldState = this.state; + this.state = newState; + + // willExit + exit on routes leaving the hierarchy. + for (const exitingRouteInfo of partition.exited) { + const route = (exitingRouteInfo as any).route; + if (route?.manager) { + route.manager.willExit(route.bucket, { transition: activeTransition } as any); + route.manager.exit(route.bucket, { transition: activeTransition } as any); + } + } + + // Filter exited routes out of currentRouteInfos. Cannot truncate to + // unchanged.length because onRouteInvokableReady may have already written + // entering routes at higher indices. + const exitedRouteObjects = new Set( + partition.exited.map((ri: any) => ri.route) + ); + if (this.currentRouteInfos) { + this.currentRouteInfos = this.currentRouteInfos.filter( + (cri: any) => !exitedRouteObjects.has(cri.route) + ) as any; + } + + // Reset routes whose context changed but which are not exiting. + for (const resetRouteInfo of partition.reset) { + const route = (resetRouteInfo as any).route; + if (route?.manager && route.bucket !== undefined) { + route.manager.willExit(route.bucket, { + transition: activeTransition, + isExiting: false, + } as any); + } + } + + // Wait for all entering routes' enter() promises before firing didEnter. + // Swallow rejections so a single failed transition does not poison the + // global RSVP unhandled rejection handler. + const enteringRouteInfos = [...partition.entered, ...partition.updatedContext]; + const enterPromises = enteringRouteInfos.map((routeInfo: any) => { + const p = routeInfo.route?.bucket?.enterPromise ?? Promise.resolve(undefined); + return (p as any).catch + ? (p as any).catch(() => undefined) + : p; + }); + + return (Promise as any).all(enterPromises).then(() => { + if (activeTransition.isAborted) return; + + try { + for (const enteredRouteInfo of partition.entered) { + const route = (enteredRouteInfo as any).route; + if (!route) continue; + + if ( + route.manager?.capabilities?.classicInterop && + (enteredRouteInfo as any).context !== undefined && + route.bucket + ) { + (route.bucket as any).context = (enteredRouteInfo as any).context; + } + route.manager.didEnter(route.bucket, { + transition: activeTransition, + enter: true, + } as any); + } + + for (const updatedRouteInfo of partition.updatedContext) { + const route = (updatedRouteInfo as any).route; + if (!route) continue; + + if ( + route.manager?.capabilities?.classicInterop && + (updatedRouteInfo as any).context !== undefined && + route.bucket + ) { + (route.bucket as any).context = (updatedRouteInfo as any).context; + } + route.manager.didEnter(route.bucket, { + transition: activeTransition, + enter: false, + } as any); + } + } catch (error) { + // Roll back state to the pre-transition values so the next transition + // sees these routes as still-unentered and re-fires their enter hooks. + // Mirrors classic setupContexts' rollback behaviour on throw. + // Copy the routeInfos array (don't alias) so subsequent transitions + // that mutate currentRouteInfos via onRouteInvokableReady don't also + // write back into preTransitionState.routeInfos, corrupting the state + // baseline used by partitionRoutes for the next transition. + this.state = preTransitionState; + this.currentRouteInfos = preTransitionState + ? (preTransitionState.routeInfos.slice() as any) + : undefined; + const errorRoute = (newState.routeInfos[newState.routeInfos.length - 1] as any)?.route; + const reason = this.transitionDidError( + { error, route: errorRoute, wasAborted: false } as any, + activeTransition + ); + throw reason; + } + + // If a route's enter or setup hook redirected (called router.transitionTo), + // the original transition is now aborted. Reject so the original handleURL/ + // transitionTo promise rejects with a TransitionAborted, mirroring main's + // setupContexts post-check before _updateURL fires. + if (activeTransition.isAborted) { + throw logAbort(activeTransition); + } + + // didExit on routes that left the hierarchy. + for (const exitedRouteInfo of partition.exited) { + const route = (exitedRouteInfo as any).route; + if (route?.manager) { + route.manager.didExit(route.bucket, { transition: activeTransition } as any); + } + } + + // Replace currentRouteInfos with the authoritative settled list. + this.currentRouteInfos = newState.routeInfos.slice(); + + // Finalize query params with the settled route infos. + this.state!.queryParams = this.finalizeQueryParamChange( + this.currentRouteInfos!, + newState.queryParams, + activeTransition + ); + + this._updateURL(activeTransition, newState); + + activeTransition.isActive = false; + this.activeTransition = undefined; + + this.triggerEvent(this.currentRouteInfos!, true, 'didTransition', []); + this.didTransition(this.currentRouteInfos!); + this.toInfos(activeTransition, newState.routeInfos, true); + this.routeDidChange(activeTransition); + + // Resolve the transition's promise with the leaf route to mirror the + // original router_js finalizeTransition contract that tests rely on. + return (newState.routeInfos[newState.routeInfos.length - 1] as any)?.route; + }); + } } +// Loose alias for InternalRouteInfo to keep TestRouter signatures legible. +type InternalRouteInfoLike = RouteInfo; + export function createHandlerInfo(name: string, options: Dict = {}): RouteInfo { class Stub extends RouteInfo { constructor(name: string, router: Router, handler?: Route) { @@ -115,8 +502,12 @@ export function createHandlerInfo(name: string, options: Dict = {}): Ro } } - let handler = (options['handler'] as Route) || createHandler('foo'); + // Tests pass the route under either `handler` or `route`; both are valid + // ergonomics. Pull whichever is set, fall back to a fresh handler. + let handler = + (options['handler'] as Route) || (options['route'] as Route) || createHandler('foo'); delete options['handler']; + delete options['route']; Object.assign(Stub.prototype, options); let stub = new Stub(name, new TestRouter(), handler); From 496807de52ff606c67c6a882334038f7a16218c3 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Mon, 11 May 2026 14:54:35 +0100 Subject: [PATCH 19/20] - moves enterPromise from bucket to route info - curries route wrapper args at outlet level --- .../glimmer/lib/component-managers/outlet.ts | 10 + .../-internals/glimmer/lib/syntax/outlet.ts | 147 ++++++++---- .../-internals/glimmer/lib/utils/outlet.ts | 34 ++- .../route-managers/classic-route-manager.ts | 217 ++++++++++++------ .../route-managers/classic-route-wrapper.gts | 61 +++++ .../routing/route-managers/route-manager.ts | 20 +- .../routing/route-managers/utils.ts | 5 - packages/@ember/routing/router.ts | 86 ++++--- packages/router_js/lib/route-info.ts | 83 +++---- packages/router_js/lib/router.ts | 9 + packages/router_js/lib/transition.ts | 12 +- packages/router_js/tests/test_helpers.ts | 73 +++--- 12 files changed, 515 insertions(+), 242 deletions(-) create mode 100644 packages/@ember/-internals/routing/route-managers/classic-route-wrapper.gts diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts b/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts index 9c932797fa2..b84b8b9820d 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts @@ -24,6 +24,7 @@ import { unwrapTemplate } from './unwrap-template'; import type { DynamicScope } from '../renderer'; import type { OutletState } from '../utils/outlet'; import type OutletView from '../views/outlet'; +import { RouteInfo } from 'router_js'; function instrumentationPayload(def: OutletDefinitionState) { // "main" used to be the outlet name, keeping it around for compatibility @@ -41,7 +42,16 @@ interface OutletInstanceState { export interface OutletDefinitionState { ref: Reference; name: string; + // The thing the outlet renders. For wrapper-driven routes this is the + // invokable returned from `manager.getInvokable` (used as `@Component` on + // the wrapper). For the legacy OutletState path (raw template from + // `setOutletState`) it is the makeRouteTemplate-wrapped component. template: object; + // Set on the wrapper-driven path. The outlet helper curries `@Component` + // (the invokable) and `@routeInfo` onto `wrapper` at render time. + wrapper?: object; + invokable?: object; + routeInfo?: RouteInfo; } const CAPABILITIES: InternalComponentCapabilities = { diff --git a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts index 287eaffaed7..0794b5b10fd 100644 --- a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts @@ -8,9 +8,14 @@ import type { Template, } from '@glimmer/interfaces'; import type { Reference } from '@glimmer/reference'; -import { createComputeRef, createConstRef, valueForRef } from '@glimmer/reference'; +import { + childRefFromParts, + createComputeRef, + createConstRef, + valueForRef, +} from '@glimmer/reference'; import type { CurriedValue } from '@glimmer/runtime'; -import { createCapturedArgs, curry, EMPTY_POSITIONAL, isCurriedValue } from '@glimmer/runtime'; +import { createCapturedArgs, curry, EMPTY_POSITIONAL } from '@glimmer/runtime'; import { dict } from '@glimmer/util'; import { hasInternalComponentManager } from '@glimmer/manager'; import { OutletComponent, type OutletDefinitionState } from '../component-managers/outlet'; @@ -82,51 +87,90 @@ export const outletHelper = internalHelper( let named = dict(); - // When the route manager provides the invokable, state.template is - // already a curried component with @controller and @model baked in. - // For backward compatibility (e.g. setOutletState from test helpers), - // raw templates and components are wrapped with makeRouteTemplate. - let template = state.template; let component: object; - if (isCurriedValue(template) || hasInternalComponentManager(template)) { - component = template; + // Wrapper-driven path: manager-provided wrapper + uncurried invokable. + // Curry the wrapper with @Component (the user's invokable), @routeInfo, + // @model, and @controller. The wrapper template uses these to render + // the route. + if (state.wrapper !== undefined && state.invokable !== undefined) { + let wrapperArgs = dict(); + wrapperArgs['Component'] = createConstRef(state.invokable, '@Component'); + wrapperArgs['routeInfo'] = createConstRef(state.routeInfo as object, '@routeInfo'); + + // @controller must be a const ref because RouteTemplateManager + // uses it as the route template's `self`, which is then passed + // as `caller` to inner internal components (LinkTo, Input, etc.) + // and those assert isConstRef(caller). Resolve the controller + // eagerly here from the route on the routeInfo. The route has an + // idempotent _initController that creates or returns the cached + // controller instance. + let routeForController = (state.routeInfo as { route?: any } | undefined)?.route; + let controller = routeForController?._initController?.(); + wrapperArgs['controller'] = createConstRef(controller, '@controller'); + + // @model is a compute ref over outletRef.render.routeInfo.context. + // The path-based ref consumes outletStateTag; when setOutletState + // dirties the tag (each transition / model update), the ref + // invalidates and re-reads the new context. Mirrors main's + // childRefFromParts(outletRef, ['render', 'model']) pattern with + // an extra hop for routeInfo. + let modelRef = childRefFromParts(outletRef, ['render', 'routeInfo', 'context']); + let model = valueForRef(modelRef); + let frozenState = state; + wrapperArgs['model'] = createComputeRef(() => { + if (lastState === frozenState) { + model = valueForRef(modelRef); + } + return model; + }); + + // isResolved=false because the wrapper is a "definition state" that + // the VM must resolve to a ComponentDefinition via its registered + // setComponentTemplate / setInternalComponentManager metadata. + component = curry( + 0 as CurriedComponent, + state.wrapper, + outletOwner, + createCapturedArgs(wrapperArgs, EMPTY_POSITIONAL), + false + ); } else { - if (DEBUG) { - // We don't appear to have a standard way or a brand to check, but for the - // purpose of avoiding obvious user errors, this probably gets you close - // enough. - let isTemplate = (template: unknown): template is Template => { - if (template === null || typeof template !== 'object') { - return false; - } else { - let t = template as Partial