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

[FEATURE ember-glimmer-remove-application-template-wrapper] #15981

Merged
merged 1 commit into from
Dec 14, 2017
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
5 changes: 5 additions & 0 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ for a detailed explanation.
Add `{{@foo}}` syntax to access named arguments in component templates per
[RFC](https://github.com/emberjs/rfcs/pull/276).

* `ember-glimmer-remove-application-template-wrapper`

Remove the `<div>` wrapper around the application template per
[RFC](https://github.com/emberjs/rfcs/pull/280).

* `ember-glimmer-template-only-components`

Use Glimmer Components semantics for template-only components per
Expand Down
1 change: 1 addition & 0 deletions features.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"ember-libraries-isregistered": null,
"ember-improved-instrumentation": null,
"ember-glimmer-named-arguments": null,
"ember-glimmer-remove-application-template-wrapper": null,
"ember-glimmer-template-only-components": null,
"ember-routing-router-service": true,
"ember-engines-mount-params": true,
Expand Down
5 changes: 2 additions & 3 deletions packages/ember-application/tests/system/application_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,9 @@ moduleFor('Ember.Application, default resolver with autoboot', class extends Def
}

[`@test Minimal Application initialized with just an application template`](assert) {
this.$().html('<script type="text/x-handlebars">Hello World</script>');
jQuery('#qunit-fixture').html('<script type="text/x-handlebars">Hello World</script>');
this.runTask(() => this.createApplication());

assert.equal(this.$().text().trim(), 'Hello World');
this.assertInnerHTML('Hello World');
}

});
Expand Down
6 changes: 3 additions & 3 deletions packages/ember-application/tests/system/visit_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ moduleFor('Ember.Application - visit()', class extends ApplicationTestCase {
'promise is resolved with an ApplicationInstance'
);
assert.equal(
this.$('.ember-view h1').text(), 'Hello world',
this.$('h1').text(), 'Hello world',
'the application was rendered once the promise resolves'
);
});
Expand Down Expand Up @@ -576,8 +576,8 @@ moduleFor('Ember.Application - visit()', class extends ApplicationTestCase {
}
}));

let $foo = jQuery('<div />').appendTo(this.$());
let $bar = jQuery('<div />').appendTo(this.$());
let $foo = jQuery('<div />').appendTo('#qunit-fixture');
let $bar = jQuery('<div />').appendTo('#qunit-fixture');

let data = encodeURIComponent(JSON.stringify({ name: 'Godfrey' }));
let instances = [];
Expand Down
1 change: 1 addition & 0 deletions packages/ember-glimmer/externs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ declare module 'ember/features' {
export const GLIMMER_CUSTOM_COMPONENT_MANAGER: boolean | null;
export const EMBER_ENGINES_MOUNT_PARAMS: boolean | null;
export const EMBER_GLIMMER_DETECT_BACKTRACKING_RERENDER: boolean | null;
export const EMBER_GLIMMER_REMOVE_APPLICATION_TEMPLATE_WRAPPER: boolean | null;
export const EMBER_GLIMMER_TEMPLATE_ONLY_COMPONENTS: boolean | null;
export const MANDATORY_SETTER: boolean | null;
}
Expand Down
50 changes: 28 additions & 22 deletions packages/ember-glimmer/lib/component-managers/outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Destroyable } from '@glimmer/util/dist/types';
import { DEBUG } from 'ember-env-flags';
import { _instrumentStart } from 'ember-metal';
import { generateGuid, guidFor } from 'ember-utils';
import { EMBER_GLIMMER_REMOVE_APPLICATION_TEMPLATE_WRAPPER } from 'ember/features';
import EmberEnvironment from '../environment';
import {
OwnedTemplate,
Expand Down Expand Up @@ -92,13 +93,35 @@ class TopLevelOutletComponentManager extends OutletComponentManager {
}
return new StateBucket(dynamicScope.outletState.value());
}

