Skip to content

Commit

Permalink
[BUGFIX canary] Restore/deprecate outlet TemplateFactory support
Browse files Browse the repository at this point in the history
Fixes #20576

The test is a bit misleading in that, this is not actually restored
to support the very old 1.6.0 version of ember-test-helpers. The
issue is that while that particular spot of the invalid usage was
fixed in 1.6.1, other similarly invalid patterns have popped up in
the code since then. However, the old tests was probably a close
enough approximation of the general problem.

This is not to avoid fixing the actual issue, but it's just that it
was very difficult for us to support for a short time, and there
may be other code that copied the same invalid pattern elsewhere.

See also emberjs/ember-test-helpers#1445
  • Loading branch information
chancancode committed Nov 18, 2023
1 parent 704447b commit 9b2b615
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 1 deletion.
35 changes: 34 additions & 1 deletion 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 } from '@ember/debug';
import { assert, deprecate } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import type { CapturedArguments, DynamicScope } from '@glimmer/interfaces';
import { CurriedType } from '@glimmer/vm';
Expand All @@ -17,6 +17,7 @@ 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 @@ -120,6 +121,38 @@ function stateFor(
let template = render.template;
if (template === undefined) return null;

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

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
126 changes: 126 additions & 0 deletions packages/ember/tests/ember-test-helpers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,130 @@ module('@ember/test-helpers emulation test', function () {
});
});
});

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

function lookupTemplate(owner, templateFullName) {
let template = owner.lookup(templateFullName);
if (typeof template === 'function') return template(owner);
return template;
}

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!');
});
});
});
});

0 comments on commit 9b2b615

Please sign in to comment.