From 414b052f34ac88b421149d3b44a400451ebe4a87 Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Mon, 6 Apr 2020 09:17:22 +0100 Subject: [PATCH 01/12] add named block tests for emberish curly component --- .../lib/suites/emberish-components.ts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/@glimmer/integration-tests/lib/suites/emberish-components.ts b/packages/@glimmer/integration-tests/lib/suites/emberish-components.ts index b46655ce24..195eaaac8d 100644 --- a/packages/@glimmer/integration-tests/lib/suites/emberish-components.ts +++ b/packages/@glimmer/integration-tests/lib/suites/emberish-components.ts @@ -312,6 +312,78 @@ export class EmberishComponentTests extends RenderTest { this.assertStableRerender(); } + // LOCKS + @test({ kind: 'curly' }) + 'yields named block'() { + class FooBar extends EmberishCurlyComponent { + [index: string]: unknown; + + constructor() { + super(); + } + } + this.registerComponent('Curly', 'FooBar', 'Hello{{yield to="baz"}}world!', FooBar); + + this.render(`<:baz> my `); + + this.assertComponent('Hello my world!'); + this.assertStableRerender(); + } + + // LOCKS + @test({ kind: 'curly' }) + 'implicit default named block'() { + class FooBar extends EmberishCurlyComponent { + [index: string]: unknown; + + constructor() { + super(); + } + } + this.registerComponent('Curly', 'FooBar', 'Hello{{yield}}world!', FooBar); + + this.render(` my `); + + this.assertComponent('Hello my world!'); + this.assertStableRerender(); + } + + // LOCKS + @test({ kind: 'curly' }) + 'explicit default named block'() { + class FooBar extends EmberishCurlyComponent { + [index: string]: unknown; + + constructor() { + super(); + } + } + this.registerComponent('Curly', 'FooBar', 'Hello{{yield to="default"}}world!', FooBar); + + this.render(`<:default> my `); + + this.assertComponent('Hello my world!'); + this.assertStableRerender(); + } + + // LOCKS + @test({ kind: 'curly' }) + 'else named block'() { + class FooBar extends EmberishCurlyComponent { + [index: string]: unknown; + + constructor() { + super(); + } + } + this.registerComponent('Curly', 'FooBar', 'Hello{{yield to="inverse"}}world!', FooBar); + + this.render(`<:else> my `); + + this.assertComponent('Hello my world!'); + this.assertStableRerender(); + } + @test({ kind: 'curly' }) 'invoking wrapped layout via angle brackets - invocation attributes merges classes'() { class FooBar extends EmberishCurlyComponent { From e5b578954fc01da65dc373f4b9851b2b7b073b48 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Tue, 7 Apr 2020 15:37:32 -0400 Subject: [PATCH 02/12] WIP - Test passes... Co-authored-by: Yehuda Katz Co-authored-by: Ricardo Mendes --- .../lib/opcode-builder/helpers/shared.ts | 17 ++++++--- .../@glimmer/opcode-compiler/lib/utils.ts | 8 +++++ .../runtime/lib/compiled/opcodes/component.ts | 36 +++++++++++++------ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts index 98ac4dcdfb..8adb2a0e15 100644 --- a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts +++ b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts @@ -30,18 +30,25 @@ export function CompileArgs({ }: ArgsOptions): StatementCompileActions { let out: StatementCompileActions = []; - if (blocks.hasAny) { - out.push(PushYieldableBlock(blocks.get('default'))); - out.push(PushYieldableBlock(blocks.get('else'))); - out.push(PushYieldableBlock(blocks.get('attrs'))); + let blockNames: string[] = ((blocks as any) as { names: string[] }).names; + for (let i = 0; i < blockNames.length; i++) { + out.push(PushYieldableBlock(blocks.get(blockNames[i]))); } + //if (blocks.hasAny) { + // out.push(PushYieldableBlock(blocks.get('default'))); + // out.push(PushYieldableBlock(blocks.get('else'))); + // out.push(PushYieldableBlock(blocks.get('attrs'))); + //} + let { count, actions } = CompilePositional(params); out.push(actions); let flags = count << 4; + // 10110101010 [X][X][X][X] + if (atNames) flags |= 0b1000; if (blocks) { @@ -58,7 +65,7 @@ export function CompileArgs({ } } - out.push(op(Op.PushArgs, strArray(names), flags)); + out.push(op(Op.PushArgs, strArray(names), strArray(blockNames), flags)); return out; } diff --git a/packages/@glimmer/opcode-compiler/lib/utils.ts b/packages/@glimmer/opcode-compiler/lib/utils.ts index 480909af7c..b653ab93a9 100644 --- a/packages/@glimmer/opcode-compiler/lib/utils.ts +++ b/packages/@glimmer/opcode-compiler/lib/utils.ts @@ -38,6 +38,14 @@ export class NamedBlocksImpl implements NamedBlocks { } } + get names() { + if (this.blocks != null) { + return Object.keys(this.blocks); + } else { + return null; + } + } + get hasAny(): boolean { return this.blocks !== null; } diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index 9085cece2a..24e3dbd218 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -1,4 +1,4 @@ -import { +¬import { check, CheckFunction, CheckHandle, @@ -39,7 +39,16 @@ import { } from '@glimmer/interfaces'; import { VersionedPathReference, VersionedReference } from '@glimmer/reference'; import { CONSTANT_TAG, isConst, isConstTag, Tag } from '@glimmer/validator'; -import { assert, dict, expect, Option, unreachable, symbol, unwrapTemplate } from '@glimmer/util'; +import { + assert, + dict, + expect, + Option, + unreachable, + symbol, + unwrapTemplate, + EMPTY_ARRAY, +} from '@glimmer/util'; import { $t0, $t1, $v0 } from '@glimmer/vm'; import { Capability, @@ -226,17 +235,23 @@ APPEND_OPCODES.add(Op.PushCurriedComponent, vm => { stack.push(definition); }); -APPEND_OPCODES.add(Op.PushArgs, (vm, { op1: _names, op2: flags }) => { +APPEND_OPCODES.add(Op.PushArgs, (vm, { op1: _names, op2: _blockNames, op3: flags }) => { let stack = vm.stack; let names = vm[CONSTANTS].getStringArray(_names); + // 10110101010 [X][X][X][X] let positionalCount = flags >> 4; + // 10110101010 [X][X][X][X] let atNames = flags & 0b1000; - let blockNames: string[] = []; + // let blockNames: string[] = []; + + // there are blocks - if (flags & 0b0100) blockNames.push('main'); - if (flags & 0b0010) blockNames.push('else'); - if (flags & 0b0001) blockNames.push('attrs'); + let blockNames = flags & 0b0111 ? vm[CONSTANTS].getStringArray(_blockNames) : EMPTY_ARRAY; + + // if (flags & 0b0100) blockNames.push('main'); + // if (flags & 0b0010) blockNames.push('else'); + // if (flags & 0b0001) blockNames.push('attrs'); vm[ARGS].setup(stack, names, blockNames, positionalCount, !!atNames); stack.push(vm[ARGS]); @@ -755,9 +770,10 @@ APPEND_OPCODES.add(Op.SetBlocks, (vm, { op1: _state }) => { let state = check(vm.fetchValue(_state), CheckFinishedComponentInstance); let { blocks } = check(vm.stack.peek(), CheckArguments); - bindBlock('&attrs', 'attrs', state, blocks, vm); - bindBlock('&else', 'else', state, blocks, vm); - bindBlock('&default', 'main', state, blocks, vm); + for (let name of blocks.names) { + bindBlock(`&${name}`, name, state, blocks, vm); + } + }); // Dynamic Invocation Only From 13f887d01c987db2b656f7609ef0f1ade357c13b Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Tue, 7 Apr 2020 17:36:38 -0400 Subject: [PATCH 03/12] Fix failure with HasBlockParams --- .../@glimmer/debug/lib/opcode-metadata.ts | 8 ++--- packages/@glimmer/debug/lib/stack-check.ts | 17 ++++++++- .../lib/syntax/push-resolution.ts | 2 +- .../lib/compiled/opcodes/-debug-strip.ts | 23 ++++++++++++ .../runtime/lib/compiled/opcodes/component.ts | 3 +- .../lib/compiled/opcodes/expressions.ts | 36 ++++++++++++------- packages/@glimmer/vm/lib/opcodes.toml | 2 +- 7 files changed, 70 insertions(+), 21 deletions(-) diff --git a/packages/@glimmer/debug/lib/opcode-metadata.ts b/packages/@glimmer/debug/lib/opcode-metadata.ts index b1bf086008..53ef3a48f1 100644 --- a/packages/@glimmer/debug/lib/opcode-metadata.ts +++ b/packages/@glimmer/debug/lib/opcode-metadata.ts @@ -1041,12 +1041,12 @@ METADATA[Op.PushArgs] = { type: 'str-array', }, { - name: 'positionalCount', - type: 'u32', + name: 'block-names', + type: 'str-array', }, { - name: 'synthetic', - type: 'bool', + name: 'flags', + type: 'u32', }, ], operands: 3, diff --git a/packages/@glimmer/debug/lib/stack-check.ts b/packages/@glimmer/debug/lib/stack-check.ts index 9adf0c37e7..75e9777125 100644 --- a/packages/@glimmer/debug/lib/stack-check.ts +++ b/packages/@glimmer/debug/lib/stack-check.ts @@ -103,6 +103,21 @@ class OptionChecker implements Checker> { } } +class MaybeChecker implements Checker> { + type!: Maybe; + + constructor(private checker: Checker) {} + + validate(value: unknown): value is Maybe { + if (value === null || value === undefined) return true; + return this.checker.validate(value); + } + + expected(): string { + return `${this.checker.expected()} or null or undefined`; + } +} + class OrChecker implements Checker { type!: T | U; @@ -215,7 +230,7 @@ export function CheckOption(checker: Checker): Checker> { } export function CheckMaybe(checker: Checker): Checker> { - return new OptionChecker(checker, undefined); + return new MaybeChecker(checker); } export function CheckInterface< diff --git a/packages/@glimmer/opcode-compiler/lib/syntax/push-resolution.ts b/packages/@glimmer/opcode-compiler/lib/syntax/push-resolution.ts index c924b8fbd2..61248bc33a 100644 --- a/packages/@glimmer/opcode-compiler/lib/syntax/push-resolution.ts +++ b/packages/@glimmer/opcode-compiler/lib/syntax/push-resolution.ts @@ -141,7 +141,7 @@ export function compileSimpleArgs( } } - out.push(op(Op.PushArgs, strArray(names), flags)); + out.push(op(Op.PushArgs, strArray(names), strArray(EMPTY_ARRAY), flags)); return out; } diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts index fc83e6c0f2..c83b391558 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts @@ -10,6 +10,7 @@ import { CheckUnknown, wrap, CheckOption, + CheckOr, } from '@glimmer/debug'; import { CapturedArguments, @@ -23,6 +24,7 @@ import { Helper, CapturedArgumentsValue, Option, + JitScopeBlock, } from '@glimmer/interfaces'; import { VersionedPathReference, Reference } from '@glimmer/reference'; import { Tag, COMPUTE } from '@glimmer/validator'; @@ -34,6 +36,7 @@ import { VMArgumentsImpl, } from '../../vm/arguments'; import { ComponentInstance, ComponentElementOperations } from './component'; +import { UNDEFINED_REFERENCE } from '../../references'; export const CheckTag: Checker = CheckInterface({ [COMPUTE]: CheckFunction, @@ -72,6 +75,20 @@ class CheckCapturedArgumentsValue implements Checker<() => CapturedArgumentsValu } } +export class UndefinedReferenceChecker implements Checker { + type!: Reference; + + validate(value: unknown): value is Reference { + return value === UNDEFINED_REFERENCE; + } + + expected(): string { + return `UNDEFINED_REFERENCE`; + } +} + +export const CheckUndefinedReference = new UndefinedReferenceChecker(); + export const CheckCapturedArguments: Checker = CheckInterface({ tag: CheckTag, length: CheckNumber, @@ -120,3 +137,9 @@ export const CheckCompilableBlock: Checker = CheckInterface({ compile: CheckFunction, symbolTable: CheckBlockSymbolTable, }); + +export const CheckScopeBlock: Checker = CheckInterface({ + 0: CheckOr(CheckHandle, CheckCompilableBlock), + 1: CheckScope, + 2: CheckBlockSymbolTable, +}); diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index 24e3dbd218..a92098dee5 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -1,4 +1,4 @@ -¬import { +import { check, CheckFunction, CheckHandle, @@ -773,7 +773,6 @@ APPEND_OPCODES.add(Op.SetBlocks, (vm, { op1: _state }) => { for (let name of blocks.names) { bindBlock(`&${name}`, name, state, blocks, vm); } - }); // Dynamic Invocation Only diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts index f902926577..7a927b8197 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts @@ -1,11 +1,18 @@ import { Option, Op, JitScopeBlock, AotScopeBlock, VM as PublicVM } from '@glimmer/interfaces'; -import { VersionedPathReference } from '@glimmer/reference'; +import { VersionedPathReference, Reference } from '@glimmer/reference'; import { $v0 } from '@glimmer/vm'; import { APPEND_OPCODES } from '../../opcodes'; import { FALSE_REFERENCE, TRUE_REFERENCE, UNDEFINED_REFERENCE } from '../../references'; import { ConcatReference } from '../expressions/concat'; import { assert } from '@glimmer/util'; -import { check, CheckOption, CheckHandle, CheckBlockSymbolTable, CheckOr } from '@glimmer/debug'; +import { + check, + CheckOption, + CheckHandle, + CheckBlockSymbolTable, + CheckOr, + CheckMaybe, +} from '@glimmer/debug'; import { stackAssert } from './assert'; import { CheckArguments, @@ -13,6 +20,8 @@ import { CheckCompilableBlock, CheckScope, CheckHelper, + CheckUndefinedReference, + CheckScopeBlock, } from './-debug-strip'; import { CONSTANTS } from '../../symbols'; @@ -92,9 +101,9 @@ APPEND_OPCODES.add(Op.GetBlock, (vm, { op1: _block }) => { APPEND_OPCODES.add(Op.JitSpreadBlock, vm => { let { stack } = vm; - let block = stack.pop(); + let block = check(stack.pop(), CheckOption(CheckOr(CheckScopeBlock, CheckUndefinedReference))); - if (block) { + if (block && !isUndefinedReference(block)) { stack.push(block[2]); stack.push(block[1]); stack.push(block[0]); @@ -105,6 +114,14 @@ APPEND_OPCODES.add(Op.JitSpreadBlock, vm => { } }); +function isUndefinedReference(input: JitScopeBlock | Reference): input is Reference { + assert( + Array.isArray(input) || input === UNDEFINED_REFERENCE, + 'a reference other than UNDEFINED_REFERENCE is illegal here' + ); + return input === UNDEFINED_REFERENCE; +} + APPEND_OPCODES.add(Op.HasBlock, vm => { let block = vm.stack.pop(); @@ -123,14 +140,9 @@ APPEND_OPCODES.add(Op.HasBlockParams, vm => { // FIXME(mmun): should only need to push the symbol table let block = vm.stack.pop(); let scope = vm.stack.pop(); - check(block, CheckOption(CheckOr(CheckHandle, CheckCompilableBlock))); - check(scope, CheckOption(CheckScope)); - let table = check(vm.stack.pop(), CheckOption(CheckBlockSymbolTable)); - - assert( - table === null || (table && typeof table === 'object' && Array.isArray(table.parameters)), - stackAssert('Option', table) - ); + check(block, CheckMaybe(CheckOr(CheckHandle, CheckCompilableBlock))); + check(scope, CheckMaybe(CheckScope)); + let table = check(vm.stack.pop(), CheckMaybe(CheckBlockSymbolTable)); let hasBlockParams = table && table.parameters.length; vm.stack.push(hasBlockParams ? TRUE_REFERENCE : FALSE_REFERENCE); diff --git a/packages/@glimmer/vm/lib/opcodes.toml b/packages/@glimmer/vm/lib/opcodes.toml index dcd3abd0da..037851ee99 100644 --- a/packages/@glimmer/vm/lib/opcodes.toml +++ b/packages/@glimmer/vm/lib/opcodes.toml @@ -708,7 +708,7 @@ operand-stack = [ [syscall.argsload] -format = ["PushArgs", "names:str-array", "positionalCount:u32", "synthetic:bool"] +format = ["PushArgs", "names:str-array", "block-names:str-array", "flags:u32"] operation = "Push a user representation of args onto the stack." operand-stack = [ ["Reference..."], From afc5a56a6f2007f7ba7cfcf8a5fbbb9df6bf818d Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Wed, 8 Apr 2020 00:25:00 +0100 Subject: [PATCH 04/12] remove commented code --- .../lib/opcode-builder/helpers/shared.ts | 8 -------- .../@glimmer/runtime/lib/compiled/opcodes/component.ts | 10 ---------- 2 files changed, 18 deletions(-) diff --git a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts index 8adb2a0e15..e0e6baef61 100644 --- a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts +++ b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts @@ -35,20 +35,12 @@ export function CompileArgs({ out.push(PushYieldableBlock(blocks.get(blockNames[i]))); } - //if (blocks.hasAny) { - // out.push(PushYieldableBlock(blocks.get('default'))); - // out.push(PushYieldableBlock(blocks.get('else'))); - // out.push(PushYieldableBlock(blocks.get('attrs'))); - //} - let { count, actions } = CompilePositional(params); out.push(actions); let flags = count << 4; - // 10110101010 [X][X][X][X] - if (atNames) flags |= 0b1000; if (blocks) { diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index a92098dee5..78c49864ea 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -239,20 +239,10 @@ APPEND_OPCODES.add(Op.PushArgs, (vm, { op1: _names, op2: _blockNames, op3: flags let stack = vm.stack; let names = vm[CONSTANTS].getStringArray(_names); - // 10110101010 [X][X][X][X] let positionalCount = flags >> 4; - // 10110101010 [X][X][X][X] let atNames = flags & 0b1000; - // let blockNames: string[] = []; - - // there are blocks - let blockNames = flags & 0b0111 ? vm[CONSTANTS].getStringArray(_blockNames) : EMPTY_ARRAY; - // if (flags & 0b0100) blockNames.push('main'); - // if (flags & 0b0010) blockNames.push('else'); - // if (flags & 0b0001) blockNames.push('attrs'); - vm[ARGS].setup(stack, names, blockNames, positionalCount, !!atNames); stack.push(vm[ARGS]); }); From c85c03b59e0c471d91a15147fa87ff0201d809e7 Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Wed, 8 Apr 2020 01:14:23 +0100 Subject: [PATCH 05/12] make it visually similar to SetNamedVariables --- packages/@glimmer/runtime/lib/compiled/opcodes/component.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index 78c49864ea..eafb6aa879 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -746,13 +746,9 @@ function bindBlock( vm: InternalVM ) { let symbol = state.table.symbols.indexOf(symbolName); - let block = blocks.get(blockName); - if (symbol !== -1) { - vm.scope().bindBlock(symbol + 1, block); - } - + if (symbol !== -1) vm.scope().bindBlock(symbol + 1, block); if (state.lookup) state.lookup[symbolName] = block; } From 3ba3de6ca684e1240124621b0d62ce121dce8617 Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Wed, 8 Apr 2020 01:14:57 +0100 Subject: [PATCH 06/12] derive names at initialization time --- packages/@glimmer/opcode-compiler/lib/utils.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/@glimmer/opcode-compiler/lib/utils.ts b/packages/@glimmer/opcode-compiler/lib/utils.ts index b653ab93a9..21f6bd8267 100644 --- a/packages/@glimmer/opcode-compiler/lib/utils.ts +++ b/packages/@glimmer/opcode-compiler/lib/utils.ts @@ -15,7 +15,11 @@ interface NamedBlocksDict { } export class NamedBlocksImpl implements NamedBlocks { - constructor(private blocks: Option) {} + public names: string[]; + + constructor(private blocks: Option) { + this.names = blocks ? Object.keys(blocks) : []; + } get(name: string): Option { if (!this.blocks) return null; @@ -38,14 +42,6 @@ export class NamedBlocksImpl implements NamedBlocks { } } - get names() { - if (this.blocks != null) { - return Object.keys(this.blocks); - } else { - return null; - } - } - get hasAny(): boolean { return this.blocks !== null; } From 88d151e61126aa3bddb65bbf4493daaa108f4487 Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Wed, 8 Apr 2020 01:21:38 +0100 Subject: [PATCH 07/12] add names property to NamedBlocks interface --- packages/@glimmer/interfaces/lib/template.d.ts | 1 + .../opcode-compiler/lib/opcode-builder/helpers/shared.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/@glimmer/interfaces/lib/template.d.ts b/packages/@glimmer/interfaces/lib/template.d.ts index 67d25e9321..3efdfc3e85 100644 --- a/packages/@glimmer/interfaces/lib/template.d.ts +++ b/packages/@glimmer/interfaces/lib/template.d.ts @@ -94,6 +94,7 @@ export interface NamedBlocks { has(name: string): boolean; with(name: string, block: Option): NamedBlocks; hasAny: boolean; + names: string[]; } export interface ContainingMetadata { diff --git a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts index e0e6baef61..0f4c389fec 100644 --- a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts +++ b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts @@ -30,7 +30,7 @@ export function CompileArgs({ }: ArgsOptions): StatementCompileActions { let out: StatementCompileActions = []; - let blockNames: string[] = ((blocks as any) as { names: string[] }).names; + let blockNames: string[] = blocks.names; for (let i = 0; i < blockNames.length; i++) { out.push(PushYieldableBlock(blocks.get(blockNames[i]))); } From e56ba609a4ca6318dac516e645357026fb9ad208 Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Thu, 9 Apr 2020 12:06:20 +0100 Subject: [PATCH 08/12] use checker in HasBlock --- .../runtime/lib/compiled/opcodes/expressions.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts index 7a927b8197..11596382f3 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts @@ -123,7 +123,15 @@ function isUndefinedReference(input: JitScopeBlock | Reference): input is Refere } APPEND_OPCODES.add(Op.HasBlock, vm => { - let block = vm.stack.pop(); + let stack = vm.stack.pop(); + + if (!Array.isArray(stack)) { + return vm.stack.push(FALSE_REFERENCE); + } + + let [_block, scope] = stack; + check(_block, CheckMaybe(CheckOr(CheckHandle, CheckCompilableBlock))); + check(scope, CheckMaybe(CheckScope)); // TODO: We check if the block is null or UNDEFINED_REFERENCE here, but it should // really only check if the block is null. The UNDEFINED_REFERENCE use case is for @@ -133,13 +141,15 @@ APPEND_OPCODES.add(Op.HasBlock, vm => { // // This code path does not work the same way as most components. In the future, // we should make sure that it does, so things are setup correctly. - vm.stack.push(block === null || block === UNDEFINED_REFERENCE ? FALSE_REFERENCE : TRUE_REFERENCE); + // let hasBlock = block !== null && block !== UNDEFINED_REFERENCE; + vm.stack.push(TRUE_REFERENCE); }); APPEND_OPCODES.add(Op.HasBlockParams, vm => { // FIXME(mmun): should only need to push the symbol table let block = vm.stack.pop(); let scope = vm.stack.pop(); + check(block, CheckMaybe(CheckOr(CheckHandle, CheckCompilableBlock))); check(scope, CheckMaybe(CheckScope)); let table = check(vm.stack.pop(), CheckMaybe(CheckBlockSymbolTable)); From 234ec45015ac0841fd96c6b3bc573df4ac4e55af Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 10 Apr 2020 11:10:49 -0400 Subject: [PATCH 09/12] Avoid string concat in Ops.SetBlocks Co-authored-by: Ricardo Mendes --- .../runtime/lib/compiled/opcodes/component.ts | 4 ++-- packages/@glimmer/runtime/lib/vm/arguments.ts | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index eafb6aa879..0d35bbb510 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -756,8 +756,8 @@ APPEND_OPCODES.add(Op.SetBlocks, (vm, { op1: _state }) => { let state = check(vm.fetchValue(_state), CheckFinishedComponentInstance); let { blocks } = check(vm.stack.peek(), CheckArguments); - for (let name of blocks.names) { - bindBlock(`&${name}`, name, state, blocks, vm); + for (let i = 0; i < blocks.names.length; i++) { + bindBlock(blocks.symbolNames[i], blocks.names[i], state, blocks, vm); } }); diff --git a/packages/@glimmer/runtime/lib/vm/arguments.ts b/packages/@glimmer/runtime/lib/vm/arguments.ts index a64938bcc1..976646e294 100644 --- a/packages/@glimmer/runtime/lib/vm/arguments.ts +++ b/packages/@glimmer/runtime/lib/vm/arguments.ts @@ -462,9 +462,14 @@ export class CapturedNamedArgumentsImpl implements CapturedNamedArguments { } } +function toSymbolName(name: string): string { + return `&${name}`; +} + export class BlockArgumentsImpl implements BlockArguments { private stack!: EvaluationStack; private internalValues: Option = null; + private _symbolNames: Option = null; public internalTag: Option = null; public names: string[] = EMPTY_ARRAY; @@ -477,6 +482,7 @@ export class BlockArgumentsImpl implements BlockArgumen this.names = EMPTY_ARRAY; this.base = base; this.length = 0; + this._symbolNames = null; this.internalTag = CONSTANT_TAG; this.internalValues = EMPTY_ARRAY; @@ -487,6 +493,7 @@ export class BlockArgumentsImpl implements BlockArgumen this.names = names; this.base = base; this.length = length; + this._symbolNames = null; if (length === 0) { this.internalTag = CONSTANT_TAG; @@ -534,6 +541,16 @@ export class BlockArgumentsImpl implements BlockArgumen capture(): CapturedBlockArguments { return new CapturedBlockArgumentsImpl(this.names, this.values); } + + get symbolNames(): string[] { + let symbolNames = this._symbolNames; + + if (symbolNames === null) { + symbolNames = this._symbolNames = this.names.map(toSymbolName); + } + + return symbolNames; + } } class CapturedBlockArgumentsImpl implements CapturedBlockArguments { From 06f67b6e79ec78bb18d443b2dfd6366b63f6d79b Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 10 Apr 2020 11:13:04 -0400 Subject: [PATCH 10/12] Cleanup Op.HasBlock to leverage CheckUndefinedReference Co-authored-by: Ricardo Mendes --- .../lib/compiled/opcodes/expressions.ts | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts index 11596382f3..1bc737b06e 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts @@ -123,26 +123,14 @@ function isUndefinedReference(input: JitScopeBlock | Reference): input is Refere } APPEND_OPCODES.add(Op.HasBlock, vm => { - let stack = vm.stack.pop(); + let { stack } = vm; + let block = check(stack.pop(), CheckOption(CheckOr(CheckScopeBlock, CheckUndefinedReference))); - if (!Array.isArray(stack)) { - return vm.stack.push(FALSE_REFERENCE); + if (block && !isUndefinedReference(block)) { + stack.push(TRUE_REFERENCE); + } else { + stack.push(FALSE_REFERENCE); } - - let [_block, scope] = stack; - check(_block, CheckMaybe(CheckOr(CheckHandle, CheckCompilableBlock))); - check(scope, CheckMaybe(CheckScope)); - - // TODO: We check if the block is null or UNDEFINED_REFERENCE here, but it should - // really only check if the block is null. The UNDEFINED_REFERENCE use case is for - // when we try to invoke a curry-component directly as a variable: - // - // {{bar}} - // - // This code path does not work the same way as most components. In the future, - // we should make sure that it does, so things are setup correctly. - // let hasBlock = block !== null && block !== UNDEFINED_REFERENCE; - vm.stack.push(TRUE_REFERENCE); }); APPEND_OPCODES.add(Op.HasBlockParams, vm => { From aa8faec07de814bf46102e35d336beb2e2bd67ae Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 10 Apr 2020 11:18:13 -0400 Subject: [PATCH 11/12] Fixup linting error. Co-authored-by: Ricardo Mendes --- packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts index 1bc737b06e..850732142b 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts @@ -13,7 +13,6 @@ import { CheckOr, CheckMaybe, } from '@glimmer/debug'; -import { stackAssert } from './assert'; import { CheckArguments, CheckPathReference, From 4e70d1f2c376d4f2246c84c19bc88a6153bb8aad Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 10 Apr 2020 11:37:30 -0400 Subject: [PATCH 12/12] Add more tests. Co-authored-by: Ricardo Mendes --- .../lib/suites/components.ts | 21 +++++++++++++++++++ .../lib/suites/has-block-params.ts | 15 +++++++++++++ 2 files changed, 36 insertions(+) diff --git a/packages/@glimmer/integration-tests/lib/suites/components.ts b/packages/@glimmer/integration-tests/lib/suites/components.ts index 4fc288e38d..4792f575f4 100644 --- a/packages/@glimmer/integration-tests/lib/suites/components.ts +++ b/packages/@glimmer/integration-tests/lib/suites/components.ts @@ -555,6 +555,27 @@ export class BasicComponents extends RenderTest { this.assertStableRerender(); } + @test({ + kind: 'glimmer', + }) + 'invoking dynamic component (path) via angle brackets with named block'() { + class TestHarness extends EmberishGlimmerComponent { + public Foo: any; + } + this.registerComponent( + 'Glimmer', + 'TestHarness', + '<:bar>Stuff!', + TestHarness + ); + this.registerComponent('Glimmer', 'Foo', '{{yield to="bar"}}'); + + this.render(''); + + this.assertHTML(`Stuff!`); + this.assertStableRerender(); + } + @test({ kind: 'glimmer', }) diff --git a/packages/@glimmer/integration-tests/lib/suites/has-block-params.ts b/packages/@glimmer/integration-tests/lib/suites/has-block-params.ts index 309bc93d53..73de803c96 100644 --- a/packages/@glimmer/integration-tests/lib/suites/has-block-params.ts +++ b/packages/@glimmer/integration-tests/lib/suites/has-block-params.ts @@ -1,5 +1,6 @@ import { RenderTest } from '../render-test'; import { test } from '../test-decorator'; +import { EmberishGlimmerComponent } from '../components'; export class HasBlockParamsHelperSuite extends RenderTest { static suiteName = 'has-block-params'; @@ -16,6 +17,20 @@ export class HasBlockParamsHelperSuite extends RenderTest { this.assertStableRerender(); } + @test({ kind: 'curly' }) + 'has-block-params from within a yielded + invoked curried component'() { + class TestHarness extends EmberishGlimmerComponent { + public Foo: any; + } + this.registerComponent('Glimmer', 'TestHarness', '{{yield (component "Foo")}}', TestHarness); + this.registerComponent('Glimmer', 'Foo', '{{#if (has-block-params)}}Yes{{else}}No{{/if}}'); + + this.render('{{Foo}}'); + + this.assertHTML('No'); + this.assertStableRerender(); + } + @test({ kind: 'curly' }) 'parameterized has-block-params (subexpr, else) when else not supplied'() { this.render({