layoutFor(definition: OutletComponentDefinition, _bucket: StateBucket, env: Environment) {
return (env as EmberEnvironment).getCompiledBlock(TopLevelOutletLayoutCompiler, definition.template);
}
}

const TOP_LEVEL_MANAGER = new TopLevelOutletComponentManager();
const TOP_LEVEL_MANAGER = (() => {
if (EMBER_GLIMMER_REMOVE_APPLICATION_TEMPLATE_WRAPPER) {
return new TopLevelOutletComponentManager();
} else {
class WrappedTopLevelOutletLayoutCompiler {
static id = 'wrapped-top-level-outlet';

constructor(public template: WrappedTemplateFactory) {
}

compile(builder: any) {
builder.wrapLayout(this.template);
builder.tag.static('div');
builder.attrs.static('id', guidFor(this));
builder.attrs.static('class', 'ember-view');
}
}

class WrappedTopLevelOutletComponentManager extends TopLevelOutletComponentManager {
layoutFor(definition: OutletComponentDefinition, _bucket: StateBucket, env: Environment) {
return (env as EmberEnvironment).getCompiledBlock(WrappedTopLevelOutletLayoutCompiler, definition.template);
}
}

return new WrappedTopLevelOutletComponentManager();
}
})();

export class TopLevelOutletComponentDefinition extends ComponentDefinition<StateBucket> {
public template: WrappedTemplateFactory;
Expand All @@ -109,23 +132,6 @@ export class TopLevelOutletComponentDefinition extends ComponentDefinition<State
}
}

class TopLevelOutletLayoutCompiler {
static id: string;
public template: WrappedTemplateFactory;
constructor(template: WrappedTemplateFactory) {
this.template = template;
}

compile(builder: any) {
builder.wrapLayout(this.template);
builder.tag.static('div');
builder.attrs.static('id', guidFor(this));
builder.attrs.static('class', 'ember-view');
}
}

TopLevelOutletLayoutCompiler.id = 'top-level-outlet';

export class OutletComponentDefinition extends ComponentDefinition<StateBucket> {
public outletName: string;
public template: OwnedTemplate;
Expand Down
117 changes: 57 additions & 60 deletions packages/ember-glimmer/tests/integration/application/rendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,23 @@ import { moduleFor, ApplicationTest } from '../../utils/test-case';
import { strip } from '../../utils/abstract-test-case';
import { Route } from 'ember-routing';
import { Component } from 'ember-glimmer';
import { jQuery } from 'ember-views';

moduleFor('Application test: rendering', class extends ApplicationTest {

['@test it can render the application template'](assert) {
['@feature(!ember-glimmer-remove-application-template-wrapper) it can render the application template'](assert) {
this.addTemplate('application', 'Hello world!');

return this.visit('/').then(() => {
this.assertText('Hello world!');
this.assertComponentElement(this.element, { content: 'Hello world!' });
});
}

['@feature(ember-glimmer-remove-application-template-wrapper) it can render the application template'](assert) {
this.addTemplate('application', 'Hello world!');

return this.visit('/').then(() => {
this.assertInnerHTML('Hello world!');
});
}

Expand All @@ -30,15 +39,13 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
`);

return this.visit('/').then(() => {
this.assertComponentElement(this.firstChild, {
content: strip`
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
`
});
this.assertInnerHTML(strip`
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
`);
});
}

Expand Down Expand Up @@ -67,15 +74,13 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
`);

return this.visit('/lists/colors/favorite').then(() => {
this.assertComponentElement(this.firstChild, {
content: strip`
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
`
});
this.assertInnerHTML(strip`
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
`);
});
}

Expand Down Expand Up @@ -118,20 +123,18 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
`);

return this.visit('/colors').then(() => {
this.assertComponentElement(this.firstChild, {
content: strip`
<nav>
<a href="https://emberjs.com/">Ember</a>
</nav>
<main>
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
</main>
`
});
this.assertInnerHTML(strip`
<nav>
<a href="https://emberjs.com/">Ember</a>
</nav>
<main>
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
</main>
`);
});
}

Expand Down Expand Up @@ -174,20 +177,18 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
`);

return this.visit('/colors').then(() => {
this.assertComponentElement(this.firstChild, {
content: strip`
<nav>
<a href="https://emberjs.com/">Ember</a>
</nav>
<main>
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
</main>
`
});
this.assertInnerHTML(strip`
<nav>
<a href="https://emberjs.com/">Ember</a>
</nav>
<main>
<ul>
<li>red</li>
<li>yellow</li>
<li>blue</li>
</ul>
</main>
`);
});
}

