Skip to content

Commit

Permalink
[CLEANUP] Remove old code that supported old ember-test-helpers
Browse files Browse the repository at this point in the history
The actual behavior change was marked with an intimate deprecation
and has aged out. The actual versions of ember-test-helpers affected
seems to be around <= 3.2.0, which should be fully absorbed by now.

The other changes to `OutletState` doesn't actually change behavior,
those fields were mostly just kept around assuming we are keeping
this code frozen and to be deleted soon, so they documented what
those were on the off chance that someone came into look. Now that
it looks like this code may be here to stay for a bit longer, it's
worth cleaning it up to make things less confusing for core devs.

Follow-up to #20570
  • Loading branch information
chancancode committed Nov 22, 2024
1 parent cc5db96 commit baa459b
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 231 deletions.
37 changes: 1 addition & 36 deletions packages/@ember/-internals/glimmer/lib/syntax/outlet.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { InternalOwner } from '@ember/-internals/owner';
import { assert, deprecate } from '@ember/debug';
import { assert } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import type { CapturedArguments, DynamicScope } from '@glimmer/interfaces';
import { CurriedType } from '@glimmer/vm';
Expand All @@ -17,7 +17,6 @@ import type { OutletDefinitionState } from '../component-managers/outlet';
import { OutletComponentDefinition } from '../component-managers/outlet';
import { internalHelper } from '../helpers/internal-helper';
import type { OutletState } from '../utils/outlet';
import { isTemplateFactory } from '../template';

/**
The `{{outlet}}` helper lets you specify where a child route will render in
Expand Down Expand Up @@ -121,40 +120,6 @@ function stateFor(
let template = render.template;
if (template === undefined) return null;

if (isTemplateFactory(template)) {
template = template(render.owner);

if (DEBUG) {
let message =
'The `template` property of `OutletState` should be a ' +
'`Template` rather than a `TemplateFactory`. This is known to be a ' +
"problem in older versions of `@ember/test-helpers`. If you haven't " +
'done so already, try upgrading to the latest version.\n\n';

if (template.result === 'ok' && typeof template.moduleName === 'string') {
message +=
'The offending template has a moduleName `' +
template.moduleName +
'`, which might be helpful for identifying ' +
'source of this issue.\n\n';
}

message +=
'Please note that `OutletState` is a private API in Ember.js ' +
"and not meant to be used outside of the framework's internal code.";

deprecate(message, false, {
id: 'outlet-state-template-factory',
until: '5.9.0',
for: 'ember-source',
since: {
available: '5.6.0',
enabled: '5.6.0',
},
});
}
}

return {
ref,
name: render.name,
Expand Down
48 changes: 1 addition & 47 deletions packages/@ember/-internals/glimmer/lib/utils/outlet.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,6 @@
import type { InternalOwner } from '@ember/-internals/owner';
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
// all of it out anyway.

export interface RenderState {
/**
* This is usually inherited from the parent (all the way up to the app
Expand All @@ -25,18 +9,6 @@ export interface RenderState {
*/
owner: InternalOwner;

/**
* @deprecated This used to specify "which parent route to render into",
* which is not a thing anymore.
*/
into: undefined;

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

