Skip to content

Commit 1b6a074

Browse files
committed
[BUGFIX] Process in-element in compiler not parser
This fixes the issue described in DockYard/ember-maybe-in-element#25 (comment) tl;dr If an AST transform introduces the element, it doesn't go through the parser, so it's missing the required `guid` and `insertBefore` normalization. This commit moves the processing to a later stage to avoid this issue.
1 parent 46f881b commit 1b6a074

File tree

4 files changed

+96
-50
lines changed

4 files changed

+96
-50
lines changed

packages/@glimmer/compiler/lib/template-compiler.ts

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ function isTrustedValue(value: any) {
1818
return value.escaped !== undefined && !value.escaped;
1919
}
2020

21-
export const THIS = 0;
22-
2321
export default class TemplateCompiler implements Processor<InputOps> {
2422
static compile(ast: AST.Template, source: string, options?: CompileOptions): Template {
2523
let templateVisitor = new TemplateVisitor();
@@ -49,6 +47,12 @@ export default class TemplateCompiler implements Processor<InputOps> {
4947
private locations: Option<SourceLocation>[] = [];
5048
private includeMeta = true;
5149

50+
private cursorCount = 0;
51+
52+
cursor() {
53+
return `%cursor:${this.cursorCount++}%`;
54+
}
55+
5256
process(
5357
actions: Action[]
5458
): { opcodes: readonly Ops<AllocateSymbolsOps>[]; locations: readonly Option<SourceLocation>[] } {
@@ -62,6 +66,7 @@ export default class TemplateCompiler implements Processor<InputOps> {
6266
}
6367

6468
startProgram([program]: [AST.Template]) {
69+
this.cursorCount = 0;
6570
this.opcode(['startProgram', program], program);
6671
}
6772

@@ -242,7 +247,12 @@ export default class TemplateCompiler implements Processor<InputOps> {
242247
}
243248

244249
block([action /*, index, count*/]: [AST.BlockStatement]) {
245-
this.prepareHelper(action, 'block');
250+
if (isInElement(action)) {
251+
this.prepareHelper(action, 'in-element');
252+
} else {
253+
this.prepareHelper(action, 'block');
254+
}
255+
246256
let templateId = this.templateIds.pop()!;
247257
let inverseId = action.inverse === null ? null : this.templateIds.pop()!;
248258
this.expression(action.path, ExpressionContext.BlockHead, action);
@@ -403,12 +413,12 @@ export default class TemplateCompiler implements Processor<InputOps> {
403413
this.opcode(['helper'], call);
404414
}
405415

406-
prepareHelper(expr: AST.Call, context: string) {
416+
prepareHelper(expr: AST.Call, context: 'helper' | 'modifier' | 'block' | 'in-element') {
407417
assertIsSimplePath(expr.path, expr.loc, context);
408418

409419
let { params, hash } = expr;
410420

411-
this.prepareHash(hash);
421+
this.prepareHash(hash, context);
412422
this.prepareParams(params);
413423
}
414424

@@ -428,23 +438,51 @@ export default class TemplateCompiler implements Processor<InputOps> {
428438
this.opcode(['prepareArray', params.length], null);
429439
}
430440

431-
prepareHash(hash: AST.Hash) {
441+
prepareHash(hash: AST.Hash, context: 'helper' | 'modifier' | 'block' | 'in-element') {
432442
let pairs = hash.pairs;
443+
let length = pairs.length;
433444

434-
if (!pairs.length) {
435-
this.opcode(['literal', null], null);
436-
return;
437-
}
445+
let isInElement = context === 'in-element';
446+
let hasInsertBefore = false;
438447

439-
for (let i = pairs.length - 1; i >= 0; i--) {
448+
for (let i = length - 1; i >= 0; i--) {
440449
let { key, value } = pairs[i];
441450

451+
if (isInElement) {
452+
if (key === 'guid') {
453+
throw new SyntaxError(
454+
`Cannot pass \`guid\` to \`{{#in-element}}\` on line ${value.loc.start.line}.`,
455+
value.loc
456+
);
457+
}
458+
459+
if (key === 'insertBefore') {
460+
hasInsertBefore = true;
461+
}
462+
}
463+
442464
assert(this[value.type], `Unimplemented ${value.type} on TemplateCompiler`);
443465
this[value.type](value as any);
444-
this.opcode(['literal', key], null);
466+
this.opcode(['literal', key]);
445467
}
446468

447-
this.opcode(['prepareObject', pairs.length], null);
469+
if (isInElement) {
470+
if (!hasInsertBefore) {
471+
this.opcode(['literal', undefined]);
472+
this.opcode(['literal', 'insertBefore']);
473+
length++;
474+
}
475+
476+
this.opcode(['literal', this.cursor()]);
477+
this.opcode(['literal', 'guid']);
478+
length++;
479+
}
480+
481+
if (length === 0) {
482+
this.opcode(['literal', null]);
483+
} else {
484+
this.opcode(['prepareObject', length]);
485+
}
448486
}
449487

450488
prepareAttributeValue(value: AST.AttrNode['value']): value is AST.TextNode {
@@ -575,6 +613,12 @@ function isPath(node: AST.Node | AST.PathExpression): node is AST.PathExpression
575613
return node.type === 'PathExpression';
576614
}
577615

616+
function isInElement(
617+
node: AST.BlockStatement
618+
): node is AST.BlockStatement & { path: AST.PathExpression } {
619+
return isPath(node.path) && node.path.original === 'in-element';
620+
}
621+
578622
function destructureDynamicComponent(element: AST.ElementNode): Option<AST.PathExpression> {
579623
let open = element.tag.charAt(0);
580624

packages/@glimmer/integration-tests/lib/suites/in-element.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { AST } from '@glimmer/syntax';
2+
import { assign } from '@glimmer/util';
13
import { RenderTest } from '../render-test';
24
import { test } from '../test-decorator';
35
import { equalsElement } from '../dom/assertions';
@@ -8,6 +10,41 @@ import { EmberishCurlyComponent } from '../components/emberish-curly';
810
export class InElementSuite extends RenderTest {
911
static suiteName = '#in-element';
1012

13+
@test
14+
'It works with AST transforms'() {
15+
this.registerPlugin(env => ({
16+
name: 'maybe-in-element',
17+
visitor: {
18+
BlockStatement(node: AST.BlockStatement) {
19+
let b = env.syntax.builders;
20+
let { path, ...rest } = node;
21+
if (path.type !== 'SubExpression' && path.original === 'maybe-in-element') {
22+
return assign({ path: b.path('in-element', path.loc) }, rest);
23+
} else {
24+
return node;
25+
}
26+
},
27+
},
28+
}));
29+
30+
let externalElement = this.delegate.createElement('div');
31+
this.render('{{#maybe-in-element externalElement}}[{{foo}}]{{/maybe-in-element}}', {
32+
externalElement,
33+
foo: 'Yippie!',
34+
});
35+
36+
equalsElement(externalElement, 'div', {}, '[Yippie!]');
37+
this.assertStableRerender();
38+
39+
this.rerender({ foo: 'Double Yups!' });
40+
equalsElement(externalElement, 'div', {}, '[Double Yups!]');
41+
this.assertStableNodes();
42+
43+
this.rerender({ foo: 'Yippie!' });
44+
equalsElement(externalElement, 'div', {}, '[Yippie!]');
45+
this.assertStableNodes();
46+
}
47+
1148
@test
1249
'Renders curlies into external element'() {
1350
let externalElement = this.delegate.createElement('div');

packages/@glimmer/opcode-compiler/lib/syntax/builtins.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ export function populateBuiltins(
167167

168168
return ReplayableIf({
169169
args() {
170+
assert(hash !== null, '[BUG] `{{#in-element}}` should have non-empty hash');
171+
170172
let [keys, values] = hash!;
171173

172174
let actions: StatementCompileActions = [];

packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@ export abstract class HandlebarsNodeVisitors extends Parser {
1313
abstract beginAttributeValue(quoted: boolean): void;
1414
abstract finishAttributeValue(): void;
1515

16-
cursorCount = 0;
17-
18-
cursor() {
19-
return `%cursor:${this.cursorCount++}%`;
20-
}
21-
2216
private get isTopLevel() {
2317
return this.elementStack.length === 0;
2418
}
@@ -28,8 +22,6 @@ export abstract class HandlebarsNodeVisitors extends Parser {
2822
Program(program: HBS.Program): AST.Template | AST.Block;
2923
Program(program: HBS.Program): AST.Block | AST.Template {
3024
let body: AST.Statement[] = [];
31-
this.cursorCount = 0;
32-
3325
let node;
3426

3527
if (this.isTopLevel) {
@@ -86,10 +78,6 @@ export abstract class HandlebarsNodeVisitors extends Parser {
8678
let program = this.Program(block.program);
8779
let inverse = block.inverse ? this.Program(block.inverse) : null;
8880

89-
if (path.original === 'in-element') {
90-
hash = addInElementHash(this.cursor(), hash, block.loc);
91-
}
92-
9381
let node = b.block(
9482
path,
9583
params,
@@ -444,31 +432,6 @@ function addElementModifier(element: Tag<'StartTag'>, mustache: AST.MustacheStat
444432
element.modifiers.push(modifier);
445433
}
446434

447-
function addInElementHash(cursor: string, hash: AST.Hash, loc: AST.SourceLocation) {
448-
let hasInsertBefore = false;
449-
hash.pairs.forEach(pair => {
450-
if (pair.key === 'guid') {
451-
throw new SyntaxError('Cannot pass `guid` from user space', loc);
452-
}
453-
454-
if (pair.key === 'insertBefore') {
455-
hasInsertBefore = true;
456-
}
457-
});
458-
459-
let guid = b.literal('StringLiteral', cursor);
460-
let guidPair = b.pair('guid', guid);
461-
hash.pairs.unshift(guidPair);
462-
463-
if (!hasInsertBefore) {
464-
let undefinedLiteral = b.literal('UndefinedLiteral', undefined);
465-
let beforeSibling = b.pair('insertBefore', undefinedLiteral);
466-
hash.pairs.push(beforeSibling);
467-
}
468-
469-
return hash;
470-
}
471-
472435
function appendDynamicAttributeValuePart(attribute: Attribute, part: AST.MustacheStatement) {
473436
attribute.isDynamic = true;
474437
attribute.parts.push(part);

0 commit comments

Comments
 (0)