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 | BREAKING] Change semantics of in-element to match emberjs/rfcs#287 #918

Merged
merged 1 commit into from
Mar 29, 2019
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
73 changes: 39 additions & 34 deletions packages/@glimmer/integration-tests/lib/suites/in-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,29 @@ export class InElementSuite extends RenderTest {
this.assertStableNodes();
}

@test
'clears existing content'() {
let externalElement = this.delegate.createElement('div');
let initialContent = '<p>Hello there!</p>';
replaceHTML(externalElement, initialContent);

this.render('{{#in-element externalElement}}[{{foo}}]{{/in-element}}', {
externalElement,
foo: 'Yippie!',
});

equalsElement(externalElement, 'div', {}, '[Yippie!]');
this.assertStableRerender();

this.rerender({ foo: 'Double Yups!' });
equalsElement(externalElement, 'div', {}, '[Double Yups!]');
this.assertStableNodes();

this.rerender({ foo: 'Yippie!' });
equalsElement(externalElement, 'div', {}, '[Yippie!]');
this.assertStableNodes();
}

@test
'Changing to falsey'() {
let first = this.delegate.createElement('div');
Expand Down Expand Up @@ -79,10 +102,13 @@ export class InElementSuite extends RenderTest {
let initialContent = '<p>Hello there!</p>';
replaceHTML(externalElement, initialContent);

this.render(stripTight`{{#in-element externalElement}}[{{foo}}]{{/in-element}}`, {
externalElement,
foo: 'Yippie!',
});
this.render(
stripTight`{{#in-element externalElement insertBefore=null}}[{{foo}}]{{/in-element}}`,
{
externalElement,
foo: 'Yippie!',
}
);

equalsElement(externalElement, 'div', {}, `${initialContent}[Yippie!]`);
this.assertHTML('<!---->');
Expand All @@ -105,38 +131,17 @@ export class InElementSuite extends RenderTest {
}

@test
'With nextSibling'() {
'`insertBefore` can only be null'() {
let externalElement = this.delegate.createElement('div');
replaceHTML(externalElement, '<b>Hello</b><em>there!</em>');

this.render(
stripTight`{{#in-element externalElement nextSibling=nextSibling}}[{{foo}}]{{/in-element}}`,
{ externalElement, nextSibling: externalElement.lastChild, foo: 'Yippie!' }
);

equalsElement(externalElement, 'div', {}, '<b>Hello</b>[Yippie!]<em>there!</em>');
this.assertHTML('<!---->');
this.assertStableRerender();

this.rerender({ foo: 'Double Yips!' });
equalsElement(externalElement, 'div', {}, '<b>Hello</b>[Double Yips!]<em>there!</em>');
this.assertHTML('<!---->');
this.assertStableNodes();
let before = this.delegate.createElement('div');

this.rerender({ nextSibling: null });
equalsElement(externalElement, 'div', {}, '<b>Hello</b><em>there!</em>[Double Yips!]');
this.assertHTML('<!---->');
this.assertStableRerender();

this.rerender({ externalElement: null });
equalsElement(externalElement, 'div', {}, '<b>Hello</b><em>there!</em>');
this.assertHTML('<!---->');
this.assertStableRerender();

this.rerender({ externalElement, nextSibling: externalElement.lastChild, foo: 'Yippie!' });
equalsElement(externalElement, 'div', {}, '<b>Hello</b>[Yippie!]<em>there!</em>');
this.assertHTML('<!---->');
this.assertStableRerender();
this.assert.throws(() => {
this.render('{{#in-element externalElement insertBefore=before}}[{{foo}}]{{/in-element}}', {
externalElement,
before,
foo: 'Yippie!',
});
}, /insertBefore only takes `null` as an argument/);
}

@test
Expand Down
2 changes: 1 addition & 1 deletion packages/@glimmer/interfaces/lib/dom/attributes.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export interface DOMStack {
pushRemoteElement(
element: SimpleElement,
guid: string,
nextSibling: Option<SimpleNode>
insertBefore: Option<null>
): Option<RemoteLiveBlock>;
popRemoteElement(): void;
popElement(): void;
Expand Down
6 changes: 3 additions & 3 deletions packages/@glimmer/node/lib/serialize-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ class SerializeBuilder extends NewElementBuilder implements ElementBuilder {
pushRemoteElement(
element: SimpleElement,
cursorId: string,
nextSibling: Option<SimpleNode> = null
insertBefore: Option<null> = null
): Option<RemoteLiveBlock> {
let { dom } = this;
let script = dom.createElement('script');
script.setAttribute('glmr', cursorId);
dom.insertBefore(element, script, nextSibling);
return super.pushRemoteElement(element, cursorId, nextSibling);
dom.insertBefore(element, script, insertBefore);
return super.pushRemoteElement(element, cursorId, insertBefore);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/@glimmer/opcode-compiler/lib/syntax/builtins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export function populateBuiltins(

for (let i = 0; i < keys.length; i++) {
let key = keys[i];
if (key === 'nextSibling' || key === 'guid') {
if (key === 'guid' || key === 'insertBefore') {
actions.push(op('Expr', values[i]));
} else {
throw new Error(`SYNTAX ERROR: #in-element does not take a \`${keys[0]}\` option`);
Expand Down
20 changes: 6 additions & 14 deletions packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ import {
isConst,
isConstTag,
} from '@glimmer/reference';
import { Option } from '@glimmer/util';
import { check, CheckString, CheckElement, CheckNode, CheckOption } from '@glimmer/debug';
import { Op } from '@glimmer/interfaces';
import { check, CheckString, CheckElement } from '@glimmer/debug';
import { Op, Option } from '@glimmer/interfaces';
import { $t0 } from '@glimmer/vm';
import {
ModifierDefinition,
Expand All @@ -22,7 +21,7 @@ import { Assert } from './vm';
import { DynamicAttribute } from '../../vm/attributes/dynamic';
import { CheckReference, CheckArguments, CheckOperations } from './-debug-strip';
import { CONSTANTS } from '../../symbols';
import { SimpleElement, SimpleNode } from '@simple-dom/interface';
import { SimpleElement } from '@simple-dom/interface';

APPEND_OPCODES.add(Op.Text, (vm, { op1: text }) => {
vm.elements().appendText(vm[CONSTANTS].getString(text));
Expand All @@ -43,11 +42,10 @@ APPEND_OPCODES.add(Op.OpenDynamicElement, vm => {

APPEND_OPCODES.add(Op.PushRemoteElement, vm => {
let elementRef = check(vm.stack.pop(), CheckReference);
let nextSiblingRef = check(vm.stack.pop(), CheckReference);
let insertBeforeRef = check(vm.stack.pop(), CheckReference);
let guidRef = check(vm.stack.pop(), CheckReference);

let element: SimpleElement;
let nextSibling: Option<SimpleNode>;
let guid = guidRef.value() as string;

if (isConst(elementRef)) {
Expand All @@ -58,15 +56,9 @@ APPEND_OPCODES.add(Op.PushRemoteElement, vm => {
vm.updateWith(new Assert(cache));
}

if (isConst(nextSiblingRef)) {
nextSibling = check(nextSiblingRef.value(), CheckOption(CheckNode));
} else {
let cache = new ReferenceCache(nextSiblingRef as Reference<Option<SimpleNode>>);
nextSibling = check(cache.peek(), CheckOption(CheckNode));
vm.updateWith(new Assert(cache));
}
let insertBefore = insertBeforeRef.value() as Option<null>;

let block = vm.elements().pushRemoteElement(element, guid, nextSibling);
let block = vm.elements().pushRemoteElement(element, guid, insertBefore);
if (block) vm.associateDestroyable(block);
});

Expand Down
16 changes: 12 additions & 4 deletions packages/@glimmer/runtime/lib/vm/element-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,18 +205,26 @@ export class NewElementBuilder implements ElementBuilder {
pushRemoteElement(
element: SimpleElement,
guid: string,
nextSibling: Option<SimpleNode> = null
insertBefore: Option<null>
): Option<RemoteLiveBlock> {
return this.__pushRemoteElement(element, guid, nextSibling);
return this.__pushRemoteElement(element, guid, insertBefore);
}

__pushRemoteElement(
element: SimpleElement,
_guid: string,
nextSibling: Option<SimpleNode>
insertBefore: Option<null>
): Option<RemoteLiveBlock> {
this.pushElement(element, nextSibling);
this.pushElement(element, insertBefore);

if (insertBefore === undefined) {
while (element.lastChild) {
element.removeChild(element.lastChild);
}
}

let block = new RemoteLiveBlock(element);

return this.pushLiveBlock(block, true);
}

Expand Down
10 changes: 8 additions & 2 deletions packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,15 +379,21 @@ export class RehydrateBuilder extends NewElementBuilder implements ElementBuilde
__pushRemoteElement(
element: SimpleElement,
cursorId: string,
nextSibling: Option<SimpleNode> = null
insertBefore: Option<null>
): Option<RemoteLiveBlock> {
let marker = this.getMarker(element as HTMLElement, cursorId);

if (marker.parentNode === element) {
if (insertBefore === undefined) {
while (element.lastChild !== marker) {
element.removeChild(element.lastChild!);
}
}

let currentCursor = this.currentCursor;
let candidate = currentCursor!.candidate;

this.pushElement(element, nextSibling);
this.pushElement(element, insertBefore);

currentCursor!.candidate = candidate;
this.candidate = this.remove(marker);
Expand Down
18 changes: 11 additions & 7 deletions packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,25 +441,29 @@ function addElementModifier(element: Tag<'StartTag'>, mustache: AST.MustacheStat
}

function addInElementHash(cursor: string, hash: AST.Hash, loc: AST.SourceLocation) {
let hasNextSibling = false;
let hasInsertBefore = false;
hash.pairs.forEach(pair => {
if (pair.key === 'guid') {
throw new SyntaxError('Cannot pass `guid` from user space', loc);
}

if (pair.key === 'nextSibling') {
hasNextSibling = true;
if (pair.key === 'insertBefore') {
if (pair.value.type !== 'NullLiteral') {
throw new SyntaxError('insertBefore only takes `null` as an argument', loc);
}

hasInsertBefore = true;
}
});

let guid = b.literal('StringLiteral', cursor);
let guidPair = b.pair('guid', guid);
hash.pairs.unshift(guidPair);

if (!hasNextSibling) {
let nullLiteral = b.literal('NullLiteral', null);
let nextSibling = b.pair('nextSibling', nullLiteral);
hash.pairs.push(nextSibling);
if (!hasInsertBefore) {
let undefinedLiteral = b.literal('UndefinedLiteral', undefined);
let beforeSibling = b.pair('insertBefore', undefinedLiteral);
hash.pairs.push(beforeSibling);
}

return hash;
Expand Down