From baa459b46da888d63e488ce111569fabde7b7e02 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Thu, 21 Nov 2024 17:21:56 -0800 Subject: [PATCH] [CLEANUP] Remove old code that supported old ember-test-helpers 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 --- .../-internals/glimmer/lib/syntax/outlet.ts | 37 +----- .../-internals/glimmer/lib/utils/outlet.ts | 48 +------ .../-internals/glimmer/lib/views/outlet.ts | 2 - .../glimmer/tests/integration/outlet-test.js | 16 --- packages/@ember/routing/route.ts | 2 - .../ember/tests/ember-test-helpers-test.js | 124 ------------------ tests/node/helpers/setup-component.js | 4 - 7 files changed, 2 insertions(+), 231 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts index 669ba8e58b9..debf7a97eb9 100644 --- a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts @@ -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'; @@ -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 @@ -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, diff --git a/packages/@ember/-internals/glimmer/lib/utils/outlet.ts b/packages/@ember/-internals/glimmer/lib/utils/outlet.ts index 7fe032ae51f..741c4a20fd7 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/outlet.ts @@ -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 @@ -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 */ @@ -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; } @@ -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; } diff --git a/packages/@ember/-internals/glimmer/lib/views/outlet.ts b/packages/@ember/-internals/glimmer/lib/views/outlet.ts index ced2115585d..dc97e2b6276 100644 --- a/packages/@ember/-internals/glimmer/lib/views/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/views/outlet.ts @@ -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, diff --git a/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js b/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js index 193ee68354c..fc17ee2730e 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js @@ -16,8 +16,6 @@ moduleFor( let outletState = { render: { owner: this.owner, - into: undefined, - outlet: 'main', name: 'application', controller: undefined, template: undefined, @@ -37,8 +35,6 @@ moduleFor( let outletState = { render: { owner: this.owner, - into: undefined, - outlet: 'main', name: 'application', controller: undefined, template: undefined, @@ -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), @@ -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), @@ -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), @@ -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), @@ -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), @@ -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), diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index c407f629caf..cfab81cfe5a 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -1842,8 +1842,6 @@ function buildRenderState(route: Route): RenderState { let render: RenderState = { owner, - into: undefined, - outlet: 'main', name, controller, model, diff --git a/packages/ember/tests/ember-test-helpers-test.js b/packages/ember/tests/ember-test-helpers-test.js index d08e271cea6..5dee7cfe4c5 100644 --- a/packages/ember/tests/ember-test-helpers-test.js +++ b/packages/ember/tests/ember-test-helpers-test.js @@ -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, @@ -97,8 +95,6 @@ module('@ember/test-helpers emulation test', function () { main: { render: { owner, - into: undefined, - outlet: 'main', name: 'index', controller: context, ViewClass: undefined, @@ -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!'); - }); - }); - }); }); diff --git a/tests/node/helpers/setup-component.js b/tests/node/helpers/setup-component.js index 174c75c0b54..7f21d180a50 100644 --- a/tests/node/helpers/setup-component.js +++ b/tests/node/helpers/setup-component.js @@ -53,8 +53,6 @@ function setupComponentTest() { this._outletState = { render: { owner: module.owner || undefined, - into: undefined, - outlet: 'main', name: 'application', controller: module, model: undefined, @@ -83,8 +81,6 @@ function render(_template) { let stateToRender = { owner: this.owner, - into: undefined, - outlet: 'main', name: 'index', controller: this, model: undefined,