From 7f388fde5fecefdb93cf0a23a8b83e2c1a7f17c9 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Wed, 8 Nov 2023 22:43:20 -0800 Subject: [PATCH] [CLEANUP] remove named outlet code from routing layer The split definitions between the rendering and routing layer was because the rendering layer was the first package to be converted into TypeScript. Now that everything is converted they should be sharing the same definition of the interface. --- packages/@ember/-internals/glimmer/index.ts | 2 +- packages/@ember/routing/route.ts | 166 +++----------------- packages/@ember/routing/router.ts | 145 ++++------------- 3 files changed, 58 insertions(+), 255 deletions(-) diff --git a/packages/@ember/-internals/glimmer/index.ts b/packages/@ember/-internals/glimmer/index.ts index b0b9c3d4fd9..d647ab9c722 100644 --- a/packages/@ember/-internals/glimmer/index.ts +++ b/packages/@ember/-internals/glimmer/index.ts @@ -474,7 +474,7 @@ export { DOMChanges, NodeDOMTreeConstruction, DOMTreeConstruction } from './lib/ // a lot of these are testing how a problem was solved // rather than the problem was solved export { default as OutletView, BootEnvironment } from './lib/views/outlet'; -export { OutletState } from './lib/utils/outlet'; +export { OutletState, RenderState } from './lib/utils/outlet'; export { componentCapabilities, modifierCapabilities, diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index ee19c036e69..f4526ba13af 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -12,7 +12,7 @@ import EmberObject, { computed, get, set, getProperties, setProperties } from '@ import Evented from '@ember/object/evented'; import { A as emberA } from '@ember/array'; import { ActionHandler } from '@ember/-internals/runtime'; -import { isEmpty, typeOf } from '@ember/utils'; +import { typeOf } from '@ember/utils'; import { isProxy, lookupDescriptor } from '@ember/-internals/utils'; import type { AnyFn } from '@ember/-internals/utility-types'; import Controller from '@ember/controller'; @@ -22,7 +22,8 @@ import EngineInstance from '@ember/engine/instance'; import { dependentKeyCompat } from '@ember/object/compat'; import { once } from '@ember/runloop'; import { DEBUG } from '@glimmer/env'; -import type { Template, TemplateFactory } from '@glimmer/interfaces'; +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 } from '@ember/routing/router'; @@ -69,8 +70,8 @@ function isStoreLike(store: unknown): store is StoreLike { ); } -export const ROUTE_CONNECTIONS = new WeakMap(); const RENDER = Symbol('render'); +const RENDER_STATE = Symbol('render-state'); /** @module @ember/routing/route @@ -807,7 +808,7 @@ class Route extends EmberObject.extend(ActionHandler, Evented) @method enter */ enter(transition: Transition) { - ROUTE_CONNECTIONS.set(this, []); + this[RENDER_STATE] = undefined; this.activate(transition); this.trigger('activate', transition); } @@ -1503,26 +1504,15 @@ class Route extends EmberObject.extend(ActionHandler, Evented) return route?.currentModel; } - /** - `this[RENDER]` is used to render a template into a region of another template - (indicated by an `{{outlet}}`). + [RENDER_STATE]: RenderState | undefined = undefined; + /** + `this[RENDER]` is used to set up the rendering option for the outlet state. @method this[RENDER] - @param {String} name the name of the template to render - @param {Object} [options] the options - @param {String} [options.into] the template to render into, - referenced by name. Defaults to the parent template - @param {String} [options.outlet] the outlet inside `options.into` to render into. - Defaults to 'main' - @param {String|Object} [options.controller] the controller to use for this template, - referenced by name or as a controller instance. Defaults to the Route's paired controller - @param {Object} [options.model] the model object to set on `options.controller`. - Defaults to the return value of the Route's model hook @private */ - [RENDER](name?: string, options?: PartialRenderOptions) { - let renderOptions = buildRenderOptions(this, name, options); - ROUTE_CONNECTIONS.get(this).push(renderOptions); + [RENDER]() { + this[RENDER_STATE] = buildRenderState(this); once(this._router, '_setOutlets'); } @@ -1536,9 +1526,8 @@ class Route extends EmberObject.extend(ActionHandler, Evented) @method teardownViews */ teardownViews() { - let connections = ROUTE_CONNECTIONS.get(this); - if (connections !== undefined && connections.length > 0) { - ROUTE_CONNECTIONS.set(this, []); + if (this[RENDER_STATE]) { + this[RENDER_STATE] = undefined; once(this._router, '_setOutlets'); } } @@ -1841,122 +1830,33 @@ class Route extends EmberObject.extend(ActionHandler, Evented) >; } -function parentRoute(route: Route) { - let routeInfo = routeInfoFor(route, route._router._routerMicrolib.state!.routeInfos, -1); - return routeInfo && routeInfo.route; -} - -function routeInfoFor(route: Route, routeInfos: InternalRouteInfo[], offset = 0) { - if (!routeInfos) { - return; - } - - let current: Route | undefined; - for (let i = 0; i < routeInfos.length; i++) { - let routeInfo = routeInfos[i]; - assert('has current routeInfo', routeInfo); - current = routeInfo.route; - if (current === route) { - return routeInfos[i + offset]; - } - } - - return; +export function getRenderState(route: Route): RenderState | undefined { + return route[RENDER_STATE]; } -function buildRenderOptions( - route: Route, - nameOrOptions?: string | PartialRenderOptions, - options?: PartialRenderOptions -): RenderOptions { - let isDefaultRender = !nameOrOptions && !options; - let _name: string; - if (!isDefaultRender) { - if (typeof nameOrOptions === 'object' && !options) { - _name = route.templateName || route.routeName; - options = nameOrOptions; - } else { - assert( - 'The name in the given arguments is undefined or empty string', - !isEmpty(nameOrOptions) - ); - // SAFETY: the check for `nameOrOptions` above should be validating this, - // and as of TS 5.1.0-dev.2023-0417 it is *not*. This cast can go away if - // TS validates it correctly *or* if we refactor this entire function to - // be less wildly dynamic in its argument handling. - _name = nameOrOptions as string; - } - } - assert( - 'You passed undefined as the outlet name.', - isDefaultRender || !(options && 'outlet' in options && options.outlet === undefined) - ); - +function buildRenderState(route: Route): RenderState { let owner = getOwner(route); assert('Route is unexpectedly missing an owner', owner); - let name, templateName, into, outlet, model; - let controller; - - if (options) { - into = options.into && options.into.replace(/\//g, '.'); - outlet = options.outlet; - controller = options.controller; - model = options.model; - } - outlet = outlet || 'main'; - - if (isDefaultRender) { - name = route.routeName; - templateName = route.templateName || name; - } else { - name = _name!.replace(/\//g, '.'); - templateName = name; - } - - if (controller === undefined) { - if (isDefaultRender) { - controller = route.controllerName || owner.lookup(`controller:${name}`); - } else { - controller = owner.lookup(`controller:${name}`) || route.controllerName || route.routeName; - } - } - if (typeof controller === 'string') { - let controllerName = controller; - controller = owner.lookup(`controller:${controllerName}`); - assert( - `You passed \`controller: '${controllerName}'\` into the \`render\` method, but no such controller could be found.`, - isDefaultRender || controller !== undefined - ); - } + let name = route.routeName; + let controller = owner.lookup(`controller:${route.controllerName || name}`); assert('Expected an instance of controller', controller instanceof Controller); - if (model === undefined) { - model = route.currentModel; - } else { - controller.set('model', model); - } - - let template = owner.lookup(`template:${templateName}`) as TemplateFactory; - assert( - `Could not find "${templateName}" template, view, or component.`, - isDefaultRender || template !== undefined - ); + let model = route.currentModel; - let parent: any; - if (into && (parent = parentRoute(route)) && into === parent.routeName) { - into = undefined; - } + let template = owner.lookup(`template:${route.templateName || name}`) as + | TemplateFactory + | undefined; - let renderOptions: RenderOptions = { + let render: RenderState = { owner, - into, - outlet, + into: undefined, + outlet: 'main', name, controller, model, - template: template !== undefined ? template(owner) : route._topLevelViewTemplate(owner), + template: template?.(owner) ?? route._topLevelViewTemplate(owner), }; if (DEBUG) { @@ -1968,23 +1868,9 @@ function buildRenderOptions( } } - return renderOptions; -} - -export interface RenderOptions { - owner: Owner; - into?: string; - outlet: string; - name: string; - controller: Controller | string | undefined; - model: unknown; - template: Template; + return render; } -type PartialRenderOptions = Partial< - Pick ->; - 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 21c1b235ef6..f41a58d6a4e 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -1,9 +1,5 @@ import { privatize as P } from '@ember/-internals/container'; -import type { - BootEnvironment, - OutletState as GlimmerOutletState, - OutletView, -} from '@ember/-internals/glimmer'; +import type { BootEnvironment, OutletState, OutletView } from '@ember/-internals/glimmer'; import { computed, get, set } from '@ember/object'; import type { default as Owner, FactoryManager } from '@ember/owner'; import { getOwner } from '@ember/owner'; @@ -28,13 +24,13 @@ import Evented from '@ember/object/evented'; import { assert, info } from '@ember/debug'; import { cancel, once, run, scheduleOnce } from '@ember/runloop'; import { DEBUG } from '@glimmer/env'; -import type { QueryParamMeta, RenderOptions } from '@ember/routing/route'; +import type { QueryParamMeta } from '@ember/routing/route'; import type Route from '@ember/routing/route'; import { defaultSerialize, getFullQueryParams, + getRenderState, hasDefaultSerialize, - ROUTE_CONNECTIONS, } from '@ember/routing/route'; import type { InternalRouteInfo, @@ -105,21 +101,6 @@ if (DEBUG) { }; } -interface RenderOutletState { - name: string; - outlet: string; -} - -interface NestedOutletState { - [key: string]: OutletState; -} - -interface OutletState { - render: T; - outlets: NestedOutletState; - wasUsed?: boolean; -} - export interface QueryParam { prop: string; urlKey: string; @@ -616,26 +597,36 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { return; } - let defaultParentState: OutletState | undefined; - let liveRoutes = null; + let root: OutletState | null = null; + let parent: OutletState | null = null; for (let routeInfo of routeInfos) { let route = routeInfo.route!; - let connections = ROUTE_CONNECTIONS.get(route); - let ownState: OutletState; - if (connections.length === 0) { - ownState = representEmptyRoute(liveRoutes, defaultParentState, route); - } else { - for (let j = 0; j < connections.length; j++) { - let appended = appendLiveRoute(liveRoutes, defaultParentState, connections[j]); - liveRoutes = appended.liveRoutes; - let { name, outlet } = appended.ownState.render; - if (name === route.routeName || outlet === 'main') { - ownState = appended.ownState; - } + let render = getRenderState(route); + + if (render) { + let state: OutletState = { + render, + outlets: { + main: undefined, + }, + }; + + if (parent) { + parent.outlets.main = state; + } else { + root = state; } + + parent = 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; } - defaultParentState = ownState!; } // when a transitionTo happens after the validation phase @@ -643,7 +634,7 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { // when no routes are active. However, it will get called // again with the correct values during the next turn of // the runloop - if (!liveRoutes) { + if (root === null) { return; } @@ -668,7 +659,7 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { assert('[BUG] unexpectedly missing `template:-outlet`', template !== undefined); this._toplevelView = OutletView.create({ environment, template, application }); - this._toplevelView.setOutletState(liveRoutes as GlimmerOutletState); + this._toplevelView.setOutletState(root); // TODO(SAFETY): At least one test runs without this set correctly. At a // later time, update the test to configure this correctly. The test ID: @@ -684,7 +675,7 @@ class EmberRouter extends EmberObject.extend(Evented) implements Evented { instance.didCreateRootView(this._toplevelView as any); } } else { - this._toplevelView.setOutletState(liveRoutes as GlimmerOutletState); + this._toplevelView.setOutletState(root); } } @@ -1816,80 +1807,6 @@ function forEachQueryParam( } } -function findLiveRoute(liveRoutes: OutletState | null, name: string) { - if (!liveRoutes) { - return; - } - let stack = [liveRoutes]; - while (stack.length > 0) { - let test = stack.shift()!; - if (test.render.name === name) { - return test; - } - let outlets = test.outlets; - for (let outletName in outlets) { - stack.push(outlets[outletName]!); - } - } - - return; -} - -function appendLiveRoute( - liveRoutes: OutletState | null, - defaultParentState: OutletState | undefined, - renderOptions: RenderOptions -) { - let ownState: OutletState = { - render: renderOptions, - outlets: Object.create(null), - wasUsed: false, - }; - let target: OutletState | undefined; - if (renderOptions.into) { - target = findLiveRoute(liveRoutes, renderOptions.into); - } else { - target = defaultParentState; - } - if (target) { - set(target.outlets, renderOptions.outlet, ownState); - } else { - liveRoutes = ownState; - } - - return { - liveRoutes, - ownState, - }; -} - -function representEmptyRoute( - liveRoutes: OutletState | null, - defaultParentState: OutletState | undefined, - { routeName }: Route -): OutletState { - // the route didn't render anything - let alreadyAppended = findLiveRoute(liveRoutes, routeName); - if (alreadyAppended) { - // But some other route has already rendered our default - // template, so that becomes the default target for any - // children we may have. - return alreadyAppended; - } else { - // Create an entry to represent our default template name, - // just so other routes can target it and inherit its place - // in the outlet hierarchy. - defaultParentState!.outlets['main'] = { - render: { - name: routeName, - outlet: 'main', - }, - outlets: {}, - }; - return defaultParentState!; - } -} - EmberRouter.reopen({ didTransition: defaultDidTransition, willTransition: defaultWillTransition,