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 nested calls to helpers in dynamic helpers #1293

Merged
merged 1 commit into from Apr 15, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,4 +1,11 @@
import { RenderTest, test, jitSuite, GlimmerishComponent, defineSimpleHelper } from '../..';
import {
RenderTest,
test,
jitSuite,
GlimmerishComponent,
defineSimpleHelper,
defineComponent,
} from '../..';

class DynamicHelpersResolutionModeTest extends RenderTest {
static suiteName = 'dynamic helpers in resolution mode';
Expand Down Expand Up @@ -35,6 +42,41 @@ class DynamicHelpersResolutionModeTest extends RenderTest {
/The `x\.foo` property path was used in a template for the `.*` component without using `this`/
);
}

@test
'Can use a dynamic helper with nested helpers'() {
const foo = defineSimpleHelper(() => 'world!');
const bar = defineSimpleHelper((value: string) => 'Hello, ' + value);
const Bar = defineComponent(
{ foo },
'{{this.bar (foo)}}',
class extends GlimmerishComponent {
bar = bar;
}
);

this.renderComponent(Bar);
this.assertHTML('Hello, world!');
this.assertStableRerender();
}

@test
'Can use a dynamic helper with nested dynamic helpers'() {
const foo = defineSimpleHelper(() => 'world!');
const bar = defineSimpleHelper((value: string) => 'Hello, ' + value);
const Bar = defineComponent(
{},
'{{this.bar (this.foo)}}',
class extends GlimmerishComponent {
foo = foo;
bar = bar;
}
);

this.renderComponent(Bar);
this.assertHTML('Hello, world!');
this.assertStableRerender();
}
}

jitSuite(DynamicHelpersResolutionModeTest);
Expand Up @@ -5,6 +5,8 @@ import {
defineSimpleModifier,
syntaxErrorFor,
GlimmerishComponent,
defineSimpleHelper,
defineComponent,
} from '../..';

class DynamicModifiersResolutionModeTest extends RenderTest {
Expand Down Expand Up @@ -131,6 +133,45 @@ class DynamicModifiersResolutionModeTest extends RenderTest {
this.registerComponent('TemplateOnly', 'Bar', '<div {{x.foo}}></div>');
}, syntaxErrorFor('You attempted to invoke a path (`{{#x.foo}}`) as a modifier, but x was not in scope. Try adding `this` to the beginning of the path', '{{x.foo}}', 'an unknown module', 1, 5));
}

@test
'Can use a dynamic modifier with a nested helper'() {
const foo = defineSimpleHelper(() => 'Hello, world!');
const bar = defineSimpleModifier(
(element: Element, value: string) => (element.innerHTML = value)
);
const Bar = defineComponent(
{ foo },
'<div {{this.bar (foo)}}></div>',
class extends GlimmerishComponent {
bar = bar;
}
);

this.renderComponent(Bar);
this.assertHTML('<div>Hello, world!</div>');
this.assertStableRerender();
}

@test
'Can use a dynamic modifier with a nested dynamic helper'() {
const foo = defineSimpleHelper(() => 'Hello, world!');
const bar = defineSimpleModifier(
(element: Element, value: string) => (element.innerHTML = value)
);
const Bar = defineComponent(
{},
'<div {{this.bar (this.foo)}}></div>',
class extends GlimmerishComponent {
foo = foo;
bar = bar;
}
);

this.renderComponent(Bar);
this.assertHTML('<div>Hello, world!</div>');
this.assertStableRerender();
}
}

jitSuite(DynamicModifiersResolutionModeTest);
@@ -1,4 +1,4 @@
import { $v0 } from '@glimmer/vm';
import { $fp, $v0 } from '@glimmer/vm';
import {
Option,
Op,
Expand Down Expand Up @@ -79,13 +79,13 @@ export function CallDynamic(
named: WireFormat.Core.Hash,
append?: () => void
): void {
op(Op.Load, $v0);
op(MachineOp.PushFrame);
SimpleArgs(op, positional, named, false);
op(Op.DynamicHelper, $v0);
op(Op.Dup, $fp, 1);
pzuraq marked this conversation as resolved.
Show resolved Hide resolved
op(Op.DynamicHelper);
if (append) {
op(Op.Fetch, $v0);
append?.();
append();
op(MachineOp.PopFrame);
} else {
op(MachineOp.PopFrame);
Expand Down
4 changes: 2 additions & 2 deletions packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts
Expand Up @@ -73,10 +73,10 @@ APPEND_OPCODES.add(Op.Curry, (vm, { op1: type, op2: _isStrict }) => {
);
});

APPEND_OPCODES.add(Op.DynamicHelper, (vm, { op1: _definitionRegister }) => {
APPEND_OPCODES.add(Op.DynamicHelper, (vm) => {
let stack = vm.stack;
let ref = check(stack.popJs(), CheckReference);
let args = check(stack.popJs(), CheckArguments).capture();
let ref = vm.fetchValue<Reference>(_definitionRegister);

let helperRef: Reference;
let initialOwner: Owner = vm.getOwner();
Expand Down