Expand Down Expand Up @@ -233,11 +234,11 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
this.addTemplate('color', 'color: {{model}}');

return this.visit('/colors/red').then(() => {
this.assertComponentElement(this.firstChild, { content: 'color: red' });
this.assertInnerHTML('color: red');
this.takeSnapshot();
return this.visit('/colors/green');
}).then(() => {
this.assertComponentElement(this.firstChild, { content: 'color: green' });
this.assertInnerHTML('color: green');
this.assertInvariants();
});
}
Expand Down Expand Up @@ -291,12 +292,10 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
this.addTemplate('color', 'model color: {{model.color}}, controller color: {{color}}');

return this.visit('/colors/red').then(() => {
this.assertComponentElement(this.firstChild, { content: 'model color: red, controller color: red' });
this.takeSnapshot();
this.assertInnerHTML('model color: red, controller color: red');
return this.visit('/colors/green');
}).then(() => {
this.assertComponentElement(this.firstChild, { content: 'model color: green, controller color: green' });
this.assertInvariants();
this.assertInnerHTML('model color: green, controller color: green');
});
}

Expand Down Expand Up @@ -333,11 +332,11 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
this.addTemplate('common', '{{prefix}} {{model}}');

return this.visit('/a').then(() => {
this.assertComponentElement(this.firstChild, { content: 'common A' });
this.assertInnerHTML('common A');
this.takeSnapshot();
return this.visit('/b');
}).then(() => {
this.assertComponentElement(this.firstChild, { content: 'common B' });
this.assertInnerHTML('common B');
this.assertInvariants();
});
}
Expand All @@ -350,7 +349,7 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
this.addTemplate('application', '{{outlet}}');
this.addTemplate('index', '{{#if true}}1{{/if}}<div>2</div>');
return this.visit('/').then(() => {
this.assertComponentElement(this.firstChild, { content: '1<div>2</div>' });
this.assertInnerHTML('1<div>2</div>');
});
}

Expand All @@ -368,9 +367,7 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
this.addTemplate('a', 'Hello from A!');

return this.visit('/').then(() => {
this.assertComponentElement(this.firstChild, {
content: `Hello from A!`
});
this.assertInnerHTML('Hello from A!');
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ moduleFor('Link-to component with query-params', class extends ApplicationTest {
this.addTemplate('index', `{{#link-to 'index' (query-params foo='456' bar='NAW')}}Index{{/link-to}}`);

return this.visit('/').then(() => {
this.assertComponentElement(this.firstChild.firstElementChild, {
this.assertComponentElement(this.firstChild, {
tagName: 'a',
attrs: { href: '/?bar=NAW&foo=456' },
content: 'Index'
Expand All @@ -130,7 +130,7 @@ moduleFor('Link-to component with query-params', class extends ApplicationTest {
this.addTemplate('index', `{{#link-to 'index' (query-params foo='123')}}Index{{/link-to}}`);

return this.visit('/').then(() => {
this.assertComponentElement(this.firstChild.firstElementChild, {
this.assertComponentElement(this.firstChild, {
tagName: 'a',
attrs: { href: '/', class: classMatcher('ember-view active') },
content: 'Index'
Expand Down
Loading