diff --git a/packages/@ember/-internals/routing/route-managers/classic/manager.ts b/packages/@ember/-internals/routing/route-managers/classic/manager.ts index da689c847cd..1627f4209bc 100644 --- a/packages/@ember/-internals/routing/route-managers/classic/manager.ts +++ b/packages/@ember/-internals/routing/route-managers/classic/manager.ts @@ -38,12 +38,7 @@ import { finalizeQueryParamChange as finalizeClassicQueryParamChange, queryParamsDidChange as classicQueryParamsDidChange, } from './query-params'; -import { - type ActiveTransition, - enterErrorSubstate as enterClassicErrorSubstate, - enterLoadingSubstate as enterClassicLoadingSubstate, - fireLoadingEvent, -} from './substates'; +import { type ActiveTransition, findSubstateName } from './substates'; type TransitionLike = Transition & { isAborted?: boolean; @@ -63,7 +58,6 @@ function isTransitionObject(value: unknown): boolean { export class ClassicRouteManager implements RouteManagerWithClassicInterop { capabilities: RouteCapabilities = routeCapabilities('1.0', { classicInterop: true, - awaitEnter: true, }); #owner: Owner; @@ -106,11 +100,11 @@ export class ClassicRouteManager implements RouteManagerWithClassicInterop { + return RSVPPromise.resolve(buildClassicInvokable(bucket)); } qp(bucket: ClassicRouteBucket): QueryParamMeta { @@ -304,30 +298,56 @@ export class ClassicRouteManager implements RouteManagerWithClassicInterop` or `*.` route - matching the route currently resolving (or erroring) and triggers an - intermediate transition into it. + matching the route currently resolving (or erroring) and returns its name. + Entering the substate is the manager's job. Mirrors the original `defaultActionHandlers.loading` and `defaultActionHandlers.error` + `forEachRouteAbove` machinery that lived @@ -14,14 +14,24 @@ import { assert } from '@ember/debug'; import type Owner from '@ember/-internals/owner'; import { getOwner } from '@ember/-internals/owner'; import type Route from '@ember/routing/route'; -import type EmberRouter from '@ember/routing/router'; import type { InternalRouteInfo } from 'router_js'; -import { getRouteManagement, hasClassicInterop, STATE_SYMBOL } from 'router_js'; -import type { ClassicRouteBucket } from './bucket'; +import { hasClassicInterop, STATE_SYMBOL } from 'router_js'; + +// Substates are classic only. A classic route has a `foo.loading` +// sibling, and only it carries the owner and names the lookup needs. +function classicRouteFor(routeInfo: InternalRouteInfo): Route | undefined { + const { manager, bucket } = routeInfo; + + if (manager === undefined || bucket === undefined || !hasClassicInterop(manager)) { + return undefined; + } + + return manager.getRoute(bucket) as Route; +} export type ActiveTransition = { isActive: boolean; - pivotHandler?: unknown; + pivotBucket?: unknown; trigger?(ignoreFailure: boolean, name: string, ...args: unknown[]): void; [STATE_SYMBOL]?: { routeInfos: InternalRouteInfo[] }; }; @@ -39,11 +49,6 @@ function findRouteSubstateName(route: Route, state: string) { let owner = getOwner(route); assert('Route is unexpectedly missing an owner', owner); - let managed = getRouteManagement(route); - if (managed === undefined || !hasClassicInterop(managed.manager)) { - return ''; - } - let { routeName, fullRouteName, _router: router } = route; let substateName = `${routeName}_${state}`; @@ -66,11 +71,6 @@ function findRouteStateName(route: Route, state: string) { let owner = getOwner(route); assert('Route is unexpectedly missing an owner', owner); - let managed = getRouteManagement(route); - if (managed === undefined || !hasClassicInterop(managed.manager)) { - return ''; - } - let { routeName, fullRouteName, _router: router } = route; let stateName = routeName === 'application' ? state : `${routeName}.${state}`; @@ -97,94 +97,6 @@ function routeHasBeenDefined(owner: Owner, router: any, localName: string, fullN return routerHasRoute && ownerHasRoute; } -/** - Fires the classic `loading` event for a slow transition. The event bubbles - through each route's `actions.loading` handler (public API — apps intercept - it for custom loading UI, or return `true` to keep bubbling); only if it - bubbles unhandled does the router's default `loading` action handler - dispatch back through `ClassicRouteManager.enterLoadingSubstate` to enter - the substate. Scheduled by the manager's `willEnter`; no-op if the - transition is no longer active by the time the timer fires. - - @private - @param {ClassicRouteBucket} bucket - @param {Transition} transition - */ -export function fireLoadingEvent(bucket: ClassicRouteBucket, transition: ActiveTransition): void { - if (!transition.isActive) { - return; - } - - transition.trigger?.(true, 'loading', transition, bucket.route); -} - -/** - Look up the `loading` substate (if any) for the route that is loading - slowly and trigger an intermediate transition into it. No-op if the - transition is no longer active or no matching substate exists. - - Reached via `ClassicRouteManager.enterLoadingSubstate`, which the router's - default `loading` action handler dispatches to through the classic-interop - contract once the loading event has bubbled unhandled. - - @private - @param {EmberRouter} router - @param {Route|undefined} originRoute the route whose model is slow; - `undefined` when that route was never created (the walk then starts at - the transition's leaf) - @param {Transition} transition - */ -export function enterLoadingSubstate( - router: EmberRouter, - originRoute: Route | undefined, - transition: ActiveTransition -): void { - if (!transition.isActive) { - return; - } - - const substateName = findSubstateName(originRoute, transition, 'loading'); - if (substateName) { - router.intermediateTransitionTo(substateName); - } -} - -/** - Look up the `error` substate (if any) for the route that errored and - trigger an intermediate transition into it, passing the error along so the - error route's `model` hook receives it. Returns `true` if a substate was - entered (and the error should be considered handled), `false` otherwise. - - Reached via `ClassicRouteManager.enterErrorSubstate`, which the router's - default `error` action handler dispatches to through the classic-interop - contract once the error has bubbled unhandled above the application route. - - @private - @param {EmberRouter} router - @param {Route|undefined} originRoute the route that errored; `undefined` - when the erroring route never got created (the walk then starts at the - transition's leaf) - @param {Transition} transition - @param {Error} error the error that triggered this substate transition - */ -export function enterErrorSubstate( - router: EmberRouter, - originRoute: Route | undefined, - transition: ActiveTransition, - error: Error -): boolean { - const substateName = findSubstateName(originRoute, transition, 'error'); - if (!substateName) { - return false; - } - - // Mark the error handled before transitioning so it is not re-raised - // after the substate has taken over rendering it. - router._markErrorAsHandled(error); - router.intermediateTransitionTo(substateName, error); - return true; -} - /** Walk up from the route currently being resolved (or erroring) through the transition's route hierarchy, returning the name of the closest matching @@ -209,13 +121,13 @@ export function enterErrorSubstate( @param {Transition} transition the active transition @param {String} state the substate to look for, e.g. `loading` or `error` */ -function findSubstateName( +export function findSubstateName( originRoute: Route | undefined, transition: ActiveTransition, state: 'loading' | 'error' ): string { const routeInfos = transition[STATE_SYMBOL]?.routeInfos ?? []; - const pivotHandler = transition.pivotHandler; + const pivotBucket = transition.pivotBucket; const originIndex = originRoute === undefined @@ -226,7 +138,9 @@ function findSubstateName( for (let i = startIndex; i >= 0; i--) { const ancestorRouteInfo = routeInfos[i]; - const ancestorRoute = ancestorRouteInfo?.route; + if (ancestorRouteInfo === undefined) continue; + + const ancestorRoute = classicRouteFor(ancestorRouteInfo); if (!ancestorRoute) continue; if (ancestorRouteInfo !== originRouteInfo) { @@ -237,7 +151,12 @@ function findSubstateName( const substateName = findRouteSubstateName(ancestorRoute, state); if (substateName) return substateName; - if (state === 'loading' && pivotHandler === ancestorRoute) break; + if ( + state === 'loading' && + pivotBucket !== undefined && + pivotBucket === ancestorRouteInfo.bucket + ) + break; } return ''; diff --git a/packages/@ember/-internals/routing/route-managers/outlet-state.ts b/packages/@ember/-internals/routing/route-managers/outlet-state.ts index 0dd3b5fdb27..32f000abf22 100644 --- a/packages/@ember/-internals/routing/route-managers/outlet-state.ts +++ b/packages/@ember/-internals/routing/route-managers/outlet-state.ts @@ -1,6 +1,10 @@ -import type { BaseRoute, InternalRouteInfo } from 'router_js'; +import { invokableFor, type BaseRoute, type InternalRouteInfo } from 'router_js'; import { tracked } from '@ember/-internals/metal/lib/tracked'; +function isPromise(value: object): value is Promise { + return 'then' in value && typeof value.then === 'function'; +} + /** * What the outlet walk descends from. */ @@ -25,6 +29,8 @@ export interface OutletParent { export class OutletState implements OutletParent { @tracked context: unknown; + @tracked invokable: object | undefined; + readonly outlets: { main: OutletState | undefined; } = { @@ -36,12 +42,29 @@ export class OutletState implements OutletParent { } constructor( - readonly manager: { getRouteWrapper(): object; getInvokable(bucket: object): object }, + readonly manager: { + getRouteWrapper(): object; + getInvokable(bucket: object): Promise; + }, readonly bucket: object, readonly routeInfo: InternalRouteInfo ) { this.context = routeInfo.context; + let invokable = invokableFor(manager, bucket); + if (isPromise(invokable)) { + invokable.then( + (invokable) => { + this.invokable = invokable; + }, + () => { + // getInvokable rejected; this level renders nothing. + } + ); + } else { + this.invokable = invokable; + } + routeInfo.enterPromise?.then( () => { this.context = routeInfo.context; diff --git a/packages/@ember/-internals/routing/route-managers/root-outlet.ts b/packages/@ember/-internals/routing/route-managers/root-outlet.ts index 0748ac75f18..df69300fa52 100644 --- a/packages/@ember/-internals/routing/route-managers/root-outlet.ts +++ b/packages/@ember/-internals/routing/route-managers/root-outlet.ts @@ -127,7 +127,7 @@ const asReference = internalHelper( It's role is to enforce the shape of outlet. */ const PROVIDER_TEMPLATE = precompileTemplate( - '', + '', { moduleName: 'packages/@ember/-internals/routing/route-managers/outlet-arg-provider.hbs', strictMode: true, diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 8f7497a1d21..9a9290998cb 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -303,9 +303,6 @@ class Route extends EmberObject.extend(ActionHandler) implement /** @internal */ _bucketCache!: BucketCache; - /** @internal */ - _internalName!: string; - private _names: unknown; _router!: EmberRouter; diff --git a/packages/@ember/routing/router-service.ts b/packages/@ember/routing/router-service.ts index 81645762229..e97eed15155 100644 --- a/packages/@ember/routing/router-service.ts +++ b/packages/@ember/routing/router-service.ts @@ -695,11 +695,7 @@ class RouterService extends Service { assert(`The route "${pivotRouteName}" was not found`, this._router.hasRoute(pivotRouteName)); assert(`The route "${pivotRouteName}" is currently not active`, this.isActive(pivotRouteName)); - let owner = getOwner(this); - assert('RouterService is unexpectedly missing an owner', owner); - let pivotRoute = owner.lookup(`route:${pivotRouteName}`) as Route; - - return this._router._routerMicrolib.refresh(pivotRoute); + return this._router._routerMicrolib.refresh(pivotRouteName); } /** diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index 6485b615184..bc996016dba 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -217,6 +217,8 @@ class EmberRouter extends EmberObject { #routeManagement = new WeakMap>(); #routeManagerInstances = new WeakMap>(); + #inaccessibleByURL = new Map(); + private namespace: any; // Begin Evented @@ -464,9 +466,11 @@ class EmberRouter extends EmberObject { ownerRouteManagement.set(routeName, managed); } - const route = managed.manager.getRoute(managed.bucket); + const route = hasClassicInterop(managed.manager) + ? managed.manager.getRoute(managed.bucket) + : managed.bucket; - // Register the route → {manager, bucket} association that router_js + // Register the handle → {manager, bucket} association that router_js // dispatches lifecycle hooks through. Owned by the router so managers // don't have to stamp anything onto their route objects. Registered on // every call (cheap WeakMap set) because a manager may instantiate its @@ -475,9 +479,20 @@ class EmberRouter extends EmberObject { associateRouteManagement(route, managed.manager, managed.bucket); } + if (hasClassicInterop(managed.manager)) { + this.#inaccessibleByURL.set( + name, + Boolean((route as { inaccessibleByURL?: boolean } | undefined)?.inaccessibleByURL) + ); + } + return route; } + isRouteInaccessibleByURL(name: string): boolean { + return this.#inaccessibleByURL.get(name) ?? false; + } + _initRouterJs(): void { let location = get(this, 'location') as EmberLocation; let router = this; @@ -498,6 +513,10 @@ class EmberRouter extends EmberObject { return route as BaseRoute; } + isRouteInaccessibleByURL(name: string) { + return router.isRouteInaccessibleByURL(name); + } + getSerializer(name: string) { let engineInfo = router._engineInfoByRoute[name]; @@ -579,7 +598,13 @@ class EmberRouter extends EmberObject { } else { // Otherwise trigger the "error" event to attempt an intermediate // transition into an error substate - transition.trigger(false, 'error', error.error, transition, error.route); + let dispatch = dispatchRouteInfoFor(transition[STATE_SYMBOL]?.routeInfos); + let manager = dispatch?.manager; + let bucket = dispatch?.bucket; + + if (manager && bucket && hasClassicInterop(manager)) { + manager.triggerErrorEvent(bucket, transition, error.error, error.route); + } if (router._isErrorHandled(error.error)) { // If we handled the error with a substate just roll the state back on // the transition and send the "routeDidChange" event for landing on @@ -801,15 +826,6 @@ class EmberRouter extends EmberObject { let { manager, bucket } = routeInfo; assert('Expected active route to have a manager and bucket', manager && bucket); - // @TODO: could be cached here but currently OutletState is rebuilt each `_setOutletPass`. - // So we save on the `bucket` instead - // @TODO: bucket is otherwise opaque and it feels weird to mutate a manager owned object. - // But invokable caching is ultimately not the managers' concern right now - let mutableBucket = bucket as any; - if (!mutableBucket.invokable) { - const invokable = manager.getInvokable(bucket); - mutableBucket.invokable = invokable; - } let state = new OutletState(manager, bucket, routeInfo); if (parent) { @@ -1655,17 +1671,12 @@ let defaultActionHandlers = { // the manager owns substate entry. loading(this: EmberRouter, routeInfos: InternalRouteInfo[], transition: Transition) { let originRoute = routeInfos[routeInfos.length - 1]?.route; + let dispatch = dispatchRouteInfoFor(routeInfos); + let manager = dispatch?.manager; + let bucket = dispatch?.bucket; - let dispatchRoute = originRoute; - for (let i = routeInfos.length - 2; dispatchRoute === undefined && i >= 0; i--) { - dispatchRoute = routeInfos[i]?.route; - } - - if (dispatchRoute !== undefined) { - let managed = getRouteManagement(dispatchRoute); - if (managed !== undefined && hasClassicInterop(managed.manager)) { - managed.manager.enterLoadingSubstate(managed.bucket, transition, originRoute); - } + if (manager && bucket && hasClassicInterop(manager)) { + manager.handleLoadingEvent(bucket, transition, originRoute); } }, @@ -1681,19 +1692,16 @@ let defaultActionHandlers = { // routeInfos are sliced to end at the route that errored, so its leaf // is the origin of the substate walk. That route may never have been // created (e.g. across an engine's async boundary) — dispatch then - // falls to the deepest created route, and the manager walks from the - // transition's leaf. + // falls to the deepest route with a classic manager, and the manager + // walks from the transition's leaf. let originRoute = routeInfos[routeInfos.length - 1]?.route; + let dispatch = dispatchRouteInfoFor(routeInfos); + let manager = dispatch?.manager; + let bucket = dispatch?.bucket; - let dispatchRoute = originRoute; - for (let i = routeInfos.length - 2; dispatchRoute === undefined && i >= 0; i--) { - dispatchRoute = routeInfos[i]?.route; - } - - if (dispatchRoute !== undefined) { - let managed = getRouteManagement(dispatchRoute); - if (managed !== undefined && hasClassicInterop(managed.manager)) { - managed.manager.enterErrorSubstate(managed.bucket, transition, error, originRoute); + if (manager && bucket && hasClassicInterop(manager)) { + if (manager.handleErrorEvent(bucket, transition, error, originRoute)) { + this._markErrorAsHandled(error); } } @@ -1701,6 +1709,21 @@ let defaultActionHandlers = { }, }; +function dispatchRouteInfoFor( + routeInfos: InternalRouteInfo[] | undefined +): InternalRouteInfo | undefined { + if (routeInfos === undefined) { + return undefined; + } + + let i = routeInfos.length - 1; + while (i >= 0 && !routeInfos[i]?.route) { + i--; + } + + return routeInfos[i]; +} + function logError(_error: any, initialMessage: string) { let errorArgs = []; let error; diff --git a/packages/ember/tests/routing/decoupled_basic_test.js b/packages/ember/tests/routing/decoupled_basic_test.js index 08e54a0bccc..c404f8dcd88 100644 --- a/packages/ember/tests/routing/decoupled_basic_test.js +++ b/packages/ember/tests/routing/decoupled_basic_test.js @@ -85,6 +85,22 @@ moduleFor( await assert.rejects(this.visit('/what-is-this-i-dont-even'), /\/what-is-this-i-dont-even/); } + async ['@test a classic route marked inaccessibleByURL cannot be entered by URL'](assert) { + this.router.map(function () { + this.route('secret'); + }); + this.add( + 'route:secret', + class extends Route { + inaccessibleByURL = true; + } + ); + + await this.visit('/'); + + await assert.rejects(this.visit('/secret'), /\/secret/); + } + ['@test The Homepage'](assert) { return this.visit('/').then(() => { assert.equal(this.appRouter.currentPath, 'home', 'currently on the home route'); diff --git a/packages/ember/tests/routing/route_manager_test.js b/packages/ember/tests/routing/route_manager_test.js index b47883fddbd..cff978318ed 100644 --- a/packages/ember/tests/routing/route_manager_test.js +++ b/packages/ember/tests/routing/route_manager_test.js @@ -38,7 +38,7 @@ class RecordingRouteManager extends ClassicRouteManager { return bucket; } - getInvokable(bucket,) { + getInvokable(bucket) { this.log.push(['getInvokable', bucket.route.routeName]); return super.getInvokable(bucket); } diff --git a/packages/router_js/index.ts b/packages/router_js/index.ts index 1da17994310..aaaaa44d1a7 100644 --- a/packages/router_js/index.ts +++ b/packages/router_js/index.ts @@ -25,6 +25,7 @@ export { hasClassicInterop, associateRouteManagement, getRouteManagement, + invokableFor, } from './lib/route-manager'; export type { RouteManager, diff --git a/packages/router_js/lib/route-info.ts b/packages/router_js/lib/route-info.ts index 915c5d40267..29c0ab6b85a 100644 --- a/packages/router_js/lib/route-info.ts +++ b/packages/router_js/lib/route-info.ts @@ -13,8 +13,8 @@ import { } from './transition'; import { isParam, isPromise, merge } from './utils'; import { throwIfAborted } from './transition-aborted-error'; -import type { EnterState, RouteManager, RouteStateBucket } from './route-manager'; -import { getRouteManagement, hasClassicInterop } from './route-manager'; +import type { EnterState, RouteManagement, RouteManager, RouteStateBucket } from './route-manager'; +import { getRouteManagement, hasClassicInterop, invokableFor } from './route-manager'; export type IModel = {} & { id?: string | number; @@ -24,21 +24,12 @@ export type ModelFor = T extends BaseRoute ? V : never; export interface BaseRoute { context: T | undefined; - - // this is used to identify the route in router_js machinery, and is not the same as the - // routeName property on classic ember routes. It is totally internal to router_js, and - // not to be confused with the routeName property on classic ember routes - _internalName?: string; - - // I think this could potentially be deleted - // it's not mentioned in any ember docs that I can find, and is only - // used in a couple of places in router_js - inaccessibleByURL?: boolean; } // used by old router_js tests that expect to be working with the classic ember routes export interface ClassicRoute extends BaseRoute { routeName: string; + inaccessibleByURL?: boolean; events?: Dict<(...args: unknown[]) => unknown>; model?(params: Dict, transition: Transition): PromiseLike | undefined | T; deserialize?(params: Dict, transition: Transition): T | PromiseLike | undefined; @@ -237,6 +228,7 @@ function attachMetadata(info: InternalRouteInfo, routeInfo: RouteInfo export default class InternalRouteInfo { private _routePromise?: Promise = undefined; private _route?: Option = null; + private _management?: RouteManagement = undefined; protected router: Router; declare paramNames: string[]; declare name: string; @@ -245,6 +237,8 @@ export default class InternalRouteInfo { declare context?: ModelFor | PromiseLike> | undefined; isResolved = false; enterPromise?: globalThis.Promise = undefined; + private beginPromise?: Promise = undefined; + private beginTransition?: InternalTransition = undefined; constructor(router: Router, name: string, paramNames: string[], route?: R) { this.name = name; @@ -263,8 +257,22 @@ export default class InternalRouteInfo { return this.params || {}; } - resolve(transition: InternalTransition): Promise> { - return Promise.resolve(this.routePromise) + beginEnter(transition: InternalTransition, eager = false): Promise { + if (eager) { + const eagerManager = this._management?.manager; + + // Classic keeps the sequential walk, so its legacy timings are exact. + if (this.isResolved || eagerManager === undefined || hasClassicInterop(eagerManager)) { + return Promise.resolve(undefined); + } + } + + if (this.beginPromise !== undefined && this.beginTransition === transition) { + return this.beginPromise; + } + + this.beginTransition = transition; + this.beginPromise = Promise.resolve(this.routePromise) .then((route: R) => { throwIfAborted(transition); return route; @@ -292,7 +300,7 @@ export default class InternalRouteInfo { to, cancel: () => transition.abort(), signal: transition.signal, - getAncestorContext: (ancestor: RouteInfo) => { + getAncestorPromise: (ancestor: RouteInfo) => { const routeInfos = transition[STATE_SYMBOL]?.routeInfos ?? []; // Only true ancestors count: searching the whole hierarchy would // hand a route its own (or a descendant's) pending enter promise — @@ -317,31 +325,20 @@ export default class InternalRouteInfo { const enterPromise = manager.enter(bucket, navigationArgs); this.enterPromise = enterPromise; - // Capture the entered context locally rather than writing it onto - // this route info: `shouldSupersede` treats an own `context` as - // meaningful when infos are reused across transitions, so the info - // must not gain one it never had. `becomeResolved` receives the - // value explicitly below. - let enteredContext: ModelFor | undefined; - enterPromise.then( - (resolvedContext) => { - if (transition.isAborted) return; - enteredContext = resolvedContext as ModelFor | undefined; - }, - () => { - // Swallow rejections; transition-level error handling reports them. - } - ); - - const awaitEnter = manager.capabilities.awaitEnter ? enterPromise : Promise.resolve(); - return awaitEnter.then(() => { - throwIfAborted(transition); - const resolvedContext = enteredContext ?? (this.context as ModelFor | undefined); - const resolved = this.becomeResolved(transition, resolvedContext); + invokableFor(manager, bucket); - return resolved; - }); + return enterPromise; }); + + return this.beginPromise; + } + + resolve(transition: InternalTransition): Promise> { + return this.beginEnter(transition).then((enteredContext) => { + throwIfAborted(transition); + + return this.becomeResolved(transition, enteredContext as ModelFor | undefined); + }); } becomeResolved( @@ -439,20 +436,28 @@ export default class InternalRouteInfo { return this.fetchRoute(); } - /** - The manager driving this route's lifecycle, read from the association the - framework router registered via `associateRouteManagement` when it resolved - the route. `undefined` until the route has loaded. - */ + // Reading before the route has loaded forces the load, matching `route`. + private get management(): RouteManagement | undefined { + if (this._management === undefined) { + let route = this.route; + if (route !== undefined) { + this._management = getRouteManagement(route); + } + } + + return this._management; + } + get manager(): RouteManager | undefined { - let route = this.route; - return route === undefined ? undefined : getRouteManagement(route)?.manager; + return this.management?.manager; } - /** The manager's bucket for this route. `undefined` until the route has loaded. */ get bucket(): RouteStateBucket | undefined { - let route = this.route; - return route === undefined ? undefined : getRouteManagement(route)?.bucket; + return this.management?.bucket; + } + + get inaccessibleByURL(): boolean { + return this.router.isRouteInaccessibleByURL(this.name); } set route(route: R | undefined) { @@ -480,7 +485,7 @@ export default class InternalRouteInfo { } private updateRoute(route: R) { - route._internalName = this.name; + this._management = getRouteManagement(route); return (this.route = route); } diff --git a/packages/router_js/lib/route-manager.ts b/packages/router_js/lib/route-manager.ts index 686c30059ff..c50dde10564 100644 --- a/packages/router_js/lib/route-manager.ts +++ b/packages/router_js/lib/route-manager.ts @@ -46,15 +46,9 @@ export interface RouteCapabilitiesVersions { surface. It is not intended to be used by managers outside the framework-provided `ClassicRouteManager`. */ - '1.0': - | { - classicInterop: true; - awaitEnter: boolean; - } - | { - classicInterop?: false; - awaitEnter?: boolean; - }; + '1.0': { + classicInterop?: boolean; + }; } /** @@ -63,7 +57,6 @@ export interface RouteCapabilitiesVersions { */ export interface RouteCapabilities { classicInterop: boolean; - awaitEnter: boolean; } /** @@ -71,7 +64,7 @@ export interface RouteCapabilities { be assigned to `manager.capabilities`. ```ts - capabilities = routeCapabilities('1.0', { classicInterop: true, awaitEnter: true }); + capabilities = routeCapabilities('1.0', { classicInterop: true }); ``` @param _managerAPI The version of the manager API the route manager targets. @@ -83,7 +76,6 @@ export function routeCapabilities | object; + +const INVOKABLES = new WeakMap(); +export function invokableFor( + manager: { getInvokable(bucket: B): globalThis.Promise }, + bucket: B +): Invokable { + let invokable = INVOKABLES.get(bucket); + if (invokable === undefined) { + let promise = manager.getInvokable(bucket); + invokable = promise; + INVOKABLES.set(bucket, invokable); + promise.then( + (value) => INVOKABLES.set(bucket, value), + () => {} + ); + } + return invokable; +} + // -- Navigation state --------------------------------------------------------- /** @@ -163,7 +175,7 @@ export interface AsyncNavigationState { A `RouteInfo` for the desired ancestor must always be passed explicitly. */ - getAncestorContext(routeInfo: RouteInfo): Promise; + getAncestorPromise(routeInfo: RouteInfo): Promise; } /** @@ -265,6 +277,7 @@ export interface CreateRouteArgs { The contract every route base class implements via its manager. The router drives this interface; nothing else in user code should. + @template Bucket The shape of the bucket the manager returns from `createRoute`. Defaults to the empty marker, override for a concrete manager implementation. @@ -283,11 +296,6 @@ export interface RouteManager; @@ -343,10 +351,9 @@ export interface RouteManager; } /** @@ -358,6 +365,12 @@ export interface RouteManager extends RouteManager { + /** + Returns the classic `Route` instance backing a bucket, for the legacy APIs + that still surface one (transition promise value, transition error route). + */ + getRoute(bucket: Bucket): unknown; + // Lifecycle hooks, widened with the capability-gated interop state. The // router narrows via `hasClassicInterop` before dispatching these shapes. willEnter(bucket: Bucket, state: ClassicWillEnterState): void; @@ -457,31 +470,13 @@ export interface RouteManagerWithClassicInterop< */ redirect(bucket: Bucket, routeInfo: RouteInfo, context: unknown, transition: Transition): void; - /** - Enters the classic `loading` substate for a slow transition: looks up - the nearest `*.loading`/`*_loading` route (stopping at the transition's - pivot) and intermediate-transitions into it. Called by the router's - default `loading` action handler once the loading event has bubbled - unhandled above the application route. - - `originRoute` is the route whose model is slow, or `undefined` when that - route was never created; typed `unknown` because router_js never - inspects route shapes. - */ - enterLoadingSubstate(bucket: Bucket, transition: Transition, originRoute: unknown): void; + triggerLoadingEvent(bucket: Bucket, transition: Transition): void; - /** - Enters the classic `error` substate for an error that bubbled unhandled - above the application route: looks up the nearest `*.error`/`*_error` - route and intermediate-transitions into it, passing the error as its - model. Returns `true` when a substate was entered and the error should - be treated as handled. - - `originRoute` is the route whose `enter` failed, or `undefined` when - that route was never created (e.g. across an engine's async boundary); - typed `unknown` because router_js never inspects route shapes. - */ - enterErrorSubstate( + triggerErrorEvent(bucket: Bucket, transition: Transition, error: Error, route: unknown): void; + + handleLoadingEvent(bucket: Bucket, transition: Transition, originRoute: unknown): void; + + handleErrorEvent( bucket: Bucket, transition: Transition, error: Error, diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index 24cc3ad14a8..8e5e5cc6884 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -65,6 +65,10 @@ export default abstract class Router { abstract routeDidChange(transition: Transition): void; abstract transitionDidError(error: TransitionError, transition: Transition): Transition | Error; + isRouteInaccessibleByURL(_routeName: string): boolean { + return false; + } + // -- Manager-driven transition lifecycle ------------------------------------- // // The three `on*` methods below drive the RouteManager lifecycle for every @@ -228,10 +232,10 @@ export default abstract class Router { // Filter exited routes out of currentRouteInfos. Truncating to // `unchanged.length` would lose entering routes that // `onRouteInvokableReady` already wrote at higher indices. - const exitedRouteObjects = new Set(partition.exited.map((ri) => ri.route)); + const exitedBuckets = new Set(partition.exited.map((ri) => ri.bucket)); if (this.currentRouteInfos) { this.currentRouteInfos = this.currentRouteInfos.filter( - (cri) => !exitedRouteObjects.has(cri.route) + (cri) => !exitedBuckets.has(cri.bucket) ); } @@ -724,7 +728,7 @@ export default abstract class Router { let oldRouteInfo = oldRouteInfos[i]!, newRouteInfo = newRouteInfos[i]!; - if (!oldRouteInfo || oldRouteInfo.route !== newRouteInfo.route) { + if (!oldRouteInfo || oldRouteInfo.bucket !== newRouteInfo.bucket) { routeChanged = true; } @@ -767,7 +771,7 @@ export default abstract class Router { for (let i = routeInfos.length - 1; i >= 0; --i) { let routeInfo = routeInfos[i]!; merge(params, routeInfo.params); - if (routeInfo.route!.inaccessibleByURL) { + if (routeInfo.inaccessibleByURL) { urlMethod = null; } } @@ -990,13 +994,18 @@ export default abstract class Router { return this.doTransition(name, args, true); } - refresh(pivotRoute?: R) { + refresh(pivot?: R | string) { let previousTransition = this.activeTransition; let state = previousTransition ? previousTransition[STATE_SYMBOL] : this.state; let routeInfos = state!.routeInfos; - if (pivotRoute === undefined) { - pivotRoute = routeInfos[0]!.route; + let pivotRouteName: string | undefined; + if (pivot === undefined) { + pivotRouteName = routeInfos[0]!.name; + } else if (typeof pivot === 'string') { + pivotRouteName = pivot; + } else { + pivotRouteName = routeInfos.find((routeInfo) => routeInfo.route === pivot)?.name; } log(this, 'Starting a refresh transition'); @@ -1004,7 +1013,7 @@ export default abstract class Router { let intent = new NamedTransitionIntent( this, name, - pivotRoute, + pivotRouteName, [], this._changedQueryParams || state!.queryParams ); diff --git a/packages/router_js/lib/transition-intent/named-transition-intent.ts b/packages/router_js/lib/transition-intent/named-transition-intent.ts index 34286abd7dd..d368738f5e7 100644 --- a/packages/router_js/lib/transition-intent/named-transition-intent.ts +++ b/packages/router_js/lib/transition-intent/named-transition-intent.ts @@ -11,7 +11,7 @@ import { isParam, merge } from '../utils'; export default class NamedTransitionIntent extends TransitionIntent { name: string; - pivotHandler?: BaseRoute; + pivotName?: string; contexts: ModelFor[]; queryParams: Dict; preTransitionState?: TransitionState = undefined; @@ -19,14 +19,14 @@ export default class NamedTransitionIntent extends Transiti constructor( router: Router, name: string, - pivotHandler: BaseRoute | undefined, + pivotName: string | undefined, contexts: ModelFor[] = [], queryParams: Dict = {}, data?: object ) { super(router, data); this.name = name; - this.pivotHandler = pivotHandler; + this.pivotName = pivotName; this.contexts = contexts; this.queryParams = queryParams; } @@ -52,10 +52,10 @@ export default class NamedTransitionIntent extends Transiti let invalidateIndex = parsedHandlers.length; - // Pivot handlers are provided for refresh transitions - if (this.pivotHandler) { + // Pivot names are provided for refresh transitions + if (this.pivotName !== undefined) { for (i = 0, len = parsedHandlers.length; i < len; ++i) { - if (parsedHandlers[i]!.handler === this.pivotHandler._internalName) { + if (parsedHandlers[i]!.handler === this.pivotName) { invalidateIndex = i; break; } diff --git a/packages/router_js/lib/transition-intent/url-transition-intent.ts b/packages/router_js/lib/transition-intent/url-transition-intent.ts index d04c0706cc5..b8249893823 100644 --- a/packages/router_js/lib/transition-intent/url-transition-intent.ts +++ b/packages/router_js/lib/transition-intent/url-transition-intent.ts @@ -1,4 +1,4 @@ -import type { BaseRoute } from '../route-info'; +import type { BaseRoute, default as InternalRouteInfo } from '../route-info'; import { UnresolvedRouteInfoByParam } from '../route-info'; import type Router from '../router'; import { TransitionIntent } from '../transition-intent'; @@ -28,15 +28,12 @@ export default class URLTransitionIntent extends Transition let statesDiffer = false; let _url = this.url; - // Checks if a handler is accessible by URL. If it is not, an error is thrown. - // For the case where the handler is loaded asynchronously, the error will be + // For the case where the route is loaded asynchronously, the error will be // thrown once it is loaded. - function checkHandlerAccessibility(handler: R) { - if (handler && handler.inaccessibleByURL) { + function checkAccessibility(routeInfo: InternalRouteInfo) { + if (routeInfo.inaccessibleByURL) { throw new UnrecognizedURLError(_url); } - - return handler; } for (i = 0, len = results.length; i < len; ++i) { @@ -58,11 +55,12 @@ export default class URLTransitionIntent extends Transition let route = newRouteInfo.route; if (route) { - checkHandlerAccessibility(route); + checkAccessibility(newRouteInfo); } else { - // If the handler is being loaded asynchronously, check if we can - // access it after it has resolved - newRouteInfo.routePromise = newRouteInfo.routePromise.then(checkHandlerAccessibility); + newRouteInfo.routePromise = newRouteInfo.routePromise.then((handler) => { + checkAccessibility(newRouteInfo); + return handler; + }); } let oldRouteInfo = oldState.routeInfos[i]!; diff --git a/packages/router_js/lib/transition-state.ts b/packages/router_js/lib/transition-state.ts index 056a2125703..2e86d3c70c3 100644 --- a/packages/router_js/lib/transition-state.ts +++ b/packages/router_js/lib/transition-state.ts @@ -113,6 +113,9 @@ export default class TransitionState { let params = this.params; forEach(this.routeInfos, (routeInfo) => { params[routeInfo.name] = routeInfo.params || {}; + routeInfo.beginEnter(transition, true).catch(() => { + // Surfaced by the sequential pass below. + }); return true; }); diff --git a/packages/router_js/lib/transition.ts b/packages/router_js/lib/transition.ts index 129b486c810..717809b8f69 100644 --- a/packages/router_js/lib/transition.ts +++ b/packages/router_js/lib/transition.ts @@ -57,7 +57,18 @@ export default class Transition implements Partial; routeInfos: InternalRouteInfo[]; targetName: Maybe; - pivotHandler: Maybe; + pivotBucket: Maybe; + + // The route behind `pivotBucket`. + get pivotHandler(): Maybe { + let bucket = this.pivotBucket; + + if (bucket === undefined) { + return undefined; + } + + return this.routeInfos.find((routeInfo) => routeInfo.bucket === bucket)?.route; + } sequence: number; isAborted = false; isActive = true; @@ -127,7 +138,7 @@ export default class Transition implements Partial implements Partial, finalParams: Dict[]) { (finalParams as any).foo = params['foo']; // TODO wat diff --git a/packages/router_js/tests/route_info_test.ts b/packages/router_js/tests/route_info_test.ts index 226004c7723..f7b63100ede 100644 --- a/packages/router_js/tests/route_info_test.ts +++ b/packages/router_js/tests/route_info_test.ts @@ -239,19 +239,15 @@ QUnit.test('RouteInfo.find returns matched', function (assert) { QUnit.module('RouteInfo - non-gating manager'); // Builds a handler whose manager mimics a manager that does not gate -// getInvokable on enterPromise, so a route becomes resolved and renders -// (e.g. a loading substate) before its `enter` settles with the context. -// The `enter` hook is supplied per test. +// getInvokable on enterPromise. The `enter` hook is supplied per test. function createNonGatingHandler( name: string, enter: (bucket: any, args: any) => Promise ): ClassicRoute { let manager = { - capabilities: { classicInterop: false, awaitEnter: false }, + capabilities: { classicInterop: false }, willEnter() {}, enter, - // Resolves immediately, without awaiting enterPromise. This is the - // behaviour that makes the context unavailable at becomeResolved time. getInvokable() { return resolve(undefined); }, @@ -280,14 +276,17 @@ QUnit.test( let transition = { isAborted: false } as unknown as InternalTransition; - let resolved = await routeInfo.resolve(transition); + let settled = false; + let pending = routeInfo.resolve(transition).then((resolvedRouteInfo) => { + settled = true; + return resolvedRouteInfo; + }); - // becomeResolved ran before `enter` settled, so the snapshot is empty. - assert.equal(resolved.context, undefined, 'context is undefined until enter settles'); + await resolve(); + assert.notOk(settled, 'resolve stays pending until enter settles'); resolveEnter(model); - await enterPromise; - await resolve(); + let resolved = await pending; assert.equal(resolved.context, model, 'resolved context syncs once enter settles'); assert.equal( @@ -298,7 +297,7 @@ QUnit.test( } ); -QUnit.test('getAncestorContext resolves with the ancestor enter result', async function (assert) { +QUnit.test('getAncestorPromise resolves with the ancestor enter result', async function (assert) { assert.expect(1); let router = new TestRouter(); @@ -314,19 +313,19 @@ QUnit.test('getAncestorContext resolves with the ancestor enter result', async f let captured: ((routeInfo: any) => Promise) | undefined; let handler = createNonGatingHandler('parent.child', (_bucket, args) => { - captured = args.getAncestorContext; + captured = args.getAncestorPromise; return resolve(undefined); }); let childInfo = new UnresolvedRouteInfoByParam(router, 'parent.child', [], {}, handler); let transition = { isAborted: false } as unknown as InternalTransition; - // Seed the transition state so getAncestorContext can find the ancestor. + // Seed the transition state so getAncestorPromise can find the ancestor. transition[STATE_SYMBOL] = { routeInfos: [ancestorInfo] } as never; await childInfo.resolve(transition); let result = await captured!({ name: 'parent' }); - assert.equal(result, ancestorModel, 'getAncestorContext resolves with the ancestor enter result'); + assert.equal(result, ancestorModel, 'getAncestorPromise resolves with the ancestor enter result'); }); QUnit.test( @@ -356,7 +355,7 @@ QUnit.test( } ); -QUnit.test('getAncestorContext only matches true ancestors', async function (assert) { +QUnit.test('getAncestorPromise only matches true ancestors', async function (assert) { assert.expect(2); let router = new TestRouter(); @@ -377,7 +376,7 @@ QUnit.test('getAncestorContext only matches true ancestors', async function (ass let captured: ((routeInfo: any) => Promise) | undefined; let handler = createNonGatingHandler('parent.child', (_bucket, args) => { - captured = args.getAncestorContext; + captured = args.getAncestorPromise; return resolve(undefined); }); let childInfo = new UnresolvedRouteInfoByParam(router, 'parent.child', [], {}, handler); diff --git a/packages/router_js/tests/router_test.ts b/packages/router_js/tests/router_test.ts index bc66df81d94..4ae0f8dabf9 100644 --- a/packages/router_js/tests/router_test.ts +++ b/packages/router_js/tests/router_test.ts @@ -117,6 +117,10 @@ scenarios.forEach(function (scenario) { return scenario.getRoute(name); } + isRouteInaccessibleByURL(name: string) { + return Boolean(routes[name]?.inaccessibleByURL); + } + getSerializer(name: string) { return scenario.getSerializer(name); } diff --git a/packages/router_js/tests/test_helpers.ts b/packages/router_js/tests/test_helpers.ts index 564c909544f..70006802d37 100644 --- a/packages/router_js/tests/test_helpers.ts +++ b/packages/router_js/tests/test_helpers.ts @@ -72,7 +72,6 @@ export { interface RouteCapabilities { classicInterop: boolean; - awaitEnter: boolean; } interface NavigationArgs { @@ -81,12 +80,13 @@ interface NavigationArgs { internalRouteInfo?: any; cancel: () => void; signal?: AbortSignal; - getAncestorContext: (routeInfo: any) => Promise; + getAncestorPromise: (routeInfo: any) => Promise; } interface RouteManagerLike { capabilities: RouteCapabilities; createRoute(definition: any, args: { name: string }): TestRouteBucket; + getRoute(bucket: TestRouteBucket): unknown; willEnter(bucket: TestRouteBucket, args: NavigationArgs): void; enter(bucket: TestRouteBucket, args: NavigationArgs): Promise; didEnter(bucket: TestRouteBucket, args: NavigationArgs & { enter?: boolean }): void; @@ -131,7 +131,7 @@ const isTransitionLike = (value: unknown): boolean => (no EmberObject, no DI container) so the manager dispatches directly. */ class TestRouteManager implements RouteManagerLike { - capabilities: RouteCapabilities = { classicInterop: true, awaitEnter: true }; + capabilities: RouteCapabilities = { classicInterop: true }; createRoute(handler: ClassicRoute, args: { name: string }): TestRouteBucket { const bucket = new TestRouteBucket(handler, args); @@ -146,6 +146,12 @@ class TestRouteManager implements RouteManagerLike { return bucket; } + // Classic-interop only: these tests drive plain handler objects through the + // classic surface, so the router still hands them back to callers. + getRoute(bucket: TestRouteBucket): unknown { + return bucket.route; + } + willEnter(_bucket: TestRouteBucket, _args: NavigationArgs): void {} enter(bucket: TestRouteBucket, args: NavigationArgs): Promise { @@ -279,14 +285,8 @@ class TestRouteManager implements RouteManagerLike { return route.buildRouteInfoMetadata ? route.buildRouteInfoMetadata() : null; } - // Gate on enterPromise so resolution stays sequential, matching the - // classic expectation that a parent's model resolves before a child's - // model starts. - getInvokable( - _bucket: TestRouteBucket, - enterPromise: Promise - ): Promise { - return (enterPromise ?? Promise.resolve()).then(() => undefined); + getInvokable(_bucket: TestRouteBucket): Promise { + return Promise.resolve(undefined); } } @@ -297,7 +297,7 @@ export function createHandler( options?: Dict ): ClassicRoute { const handler = Object.assign( - { name, routeName: name, context: {}, names: [], handler: name, _internalName: name }, + { name, routeName: name, context: {}, names: [], handler: name }, options ) as unknown as ClassicRoute; // Attach the shared test manager + bucket so the resolve path has something diff --git a/packages/router_js/tests/transition_state_test.ts b/packages/router_js/tests/transition_state_test.ts index f090fd62d7b..4d3d3093168 100644 --- a/packages/router_js/tests/transition_state_test.ts +++ b/packages/router_js/tests/transition_state_test.ts @@ -8,6 +8,7 @@ import { import TransitionState, { type TransitionError } from '../lib/transition-state'; import { Promise, resolve } from 'rsvp'; import { createHandler, createHandlerInfo, TestRouter } from './test_helpers'; +import { associateRouteManagement } from '../lib/route-manager'; QUnit.module('TransitionState'); @@ -113,3 +114,152 @@ QUnit.test('Integration w/ HandlerInfos', function (assert) { assert.ok(false, 'Caught error: ' + error); }); }); + +function createManagedHandler(name: string, enter: () => Promise, classicInterop = false) { + let manager = { + capabilities: { classicInterop }, + willEnter() {}, + enter, + redirect() {}, + getInvokable() { + return resolve(undefined); + }, + }; + + let handler = createHandler(name); + associateRouteManagement(handler, manager as never, { route: handler, invokable: undefined }); + return handler; +} + +QUnit.test('routes load in parallel while an ancestor is still pending', async function (assert) { + assert.expect(3); + + let router = new TestRouter(); + let order: string[] = []; + + let settleParent!: (value: unknown) => void; + let parentEnter = new Promise((res) => { + settleParent = res; + }); + let committedParentAtChildEnter: boolean | undefined; + let transition = { isAborted: false } as unknown as Transition; + + let state = new TransitionState(); + state.routeInfos = [ + new UnresolvedRouteInfoByParam( + router, + 'parent', + [], + {}, + createManagedHandler('parent', () => { + order.push('parent:enter'); + return parentEnter; + }) + ), + new UnresolvedRouteInfoByParam( + router, + 'parent.child', + [], + {}, + createManagedHandler('parent.child', () => { + order.push('child:enter'); + committedParentAtChildEnter = 'parent' in (transition.resolvedModels ?? {}); + return resolve('child-model'); + }) + ), + ]; + + let done = state.resolve(transition); + + await resolve(); + await resolve(); + await resolve(); + + assert.deepEqual( + order.slice(), + ['parent:enter', 'child:enter'], + 'the child starts loading while the parent is still pending' + ); + + settleParent('parent-model'); + await done; + + assert.deepEqual(order.slice(), ['parent:enter', 'child:enter'], 'both entered exactly once'); + assert.false( + committedParentAtChildEnter, + 'the parent had not been committed when the child began loading' + ); +}); + +QUnit.test( + 'a classic route is not entered until its ancestor has published', + async function (assert) { + assert.expect(3); + + let router = new TestRouter(); + let published: string[] = []; + router.onRouteInvokableReady = (routeInfo) => { + published.push(routeInfo.name); + }; + + let order: string[] = []; + let publishedWhenChildEntered: string[] | undefined; + + let settleParent!: (value: unknown) => void; + let parentEnter = new Promise((res) => { + settleParent = res; + }); + + let state = new TransitionState(); + state.routeInfos = [ + new UnresolvedRouteInfoByParam( + router, + 'parent', + [], + {}, + createManagedHandler( + 'parent', + () => { + order.push('parent:enter'); + return parentEnter; + }, + true + ) + ), + new UnresolvedRouteInfoByParam( + router, + 'parent.child', + [], + {}, + createManagedHandler( + 'parent.child', + () => { + order.push('child:enter'); + publishedWhenChildEntered = published.slice(); + return resolve('child-model'); + }, + true + ) + ), + ]; + + let transition = { isAborted: false, router } as unknown as Transition; + let done = state.resolve(transition); + + await resolve(); + await resolve(); + await resolve(); + + assert.deepEqual(order.slice(), ['parent:enter'], 'the child had not entered'); + + settleParent('parent-model'); + await done; + + assert.deepEqual(order.slice(), ['parent:enter', 'child:enter'], 'both entered in order'); + assert.deepEqual( + publishedWhenChildEntered, + ['parent'], + 'the ancestor had been published before the child entered' + ); + } +); diff --git a/smoke-tests/scenarios/classic-route-timing-test.ts b/smoke-tests/scenarios/classic-route-timing-test.ts new file mode 100644 index 00000000000..0cd11cdabdf --- /dev/null +++ b/smoke-tests/scenarios/classic-route-timing-test.ts @@ -0,0 +1,240 @@ +import { strictAppScenarios } from './scenarios'; +import type { PreparedApp } from 'scenario-tester'; +import * as QUnit from 'qunit'; +const { module: Qmodule, test } = QUnit; + +strictAppScenarios + .map('classic-route-timing', (project) => { + project.mergeFiles({ + app: { + 'app.js': ` + import Application from '@ember/application'; + import Router from './router'; + + export default class App extends Application { + modules = { + './router': { default: Router }, + ...import.meta.glob('./services/**/*.{js,ts}', { eager: true }), + ...import.meta.glob('./routes/**/*.{js,ts}', { eager: true }), + ...import.meta.glob('./templates/**/*.hbs', { eager: true }), + }; + } + `, + 'router.js': ` + import EmberRouter from '@embroider/router'; + import config from 'v2-app-template/config/environment'; + + export default class Router extends EmberRouter { + location = config.locationType; + rootURL = config.rootURL; + } + + Router.map(function () { + this.route('parent', function () { + this.route('child'); + }); + }); + `, + services: { + 'flow.js': ` + import Service from '@ember/service'; + + export default class FlowService extends Service { + starts = []; + _resolvers = {}; + _released = {}; + + hold(key) { + this.starts.push(key); + + if (this._released[key]) { + delete this._released[key]; + return Promise.resolve(); + } + + return new Promise((resolve) => { + this._resolvers[key] = resolve; + }); + } + + release(key) { + let resolve = this._resolvers[key]; + + if (resolve) { + delete this._resolvers[key]; + resolve(); + } else { + this._released[key] = true; + } + } + } + `, + }, + routes: { + 'parent.js': ` + import Route from '@ember/routing/route'; + import { service } from '@ember/service'; + + export default class extends Route { + @service flow; + + async model() { + await this.flow.hold('parent'); + return { name: 'parent' }; + } + } + `, + parent: { + 'child.js': ` + import Route from '@ember/routing/route'; + import { service } from '@ember/service'; + + export default class extends Route { + @service flow; + + async model() { + await this.flow.hold('child'); + return { name: 'child', ancestor: this.modelFor('parent').name }; + } + } + `, + }, + }, + templates: { + 'application.hbs': ` +
{{outlet}}
+ `, + 'parent.hbs': ` +
{{@model.name}}{{outlet}}
+ `, + parent: { + 'child.hbs': ` +
{{@model.name}}
+
{{@model.ancestor}}
+ `, + }, + }, + }, + tests: { + acceptance: { + 'classic-route-timing-test.js': ` + import { module, test } from 'qunit'; + import { settled, visit, waitUntil } from '@ember/test-helpers'; + import { setupApplicationTest } from 'v2-app-template/tests/helpers'; + + module('Acceptance | classic route timing', function (hooks) { + setupApplicationTest(hooks); + + test('a descendant model does not start while its ancestor model is pending', async function (assert) { + let flow = this.owner.lookup('service:flow'); + + visit('/parent/child'); + + try { + await waitUntil(() => flow.starts.length === 2, { timeout: 2000 }); + } catch (e) { + // Fall through: the snapshot below reports what did start. + } + + let startedWhilePending = flow.starts.slice(); + + flow.release('parent'); + flow.release('child'); + await settled(); + + assert.deepEqual( + startedWhilePending, + ['parent'], + 'only the ancestor model had started' + ); + }); + + test('a child route reads its ancestor model through modelFor', async function (assert) { + let flow = this.owner.lookup('service:flow'); + + visit('/parent/child'); + + await waitUntil(() => flow.starts.length >= 1, { timeout: 2000 }); + assert + .dom('[data-test="child"]') + .doesNotExist('not rendered while the ancestor model is pending'); + + flow.release('parent'); + + await waitUntil(() => flow.starts.length === 2, { timeout: 2000 }); + assert + .dom('[data-test="child"]') + .doesNotExist('not rendered while its own model is pending'); + + flow.release('child'); + await settled(); + + assert + .dom('[data-test="child-ancestor"]') + .hasText('parent', 'modelFor returned the ancestor model'); + }); + + test('a child route waits for its ancestor even when released first', async function (assert) { + let flow = this.owner.lookup('service:flow'); + + flow.release('child'); + + visit('/parent/child'); + + await waitUntil(() => flow.starts.length >= 1, { timeout: 2000 }); + + assert.deepEqual( + flow.starts.slice(), + ['parent'], + 'the child model had not started' + ); + assert + .dom('[data-test="child"]') + .doesNotExist('not rendered while the ancestor model is pending'); + + flow.release('parent'); + await settled(); + + assert + .dom('[data-test="child-ancestor"]') + .hasText('parent', 'modelFor returned the ancestor model'); + }); + + test('a route does not render while its own model is pending', async function (assert) { + let flow = this.owner.lookup('service:flow'); + + visit('/parent/child'); + + await waitUntil(() => flow.starts.length >= 1, { timeout: 2000 }); + + let renderedWhilePending = Boolean( + document.querySelector('[data-test="parent"]') + ); + + flow.release('parent'); + flow.release('child'); + await settled(); + + assert.false(renderedWhilePending, 'the ancestor had not rendered'); + assert.dom('[data-test="child"]').hasText('child'); + }); + }); + `, + }, + }, + }); + }) + .forEachScenario((scenario) => { + Qmodule(scenario.name, function (hooks) { + let app: PreparedApp; + + hooks.before(async () => { + app = await scenario.prepare(); + }); + + test('ember test', async function (assert) { + let result = await app.execute('pnpm test'); + assert.equal(result.exitCode, 0, result.output); + }); + }); + }); diff --git a/smoke-tests/scenarios/pioneer-route-timing-test.ts b/smoke-tests/scenarios/pioneer-route-timing-test.ts new file mode 100644 index 00000000000..fbdc4e1e50d --- /dev/null +++ b/smoke-tests/scenarios/pioneer-route-timing-test.ts @@ -0,0 +1,251 @@ +import { v1AppScenarios } from './scenarios'; +import type { PreparedApp } from 'scenario-tester'; +import * as QUnit from 'qunit'; +const { module: Qmodule, test } = QUnit; + +const appName = 'ember-test-app'; + +v1AppScenarios + .only('classic') + .map('pioneer-route-timing', (project) => { + project.mergeFiles({ + app: { + 'router.js': ` + import EmberRouter from '@ember/routing/router'; + import config from '${appName}/config/environment'; + + export default class Router extends EmberRouter { + location = config.locationType; + rootURL = config.rootURL; + } + + Router.map(function () { + this.route('parent', function () { + this.route('child'); + }); + }); + `, + services: { + 'flow.js': ` + import Service from '@ember/service'; + + export default class FlowService extends Service { + starts = []; + _resolvers = {}; + _released = {}; + + hold(key) { + this.starts.push(key); + + if (this._released[key]) { + delete this._released[key]; + return Promise.resolve(); + } + + return new Promise((resolve) => { + this._resolvers[key] = resolve; + }); + } + + release(key) { + let resolve = this._resolvers[key]; + + if (resolve) { + delete this._resolvers[key]; + resolve(); + } else { + this._released[key] = true; + } + } + } + `, + }, + components: { + 'pioneer-components.gjs': ` + export const PioneerOutlet = ; + + export const ParentComponent = ; + + export const ChildComponent = ; + `, + }, + 'route-managers': { + 'pioneer.js': ` + import { routeCapabilities } from '@ember/routing'; + import { + ChildComponent, + ParentComponent, + PioneerOutlet, + } from '${appName}/components/pioneer-components'; + + const ROUTES = { + parent: ParentComponent, + 'parent.child': ChildComponent, + }; + + class PioneerBucket { + constructor(name, route, invokable) { + this.name = name; + this.route = route; + this.invokable = invokable; + } + } + + export default class PioneerRouteManager { + capabilities = routeCapabilities('1.0'); + + constructor(owner) { + this.owner = owner; + } + + createRoute(RouteClass, { name }) { + return new PioneerBucket(name, new RouteClass(this.owner), ROUTES[name]); + } + + getDestroyable() { + return null; + } + + getRouteWrapper() { + return PioneerOutlet; + } + + willEnter() {} + + async enter(bucket) { + return bucket.route.model(); + } + + didEnter() {} + willExit() {} + exit() {} + didExit() {} + + async getInvokable(bucket) { + return bucket.invokable; + } + } + `, + }, + routes: { + 'pioneer.js': ` + import { setOwner } from '@ember/owner'; + import { setRouteManager } from '@ember/routing'; + import PioneerRouteManager from '${appName}/route-managers/pioneer'; + + export default class PioneerRoute { + constructor(owner) { + setOwner(this, owner); + } + } + + setRouteManager((owner) => new PioneerRouteManager(owner), PioneerRoute); + `, + 'parent.js': ` + import { service } from '@ember/service'; + import PioneerRoute from '${appName}/routes/pioneer'; + + export default class extends PioneerRoute { + @service flow; + + async model() { + await this.flow.hold('parent'); + return { name: 'parent' }; + } + } + `, + parent: { + 'child.js': ` + import { service } from '@ember/service'; + import PioneerRoute from '${appName}/routes/pioneer'; + + export default class extends PioneerRoute { + @service flow; + + async model() { + await this.flow.hold('child'); + return { name: 'child' }; + } + } + `, + }, + }, + }, + tests: { + acceptance: { + 'pioneer-route-timing-test.js': ` + import { module, test } from 'qunit'; + import { settled, visit, waitUntil } from '@ember/test-helpers'; + import { setupApplicationTest } from '${appName}/tests/helpers'; + + module('Acceptance | pioneer route timing', function (hooks) { + setupApplicationTest(hooks); + + test('a descendant model starts while its ancestor model is pending', async function (assert) { + let flow = this.owner.lookup('service:flow'); + + visit('/parent/child'); + + try { + await waitUntil(() => flow.starts.length === 2, { timeout: 2000 }); + } catch (e) { + // Fall through: the snapshot below reports what did start. + } + + let startedWhilePending = flow.starts.slice(); + + flow.release('parent'); + flow.release('child'); + await settled(); + + assert.deepEqual( + startedWhilePending, + ['parent', 'child'], + 'the descendant loaded alongside its ancestor' + ); + }); + + test('a route does not render while its own model is pending', async function (assert) { + let flow = this.owner.lookup('service:flow'); + + visit('/parent/child'); + + await waitUntil(() => flow.starts.length >= 1, { timeout: 2000 }); + + assert + .dom('[data-test="parent"]') + .doesNotExist('the ancestor had not rendered'); + + flow.release('parent'); + flow.release('child'); + await settled(); + + assert.dom('[data-test="child"]').hasText('child'); + }); + }); + `, + }, + }, + }); + }) + .forEachScenario((scenario) => { + Qmodule(scenario.name, function (hooks) { + let app: PreparedApp; + + hooks.before(async () => { + app = await scenario.prepare(); + }); + + test('ember test', async function (assert) { + let result = await app.execute('pnpm test'); + assert.equal(result.exitCode, 0, result.output); + }); + }); + }); diff --git a/smoke-tests/scenarios/route-managers-test.ts b/smoke-tests/scenarios/route-managers-test.ts index 2ef8991f793..e03b9712c76 100644 --- a/smoke-tests/scenarios/route-managers-test.ts +++ b/smoke-tests/scenarios/route-managers-test.ts @@ -25,9 +25,36 @@ function createRouteComponent(componentName: string, route: string): string { } function routeManagerTests(scenarios: Scenarios, appName: string) { - const FUNKY_ROUTE_SOURCE = ` + const modelFor = (name: string) => `model:${name}`; + + const funkyRoute = (name: string) => ` import FunkyRoute from '${appName}/routes/funky'; - export default class extends FunkyRoute {} + + export default class extends FunkyRoute { + model() { + return '${modelFor(name)}'; + } + } + `; + + const classicRoute = (name: string) => ` + import Route from '@ember/routing/route'; + + export default class extends Route { + model() { + return '${modelFor(name)}'; + } + } + `; + + const glimmerRoute = (name: string) => ` + import GlimmerRoute from '${appName}/routes/glimmer-route'; + + export default class extends GlimmerRoute { + model() { + return '${modelFor(name)}'; + } + } `; const ROUTE_FIXTURES: RouteChainManifest[] = [ @@ -38,8 +65,9 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { + 'classic-to-funky.js': classicRoute('classic-to-funky'), 'classic-to-funky': { - 'child.js': FUNKY_ROUTE_SOURCE, + 'child.js': funkyRoute('classic-to-funky.child'), }, }, templates: { @@ -47,6 +75,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { @@ -54,7 +83,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }, routeComponent: createRouteComponent( 'ClassicToFunkyChild', - `
funky child
` + `
{{@context}}funky child
` ), managerInvokableMap: ` 'classic-to-funky.child': COMPONENTS.ClassicToFunkyChild, @@ -67,13 +96,16 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { - 'funky-to-classic.js': FUNKY_ROUTE_SOURCE, + 'funky-to-classic.js': funkyRoute('funky-to-classic'), + 'funky-to-classic': { + 'child.js': classicRoute('funky-to-classic.child'), + }, }, templates: { 'funky-to-classic': { 'child.gjs': ` `, }, @@ -82,6 +114,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToClassic', `
funky parent + {{@context}}
{{outlet}}
` ), @@ -98,35 +131,11 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { - 'classic-to-funky-to-classic.js': ` - import Route from '@ember/routing/route'; - - export default class extends Route { - model() { - return '1'; - } - } - `, + 'classic-to-funky-to-classic.js': classicRoute('classic-to-funky-to-classic'), 'classic-to-funky-to-classic': { - 'child.js': ` - import FunkyRoute from '${appName}/routes/funky'; - - export default class extends FunkyRoute { - model() { - return '2'; - } - } - `, + 'child.js': funkyRoute('classic-to-funky-to-classic.child'), child: { - 'grandchild.js': ` - import Route from '@ember/routing/route'; - - export default class extends Route { - model() { - return '3'; - } - } - `, + 'grandchild.js': classicRoute('classic-to-funky-to-classic.child.grandchild'), }, }, }, @@ -157,7 +166,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'ClassicToFunkyToClassicChild', `
funky child - {{@model}} + {{@context}}
{{outlet}}
` ), @@ -174,35 +183,11 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { - 'funky-to-classic-to-funky.js': ` - import FunkyRoute from '${appName}/routes/funky'; - - export default class extends FunkyRoute { - model() { - return '1'; - } - } - `, + 'funky-to-classic-to-funky.js': funkyRoute('funky-to-classic-to-funky'), 'funky-to-classic-to-funky': { - 'child.js': ` - import Route from '@ember/routing/route'; - - export default class extends Route { - model() { - return '2'; - } - } - `, + 'child.js': classicRoute('funky-to-classic-to-funky.child'), child: { - 'grandchild.js': ` - import FunkyRoute from '${appName}/routes/funky'; - - export default class extends FunkyRoute { - model() { - return '3'; - } - } - `, + 'grandchild.js': funkyRoute('funky-to-classic-to-funky.child.grandchild'), }, }, }, @@ -224,7 +209,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToClassicToFunky', `
funky parent - {{@model}} + {{@context}}
{{outlet}}
` ) + @@ -232,7 +217,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToClassicToFunkyGrandchild', `
funky grandchild - {{@model}} + {{@context}}
` ), managerInvokableMap: ` @@ -251,11 +236,16 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { - 'funky-to-funky-to-funky-to-classic.js': FUNKY_ROUTE_SOURCE, + 'funky-to-funky-to-funky-to-classic.js': funkyRoute('funky-to-funky-to-funky-to-classic'), 'funky-to-funky-to-funky-to-classic': { - 'child.js': FUNKY_ROUTE_SOURCE, + 'child.js': funkyRoute('funky-to-funky-to-funky-to-classic.child'), child: { - 'grandchild.js': FUNKY_ROUTE_SOURCE, + 'grandchild.js': funkyRoute('funky-to-funky-to-funky-to-classic.child.grandchild'), + grandchild: { + 'great-grandchild.js': classicRoute( + 'funky-to-funky-to-funky-to-classic.child.grandchild.great-grandchild' + ), + }, }, }, }, @@ -267,6 +257,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { `, @@ -279,6 +270,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToFunkyToFunkyToClassic', `
funky parent + {{@context}}
{{outlet}}
` ) + @@ -286,6 +278,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToFunkyToFunkyToClassicChild', `
funky middle + {{@context}}
{{outlet}}
` ) + @@ -293,6 +286,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToFunkyToFunkyToClassicGrandchild', `
funky child + {{@context}}
{{outlet}}
` ), @@ -315,13 +309,23 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { - 'funky-to-funky-to-classic-to-classic-to-funky.js': FUNKY_ROUTE_SOURCE, + 'funky-to-funky-to-classic-to-classic-to-funky.js': funkyRoute( + 'funky-to-funky-to-classic-to-classic-to-funky' + ), 'funky-to-funky-to-classic-to-classic-to-funky': { - 'child.js': FUNKY_ROUTE_SOURCE, + 'child.js': funkyRoute('funky-to-funky-to-classic-to-classic-to-funky.child'), child: { + 'grandchild.js': classicRoute( + 'funky-to-funky-to-classic-to-classic-to-funky.child.grandchild' + ), grandchild: { + 'great-grandchild.js': classicRoute( + 'funky-to-funky-to-classic-to-classic-to-funky.child.grandchild.great-grandchild' + ), 'great-grandchild': { - 'great-great-grandchild.js': FUNKY_ROUTE_SOURCE, + 'great-great-grandchild.js': funkyRoute( + 'funky-to-funky-to-classic-to-classic-to-funky.child.grandchild.great-grandchild.great-great-grandchild' + ), }, }, }, @@ -334,6 +338,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { @@ -343,6 +348,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { @@ -356,6 +362,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToFunkyToClassicToClassicToFunky', `
funky parent + {{@context}}
{{outlet}}
` ) + @@ -363,6 +370,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToFunkyToClassicToClassicToFunkyChild', `
funky child + {{@context}}
{{outlet}}
` ) + @@ -370,6 +378,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyToFunkyToClassicToClassicToFunkyLeaf', `
funky leaf + {{@context}}
` ), managerInvokableMap: ` @@ -391,11 +400,24 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { + 'classic-to-classic-to-funky-to-funky-to-classic.js': classicRoute( + 'classic-to-classic-to-funky-to-funky-to-classic' + ), 'classic-to-classic-to-funky-to-funky-to-classic': { + 'child.js': classicRoute('classic-to-classic-to-funky-to-funky-to-classic.child'), child: { - 'grandchild.js': FUNKY_ROUTE_SOURCE, + 'grandchild.js': funkyRoute( + 'classic-to-classic-to-funky-to-funky-to-classic.child.grandchild' + ), grandchild: { - 'great-grandchild.js': FUNKY_ROUTE_SOURCE, + 'great-grandchild.js': funkyRoute( + 'classic-to-classic-to-funky-to-funky-to-classic.child.grandchild.great-grandchild' + ), + 'great-grandchild': { + 'great-great-grandchild.js': classicRoute( + 'classic-to-classic-to-funky-to-funky-to-classic.child.grandchild.great-grandchild.great-great-grandchild' + ), + }, }, }, }, @@ -405,6 +427,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { @@ -414,6 +437,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { @@ -425,6 +449,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { `, @@ -438,6 +463,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'ClassicToClassicToFunkyToFunkyToClassicGrandchild', `
funky middle + {{@context}}
{{outlet}}
` ) + @@ -445,6 +471,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'ClassicToClassicToFunkyToFunkyToClassicGreatGrandchild', `
funky child + {{@context}}
{{outlet}}
` ), @@ -461,9 +488,10 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { - 'sibling-transitions.js': FUNKY_ROUTE_SOURCE, + 'sibling-transitions.js': funkyRoute('sibling-transitions'), 'sibling-transitions': { - 'funky-child.js': FUNKY_ROUTE_SOURCE, + 'classic-child.js': classicRoute('sibling-transitions.classic-child'), + 'funky-child.js': funkyRoute('sibling-transitions.funky-child'), }, }, templates: { @@ -472,6 +500,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { `, @@ -482,6 +511,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'SiblingTransitions', `
funky parent + {{@context}}
{{outlet}}
` ) + @@ -489,6 +519,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'SiblingTransitionsFunkyChild', `
funky sibling + {{@context}}
` ), managerInvokableMap: ` @@ -515,7 +546,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'FunkyReentry', `
funky reentry - {{@model}} + {{@context}}
` ), managerInvokableMap: ` @@ -531,6 +562,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { routes: { 'reactive-context.js': ` import ReactiveRoute from '${appName}/routes/reactive'; + import { modelStarts } from '${appName}/router'; let resolve; export function resolveModel(value) { @@ -539,6 +571,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { export default class extends ReactiveRoute { model() { + modelStarts.push('reactive-context'); return new Promise((r) => (resolve = r)); } } @@ -546,6 +579,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { 'reactive-context': { 'child.js': ` import ReactiveRoute from '${appName}/routes/reactive'; + import { modelStarts } from '${appName}/router'; let resolve; export function resolveModel(value) { @@ -554,6 +588,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { export default class extends ReactiveRoute { model() { + modelStarts.push('reactive-context.child'); return new Promise((r) => (resolve = r)); } } @@ -610,25 +645,9 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }); `, routes: { - 'glimmer-wrapper.js': ` - import GlimmerRoute from '${appName}/routes/glimmer-route'; - - export default class extends GlimmerRoute { - model() { - return 'glimmer-wrapper'; - } - } - `, + 'glimmer-wrapper.js': glimmerRoute('glimmer-wrapper'), 'glimmer-wrapper': { - 'child.js': ` - import GlimmerRoute from '${appName}/routes/glimmer-route'; - - export default class extends GlimmerRoute { - model() { - return 'glimmer-wrapper.child'; - } - } - `, + 'child.js': glimmerRoute('glimmer-wrapper.child'), }, }, routeComponent: @@ -673,6 +692,8 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { import EmberRouter from '@ember/routing/router'; import config from '${appName}/config/environment'; + export const modelStarts = []; + export default class Router extends EmberRouter { location = config.locationType; rootURL = config.rootURL; @@ -782,10 +803,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { import { tracked } from '@glimmer/tracking'; import { on } from '@ember/modifier'; - // \`model\` is tracked; no render-state plumbing. export class FunkyBucket { - @tracked model; - constructor(name, route, invokable) { this.name = name; this.route = route; @@ -803,7 +821,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { ; `, 'swap-outlet.gjs': ` @@ -883,10 +906,10 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { }; const BUCKETS = new Map(); - let renderStateCalls = 0; + let getInvokableCalls = 0; - export function renderStateCallCount() { - return renderStateCalls; + export function getInvokableCallCount() { + return getInvokableCalls; } export function finishLoading(name) { @@ -896,8 +919,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { class SwapBucket { @tracked ready = false; - constructor(name, route, invokable) { - this.name = name; + constructor(route, invokable) { this.route = route; this.invokable = invokable; } @@ -911,15 +933,11 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { } createRoute(RouteClass, { name }) { - let bucket = new SwapBucket(name, new RouteClass(this.owner), ROUTES[name]); + let bucket = new SwapBucket(new RouteClass(this.owner), ROUTES[name]); BUCKETS.set(name, bucket); return bucket; } - getRoute(bucket) { - return bucket.route; - } - getDestroyable() { return null; } @@ -935,8 +953,8 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { exit() {} didExit() {} - getInvokable(bucket) { - renderStateCalls++; + async getInvokable(bucket) { + getInvokableCalls++; return bucket.invokable; } } @@ -970,10 +988,6 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { return new ReactiveBucket(name, new RouteClass(this.owner), ROUTES[name]); } - getRoute(bucket) { - return bucket.route; - } - getDestroyable() { return null; } @@ -993,7 +1007,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { exit() {} didExit() {} - getInvokable(bucket) { + async getInvokable(bucket) { return bucket.invokable; } } @@ -1018,10 +1032,6 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { return new FunkyBucket(name, new RouteClass(this.owner), ROUTES[name]); } - getRoute(bucket) { - return bucket.route; - } - getDestroyable() { return null; } @@ -1034,9 +1044,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { async enter(bucket, state) { let info = state.to.find((i) => i.name === bucket.name) ?? state.to; - let model = await bucket.route.model?.(info.params); - bucket.model = model; - return model; + return bucket.route.model?.(info.params); } didEnter() {} @@ -1044,7 +1052,7 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { exit() {} didExit() {} - getInvokable(bucket) { + async getInvokable(bucket) { return bucket.invokable; } } @@ -1138,18 +1146,19 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { import { module, test } from 'qunit'; import { click, findAll, settled, visit, waitUntil } from '@ember/test-helpers'; import { setupApplicationTest } from '${appName}/tests/helpers'; + import { modelStarts } from '${appName}/router'; import { resolveModel as resolveParentModel } from '${appName}/routes/reactive-context'; import { resolveModel as resolveChildModel } from '${appName}/routes/reactive-context/child'; import { finishLoading as finishSwapLoading, - renderStateCallCount, + getInvokableCallCount, } from '${appName}/route-managers/swap'; const CLASSIC_ROUTE_SELECTOR = '[data-test-classic-route]'; const FUNKY_ROUTE_SELECTOR = '[data-test-funky-route]'; const GATE_SELECTOR = 'button[data-test-render-route]'; - function assertClassicRoute(assert, index, name, expectedModel) { + function assertClassicRoute(assert, index, name, expectedModel = 'model:' + name) { let route = findAll(CLASSIC_ROUTE_SELECTOR)[index]; assert.dom(route).hasAttribute( @@ -1157,14 +1166,12 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { name ); - if (expectedModel !== undefined) { - assert - .dom(route.querySelector(':scope > [data-test-route-model]')) - .hasText(expectedModel); - } + assert + .dom(route.querySelector(':scope > [data-test-route-model]')) + .hasText(expectedModel); } - function assertFunkyRoute(assert, index, name, expectedModel) { + function assertFunkyRoute(assert, index, name, expectedModel = 'model:' + name) { let route = findAll(FUNKY_ROUTE_SELECTOR)[index]; assert.dom(route).hasAttribute( @@ -1172,11 +1179,9 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { name ); - if (expectedModel !== undefined) { - assert - .dom(route.querySelector(':scope > [data-test-route-model]')) - .hasText(expectedModel); - } + assert + .dom(route.querySelector(':scope > [data-test-route-model]')) + .hasText(expectedModel); } async function openFunkyRoute(assert, name) { @@ -1208,15 +1213,14 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { test('classic -> funky -> classic', async function (assert) { await visit('/classic-to-funky-to-classic/child/grandchild'); - assertClassicRoute(assert, 0, 'classic-to-funky-to-classic', '1'); + assertClassicRoute(assert, 0, 'classic-to-funky-to-classic'); assert.dom(FUNKY_ROUTE_SELECTOR).doesNotExist(); await openFunkyRoute(assert, 'classic-to-funky-to-classic.child'); - assertFunkyRoute(assert, 0, 'classic-to-funky-to-classic.child', '2'); + assertFunkyRoute(assert, 0, 'classic-to-funky-to-classic.child'); assertClassicRoute( assert, 1, - 'classic-to-funky-to-classic.child.grandchild', - '3' + 'classic-to-funky-to-classic.child.grandchild' ); }); @@ -1318,8 +1322,8 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { assert.dom(FUNKY_ROUTE_SELECTOR).doesNotExist(); await openFunkyRoute(assert, 'funky-to-classic-to-funky'); - assertFunkyRoute(assert, 0, 'funky-to-classic-to-funky', '1'); - assertClassicRoute(assert, 0, 'funky-to-classic-to-funky.child', '2'); + assertFunkyRoute(assert, 0, 'funky-to-classic-to-funky'); + assertClassicRoute(assert, 0, 'funky-to-classic-to-funky.child'); assert.dom(FUNKY_ROUTE_SELECTOR).exists({ count: 1 }); await openFunkyRoute( assert, @@ -1328,56 +1332,44 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { assertFunkyRoute( assert, 1, - 'funky-to-classic-to-funky.child.grandchild', - '3' + 'funky-to-classic-to-funky.child.grandchild' ); }); - test('a manager that renders before its model resolves has its context filled in', async function (assert) { - // Not awaited: both levels mount while their models are pending. - visit('/reactive-context/child'); - await waitUntil(() => - document.querySelector('[data-test-reactive-route="reactive-context.child"]') - ); + test('a non-classic manager loads ancestor and descendant models in parallel', async function (assert) { + modelStarts.length = 0; - assert - .dom('[data-test-reactive-route="reactive-context"] > [data-test-route-model]') - .hasText(''); - assert - .dom('[data-test-reactive-route="reactive-context.child"] > [data-test-route-model]') - .hasText(''); + visit('/reactive-context/child'); - // Parent only: the child's pending model keeps the transition - // unsettled, so no outlet pass can explain what renders next. - resolveParentModel('PARENT-CTX'); - await waitUntil( - () => - document.querySelector( - '[data-test-reactive-route="reactive-context"] > [data-test-route-model]' - ).textContent.trim() === 'PARENT-CTX' - ); + try { + await waitUntil(() => modelStarts.length === 2, { timeout: 2000 }); + } catch (e) { + // Fall through: the snapshot below reports what did start. + } - assert - .dom('[data-test-reactive-route="reactive-context.child"] > [data-test-route-model]') - .hasText(''); + let startedWhilePending = modelStarts.slice(); + resolveParentModel('PARENT-CTX'); resolveChildModel('CHILD-CTX'); await settled(); - assert - .dom('[data-test-reactive-route="reactive-context.child"] > [data-test-route-model]') - .hasText('CHILD-CTX'); + assert.deepEqual( + startedWhilePending, + ['reactive-context', 'reactive-context.child'], + 'the child model started while the parent model was still pending' + ); }); test('a filled-in context survives a transition that keeps the route mounted', async function (assert) { - visit('/reactive-context/child'); - await waitUntil(() => - document.querySelector('[data-test-reactive-route="reactive-context.child"]') - ); + modelStarts.length = 0; + + let visitPromise = visit('/reactive-context/child'); + + await waitUntil(() => modelStarts.length === 2, { timeout: 2000 }); resolveParentModel('PARENT-CTX'); resolveChildModel('CHILD-CTX'); - await settled(); + await visitPromise; assert .dom('[data-test-reactive-route="reactive-context"] > [data-test-route-model]') @@ -1391,21 +1383,21 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { .hasText('PARENT-CTX'); }); - test('a manager swaps what is rendered from tracked state, with no router pass', async function (assert) { + test('a manager swaps what is rendered from tracked state, without re-entering the route', async function (assert) { await visit('/swap-invokable'); assert.dom('[data-test-swap-invokable="loading"]').exists(); assert.dom('[data-test-swap-invokable="ready"]').doesNotExist(); - let passesBefore = renderStateCallCount(); + let callsBefore = getInvokableCallCount(); finishSwapLoading('swap-invokable'); await settled(); assert.strictEqual( - renderStateCallCount(), - passesBefore, - 'the swap happened without a _setOutlets pass' + getInvokableCallCount(), + callsBefore, + 'the swap needed no new invokable, so no re-entry' ); assert.dom('[data-test-swap-invokable="loading"]').doesNotExist(); assert.dom('[data-test-swap-invokable="ready"]').exists(); @@ -1432,9 +1424,12 @@ function routeManagerTests(scenarios: Scenarios, appName: string) { assert.dom('[data-test-glimmer-route="glimmer-wrapper"]').exists(); assert.dom('[data-test-glimmer-route="glimmer-wrapper.child"]').exists(); + assert + .dom('[data-test-glimmer-route="glimmer-wrapper"] > [data-test-route-model]') + .hasText('model:glimmer-wrapper'); assert .dom('[data-test-glimmer-route="glimmer-wrapper.child"] > [data-test-route-model]') - .hasText('glimmer-wrapper.child'); + .hasText('model:glimmer-wrapper.child'); assert .dom('[data-test-outlet-with-service="glimmer-wrapper"]')