/**
* The name of the route/template
*/
Expand All @@ -53,7 +25,7 @@ export interface RenderState {
model: unknown;

/**
* template (the layout of the outlet component)
* The template (the route template to use in the {{outlet}})
*/
template: Template | undefined;
}
Expand All @@ -75,22 +47,4 @@ export interface OutletState {
outlets: {
main: OutletState | undefined;
};

/**
* @deprecated
*
* 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?: undefined;
}
2 changes: 0 additions & 2 deletions packages/@ember/-internals/glimmer/lib/views/outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ export default class OutletView {
outlets: { main: undefined },
render: {
owner: owner,
into: undefined,
outlet: 'main',
name: TOP_LEVEL_NAME,
controller: undefined,
model: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ moduleFor(
let outletState = {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'application',
controller: undefined,
template: undefined,
Expand All @@ -37,8 +35,6 @@ moduleFor(
let outletState = {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'application',
controller: undefined,
template: undefined,
Expand All @@ -57,8 +53,6 @@ moduleFor(
outletState = {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'application',
controller: {},
template: this.owner.lookup('template:application')(this.owner),
Expand All @@ -76,8 +70,6 @@ moduleFor(
outletState.outlets.main = {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'index',
controller: {},
template: this.owner.lookup('template:index')(this.owner),
Expand All @@ -95,8 +87,6 @@ moduleFor(
let outletState = {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'application',
controller: {},
template: this.owner.lookup('template:application')(this.owner),
Expand All @@ -116,8 +106,6 @@ moduleFor(
outletState.outlets.main = {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'index',
controller: {},
template: this.owner.lookup('template:index')(this.owner),
Expand Down Expand Up @@ -146,8 +134,6 @@ moduleFor(
let outletState = {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'outer',
controller: {},
template: this.owner.lookup('template:outer')(this.owner),
Expand All @@ -156,8 +142,6 @@ moduleFor(
main: {
render: {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'inner',
controller: {},
template: this.owner.lookup('template:inner')(this.owner),
Expand Down
2 changes: 0 additions & 2 deletions packages/@ember/routing/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1842,8 +1842,6 @@ function buildRenderState(route: Route): RenderState {

let render: RenderState = {
owner,
into: undefined,
outlet: 'main',
name,
controller,
model,
Expand Down
124 changes: 0 additions & 124 deletions packages/ember/tests/ember-test-helpers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ module('@ember/test-helpers emulation test', function () {
let outletState = {
render: {
owner,
into: undefined,
outlet: 'main',
name: 'application',
controller: undefined,
ViewClass: undefined,
Expand All @@ -97,8 +95,6 @@ module('@ember/test-helpers emulation test', function () {
main: {
render: {
owner,
into: undefined,
outlet: 'main',
name: 'index',
controller: context,
ViewClass: undefined,
Expand Down Expand Up @@ -138,124 +134,4 @@ module('@ember/test-helpers emulation test', function () {
});
});
});

module('v1.6.0', function () {
let EMPTY_TEMPLATE = compile('');

function settled() {
return new Promise(function (resolve) {
let watcher = setInterval(() => {
if (_getCurrentRunLoop() || _hasScheduledTimers()) {
return;
}

// Stop polling
clearInterval(watcher);

// Synchronously resolve the promise
run(null, resolve);
}, 10);
});
}

async function setupContext(context) {
// condensed version of https://github.com/emberjs/ember-test-helpers/blob/v1.6.0/addon-test-support/%40ember/test-helpers/build-owner.ts#L38
// without support for "custom resolver"
await context.application.boot();

context.owner = await context.application.buildInstance().boot();
}

function setupRenderingContext(context) {
let { owner } = context;
let OutletView = owner.factoryFor('view:-outlet');
let environment = owner.lookup('-environment:main');
let outletTemplateFactory = owner.lookup('template:-outlet');
let toplevelView = OutletView.create({ environment, template: outletTemplateFactory });

owner.register('-top-level-view:main', {
create() {
return toplevelView;
},
});

// initially render a simple empty template
return render(EMPTY_TEMPLATE, context).then(() => {
let rootElement = document.querySelector(owner.rootElement);
run(toplevelView, 'appendTo', rootElement);

context.element = rootElement;

return settled();
});
}

let templateId = 0;
function render(template, context) {
let { owner } = context;
let toplevelView = owner.lookup('-top-level-view:main');
templateId += 1;
let templateFullName = `template:-undertest-${templateId}`;
owner.register(templateFullName, template);

let outletState = {
render: {
owner,
into: undefined,
outlet: 'main',
name: 'application',
controller: undefined,
ViewClass: undefined,
template: owner.lookup('template:-outlet'),
},

outlets: {
main: {
render: {
owner,
into: undefined,
outlet: 'main',
name: 'index',
controller: context,
ViewClass: undefined,
template: owner.lookup(templateFullName),
outlets: {},
},
outlets: {},
},
},
};
toplevelView.setOutletState(outletState);

return settled();
}

module('setupRenderingContext', function (hooks) {
hooks.beforeEach(async function () {
expectDeprecation(
/The `template` property of `OutletState` should be a `Template` rather than a `TemplateFactory`/
);

this.application = Application.create({
rootElement: '#qunit-fixture',
autoboot: false,
Resolver: ModuleBasedTestResolver,
});

await setupContext(this);
await setupRenderingContext(this);
});

hooks.afterEach(function () {
run(this.owner, 'destroy');
run(this.application, 'destroy');
});

test('it basically works', async function (assert) {
await render(compile('Hi!'), this);

assert.equal(this.element.textContent, 'Hi!');
});
});
});
});
4 changes: 0 additions & 4 deletions tests/node/helpers/setup-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ function setupComponentTest() {
this._outletState = {
render: {
owner: module.owner || undefined,
into: undefined,
outlet: 'main',
name: 'application',
controller: module,
model: undefined,
Expand Down Expand Up @@ -83,8 +81,6 @@ function render(_template) {

let stateToRender = {
owner: this.owner,
into: undefined,
outlet: 'main',
name: 'index',
controller: this,
model: undefined,
Expand Down

0 comments on commit baa459b

Please sign in to comment.