Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLEANUP] Remove all traces of named outlets code #20570

Merged
merged 2 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@ember/-internals/glimmer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 16 additions & 14 deletions packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ import type { OutletState } from '../utils/outlet';
import type OutletView from '../views/outlet';

function instrumentationPayload(def: OutletDefinitionState) {
return { object: `${def.name}:${def.outlet}` };
// "main" used to be the outlet name, keeping it around for compatibility
return { object: `${def.name}:main` };
}

interface OutletInstanceState {
self: Reference;
outlet?: { name: string };
outletBucket?: {};
engineBucket?: { mountPoint: string };
engine?: EngineInstance;
finalize: () => void;
Expand All @@ -46,7 +47,6 @@ interface OutletInstanceState {
export interface OutletDefinitionState {
ref: Reference<OutletState | undefined>;
name: string;
outlet: string;
template: Template;
controller: unknown;
model: unknown;
Expand Down Expand Up @@ -91,7 +91,8 @@ class OutletComponentManager
};

if (env.debugRenderTree !== undefined) {
state.outlet = { name: definition.outlet };
state.outletBucket = {};

let parentState = valueForRef(parentStateRef);
let parentOwner = parentState && parentState.render && parentState.render.owner;
let currentOwner = valueForRef(currentStateRef)!.render!.owner;
Expand Down Expand Up @@ -126,16 +127,17 @@ class OutletComponentManager
): CustomRenderNode[] {
let nodes: CustomRenderNode[] = [];

if (state.outlet) {
nodes.push({
bucket: state.outlet,
type: 'outlet',
name: state.outlet.name,
args: EMPTY_ARGS,
instance: undefined,
template: undefined,
});
}
assert('[BUG] outletBucket must be set', state.outletBucket);

nodes.push({
bucket: state.outletBucket,
type: 'outlet',
// "main" used to be the outlet name, keeping it around for compatibility
name: 'main',
args: EMPTY_ARGS,
instance: undefined,
template: undefined,
});

if (state.engineBucket) {
nodes.push({
Expand Down
12 changes: 1 addition & 11 deletions packages/@ember/-internals/glimmer/lib/syntax/outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { dict } from '@glimmer/util';
import type { OutletDefinitionState } from '../component-managers/outlet';
import { OutletComponentDefinition } from '../component-managers/outlet';
import { internalHelper } from '../helpers/internal-helper';
import { isTemplateFactory } from '../template';
import type { OutletState } from '../utils/outlet';

/**
Expand Down Expand Up @@ -53,9 +52,7 @@ export const outletHelper = internalHelper(

let outletRef = createComputeRef(() => {
let state = valueForRef(scope.get('outletState') as Reference<OutletState | undefined>);
let outlets = state !== undefined ? state.outlets : undefined;

return outlets !== undefined ? outlets['main'] : undefined;
return state?.outlets?.main;
});

let lastState: OutletDefinitionState | null = null;
Expand Down Expand Up @@ -120,16 +117,9 @@ function stateFor(ref: Reference, outlet: OutletState | undefined): OutletDefini
let template = render.template;
if (template === undefined) return null;

// this guard can be removed once @ember/test-helpers@1.6.0 has "aged out"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.6.0 😅

// and is no longer considered supported
if (isTemplateFactory(template)) {
template = template(render.owner);
}

return {
ref,
name: render.name,
outlet: render.outlet,
template,
controller: render.controller,
model: render.model,
Expand Down
75 changes: 54 additions & 21 deletions packages/@ember/-internals/glimmer/lib/utils/outlet.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,41 @@
import type { InternalOwner } from '@ember/-internals/owner';
import type { Template, TemplateFactory } from '@glimmer/interfaces';
import type { Template } from '@glimmer/interfaces';

// Note: a lot of these does not make sense anymore. This design was from back
// when we supported "named outlets", where a route can do:
//
// this.renderTemplate("some-template", {
// into: 'some-parent-route',
// outlet: 'some-name' /* {{outlet "some-name"}} */ | undefined /* {{outlet}} */,
// controller: 'some-controller' | SomeController,
// model: { ... },
// });
//
// And interface reflects that. Now that this is not supported anymore, each
// route implicitly renders into its immediate parent's `{{outlet}}` (no name).
// Keeping around most of these to their appropriately hardcoded values for the
// time being to minimize churn for external consumers, as we are about to rip
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

// all of it out anyway.

export interface RenderState {
/**
* Not sure why this is here, we use the owner of the template for lookups.
*
* Maybe this is for the render helper?
* This is usually inherited from the parent (all the way up to the app
* instance). However, engines uses this to swap out the owner when crossing
* a mount point.
*/
owner: InternalOwner;

/**
* The name of the parent outlet state.
* @deprecated This used to specify "which parent route to render into",
* which is not a thing anymore.
*/
into: string | undefined;
into: undefined;

/*
* The outlet name in the parent outlet state's outlets.
/**
* @deprecated This used to specify "which named outlet in the parent
* template to render into", which is not a thing anymore.
*/
outlet: string;
outlet: 'main';

/**
* The name of the route/template
Expand All @@ -37,27 +55,42 @@ export interface RenderState {
/**
* template (the layout of the outlet component)
*/
template: Template | TemplateFactory | undefined;
}

export interface Outlets {
[name: string]: OutletState | undefined;
template: Template | undefined;
}

export interface OutletState {
/**
* Nested outlet connections.
* Represents what was rendered into this outlet.
*/
outlets: Outlets;
render: RenderState | undefined;

/**
* Represents what was rendered into this outlet.
* Represents what, if any, should be rendered into the next {{outlet}} found
* at this level.
*
* This used to be a dictionary of children outlets, including the {{outlet}}
* "main" outlet any {{outlet "named"}} named outlets. Since named outlets
* are not a thing anymore, this can now just be a single`child`.
*/
render: RenderState | undefined;
outlets: {
main: OutletState | undefined;
};

/**
* Has to do with render helper and orphan outlets.
* Whether outlet state was rendered.
* @deprecated
*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really appreciate all these comments -- as someone who is just learning how the glue fits together, this is immensely helpful for provided much needed context as well as a bit of why

* This tracks whether this outlet state actually made it onto the page
* somewhere. This was more of a problem when you can declare named outlets
* left and right, and anything can render into anywhere else. We want to
* warn users when you tried to render into somewhere that does not exist,
* but we don't know what named outlets exists until after we have rendered
* everything, so this was used to track these orphan renders.
*
* This can still happen, if, according to the router, a route is active and
* so its template should be rendered, but the parent template is missing the
* `{{outlet}}` keyword, or that it was hidden by an `{{#if}}` or something.
* I guess that is considered valid, because nothing checks for this anymore.
* seems valid for the parent to decide not to render a child template?
*/
wasUsed?: boolean;
wasUsed?: undefined;
}
4 changes: 1 addition & 3 deletions packages/@ember/-internals/glimmer/lib/views/outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export interface BootEnvironment {
}

const TOP_LEVEL_NAME = '-top-level';
const TOP_LEVEL_OUTLET = 'main';

export default class OutletView {
static extend(injections: any): typeof OutletView {
Expand Down Expand Up @@ -69,7 +68,7 @@ export default class OutletView {
render: {
owner: owner,
into: undefined,
outlet: TOP_LEVEL_OUTLET,
outlet: 'main',
name: TOP_LEVEL_NAME,
controller: undefined,
model: undefined,
Expand All @@ -91,7 +90,6 @@ export default class OutletView {
this.state = {
ref,
name: TOP_LEVEL_NAME,
outlet: TOP_LEVEL_OUTLET,
template,
controller: undefined,
model: undefined,
Expand Down
Loading
Loading