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

Fix issue when using named blocks with wrapped components (e.g. @ember/component) #1068

Merged
merged 12 commits into from
Apr 12, 2020
8 changes: 4 additions & 4 deletions packages/@glimmer/debug/lib/opcode-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 16 additions & 1 deletion packages/@glimmer/debug/lib/stack-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,21 @@ class OptionChecker<T> implements Checker<Option<T>> {
}
}

class MaybeChecker<T> implements Checker<Maybe<T>> {
type!: Maybe<T>;

constructor(private checker: Checker<T>) {}

validate(value: unknown): value is Maybe<T> {
if (value === null || value === undefined) return true;
return this.checker.validate(value);
}

expected(): string {
return `${this.checker.expected()} or null or undefined`;
}
}

class OrChecker<T, U> implements Checker<T | U> {
type!: T | U;

Expand Down Expand Up @@ -215,7 +230,7 @@ export function CheckOption<T>(checker: Checker<T>): Checker<Option<T>> {
}

export function CheckMaybe<T>(checker: Checker<T>): Checker<Maybe<T>> {
return new OptionChecker(checker, undefined);
return new MaybeChecker(checker);
}

export function CheckInterface<
Expand Down
21 changes: 21 additions & 0 deletions packages/@glimmer/integration-tests/lib/suites/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
'<this.Foo><:bar>Stuff!</:bar></this.Foo>',
locks marked this conversation as resolved.
Show resolved Hide resolved
TestHarness
);
this.registerComponent('Glimmer', 'Foo', '{{yield to="bar"}}');

this.render('<TestHarness @Foo={{component "Foo"}} />');

this.assertHTML(`Stuff!`);
this.assertStableRerender();
}

@test({
kind: 'glimmer',
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(`<FooBar><:baz> my </:baz></FooBar>`);

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(`<FooBar> my </FooBar>`);

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(`<FooBar as |baz|><:default> my </:default></FooBar>`);

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(`<FooBar><:else> my </:else></FooBar>`);

this.assertComponent('Hello my world!');
this.assertStableRerender();
}

@test({ kind: 'curly' })
'invoking wrapped layout via angle brackets - invocation attributes merges classes'() {
class FooBar extends EmberishCurlyComponent {
Expand Down
15 changes: 15 additions & 0 deletions packages/@glimmer/integration-tests/lib/suites/has-block-params.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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('<TestHarness as |Foo|>{{Foo}}</TestHarness>');

this.assertHTML('No');
this.assertStableRerender();
}

@test({ kind: 'curly' })
'parameterized has-block-params (subexpr, else) when else not supplied'() {
this.render({
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/interfaces/lib/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export interface NamedBlocks {
has(name: string): boolean;
with(name: string, block: Option<CompilableBlock>): NamedBlocks;
hasAny: boolean;
names: string[];
}

export interface ContainingMetadata {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ 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.names;
for (let i = 0; i < blockNames.length; i++) {
out.push(PushYieldableBlock(blocks.get(blockNames[i])));
}

let { count, actions } = CompilePositional(params);
Expand All @@ -58,7 +57,7 @@ export function CompileArgs({
}
}

out.push(op(Op.PushArgs, strArray(names), flags));
out.push(op(Op.PushArgs, strArray(names), strArray(blockNames), flags));

return out;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion packages/@glimmer/opcode-compiler/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ interface NamedBlocksDict {
}

export class NamedBlocksImpl implements NamedBlocks {
constructor(private blocks: Option<NamedBlocksDict>) {}
public names: string[];

constructor(private blocks: Option<NamedBlocksDict>) {
this.names = blocks ? Object.keys(blocks) : [];
}

get(name: string): Option<CompilableBlock> {
if (!this.blocks) return null;
Expand Down
23 changes: 23 additions & 0 deletions packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
CheckUnknown,
wrap,
CheckOption,
CheckOr,
} from '@glimmer/debug';
import {
CapturedArguments,
Expand All @@ -23,6 +24,7 @@ import {
Helper,
CapturedArgumentsValue,
Option,
JitScopeBlock,
} from '@glimmer/interfaces';
import { VersionedPathReference, Reference } from '@glimmer/reference';
import { Tag, COMPUTE } from '@glimmer/validator';
Expand All @@ -34,6 +36,7 @@ import {
VMArgumentsImpl,
} from '../../vm/arguments';
import { ComponentInstance, ComponentElementOperations } from './component';
import { UNDEFINED_REFERENCE } from '../../references';

export const CheckTag: Checker<Tag> = CheckInterface({
[COMPUTE]: CheckFunction,
Expand Down Expand Up @@ -72,6 +75,20 @@ class CheckCapturedArgumentsValue implements Checker<() => CapturedArgumentsValu
}
}

export class UndefinedReferenceChecker implements Checker<Reference> {
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<CapturedArguments> = CheckInterface({
tag: CheckTag,
length: CheckNumber,
Expand Down Expand Up @@ -120,3 +137,9 @@ export const CheckCompilableBlock: Checker<CompilableBlock> = CheckInterface({
compile: CheckFunction,
symbolTable: CheckBlockSymbolTable,
});

export const CheckScopeBlock: Checker<JitScopeBlock> = CheckInterface({
0: CheckOr(CheckHandle, CheckCompilableBlock),
1: CheckScope,
2: CheckBlockSymbolTable,
});
31 changes: 16 additions & 15 deletions packages/@glimmer/runtime/lib/compiled/opcodes/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -226,17 +235,13 @@ 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);

let positionalCount = flags >> 4;
let atNames = flags & 0b1000;
let blockNames: string[] = [];

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;

vm[ARGS].setup(stack, names, blockNames, positionalCount, !!atNames);
stack.push(vm[ARGS]);
Expand Down Expand Up @@ -741,23 +746,19 @@ function bindBlock<C extends JitOrAotBlock>(
vm: InternalVM<C>
) {
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;
}

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 i = 0; i < blocks.names.length; i++) {
bindBlock(blocks.symbolNames[i], blocks.names[i], state, blocks, vm);
}
});

// Dynamic Invocation Only
Expand Down
Loading