Skip to content

Commit

Permalink
[FEATURE | BREAKING] Change semantics of in-element
Browse files Browse the repository at this point in the history
- Remove nextSibling in favor of insertBefore
- Change semantics of insertBefore to always clear the remote element before inserting
  • Loading branch information
chadhietala committed Mar 29, 2019
1 parent fca7da1 commit 22d1ea9
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 66 deletions.
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

0 comments on commit 22d1ea9

Please sign in to comment.