Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/@glimmer-workspace/integration-tests/lib/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ export function createTemplate(
): TemplateFactory {
options.locals = options.locals ?? Object.keys(scopeValues ?? {});
let [block, usedLocals] = precompileJSON(templateSource, options);
let reifiedScopeValues = usedLocals.map((key) => scopeValues[key]);
let reifiedScope: Record<string, unknown> = {};
for (let key of usedLocals) {
reifiedScope[key] = scopeValues[key];
}

if ('emit' in options && options.emit?.debugSymbols) {
block.push(usedLocals);
Expand All @@ -34,7 +37,7 @@ export function createTemplate(
id: String(templateId++),
block: JSON.stringify(block),
moduleName: options.meta?.moduleName ?? '(unknown template module)',
scope: reifiedScopeValues.length > 0 ? () => reifiedScopeValues : null,
scope: usedLocals.length > 0 ? () => reifiedScope : null,
isStrictMode: options.strictMode ?? false,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ module('[glimmer-compiler] precompile', ({ test }) => {
...WireFormat.Statement[],
];

assert.deepEqual(wire.scope?.(), [hello]);
assert.deepEqual(wire.scope?.(), { hello });

assert.deepEqual(
componentNameExpr,
Expand All @@ -84,7 +84,7 @@ module('[glimmer-compiler] precompile', ({ test }) => {
...WireFormat.Statement[],
];

assert.deepEqual(wire.scope?.(), [f]);
assert.deepEqual(wire.scope?.(), { f });
assert.deepEqual(
componentNameExpr,
[SexpOpcodes.GetLexicalSymbol, 0, ['hello']],
Expand Down Expand Up @@ -218,7 +218,7 @@ module('[glimmer-compiler] precompile', ({ test }) => {
_wire = compile(`{{this.message}}`, ['this'], (source) => eval(source));
}).call(target);
let wire = _wire!;
assert.deepEqual(wire.scope?.(), [target]);
assert.deepEqual(wire.scope?.(), { this: target });
assert.deepEqual(wire.block[0], [
[SexpOpcodes.Append, [SexpOpcodes.GetLexicalSymbol, 0, ['message']]],
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
import type { TemplateOnlyComponent } from '@glimmer/runtime';
import type { EmberishCurlyComponent } from '@glimmer-workspace/integration-tests';
import { expect } from '@glimmer/debug-util';
import { DEBUG } from '@glimmer/env';
import { modifierCapabilities, setComponentTemplate, setModifierManager } from '@glimmer/manager';
import { EMPTY_ARGS, templateOnlyComponent, TemplateOnlyComponentManager } from '@glimmer/runtime';
import { assign } from '@glimmer/util';
Expand Down Expand Up @@ -110,6 +111,88 @@ class DebugRenderTreeTest extends RenderTest {
]);
}

@test 'strict-mode components without debug symbols preserve names from scope'() {
const HelloWorld = defComponent('{{@arg}}');
const Root = defComponent(`<HelloWorld @arg="first"/>`, {
scope: { HelloWorld },
emit: { moduleName: 'root.hbs', debugSymbols: false },
});

this.renderComponent(Root);

this.assertRenderTree([
{
type: 'component',
name: '{ROOT}',
args: { positional: [], named: {} },
instance: null,
template: 'root.hbs',
bounds: this.elementBounds(this.delegate.getInitialElement()),
children: [
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
children: [],
},
],
},
]);
}

@test({ skip: !DEBUG }) 'dynamic component via <this.dynamicComponent>'() {
const HelloWorld = defComponent('{{@arg}}');

class Root extends GlimmerishComponent {
HelloWorld = HelloWorld;
}

const RootDef = defComponent(`<this.HelloWorld @arg="first"/>`, {
component: Root,
emit: { moduleName: 'root.hbs' },
});

this.renderComponent(RootDef);

const rootChildren = this.delegate.getCapturedRenderTree()[0]?.children ?? [];
const componentNode = rootChildren.find(
(n: CapturedRenderNode) => n.type === 'component' && n.name !== '{ROOT}'
);

this.assert.ok(componentNode, 'found a component child node');

this.assert.strictEqual(
componentNode?.name,
'this.HelloWorld',
`dynamic <this.X> component name (got "${componentNode?.name}")`
);
}

@test({ skip: !DEBUG }) 'dynamic component via <@argComponent>'() {
const HelloWorld = defComponent('{{@arg}}');
const Root = defComponent(`<@Greeting @arg="first"/>`, {
emit: { moduleName: 'root.hbs' },
});

this.renderComponent(Root, { Greeting: HelloWorld });

const rootChildren = this.delegate.getCapturedRenderTree()[0]?.children ?? [];
const componentNode = rootChildren.find(
(n: CapturedRenderNode) => n.type === 'component' && n.name !== '{ROOT}'
);

this.assert.ok(componentNode, 'found a component child node');

this.assert.strictEqual(
componentNode?.name,
'@Greeting',
`dynamic <@X> component name (got "${componentNode?.name}")`
);
}

@test 'strict-mode modifiers'() {
const state = trackedObj({ showSecond: false });

Expand Down
9 changes: 8 additions & 1 deletion packages/@glimmer/compiler/lib/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,14 @@ export function precompile(
let stringified = JSON.stringify(templateJSONObject);

if (usedLocals.length > 0) {
const scopeFn = `()=>[${usedLocals.join(',')}]`;
const scopeEntries = usedLocals.map((name) => {
// Reserved words like "this" can't use shorthand property syntax
if (name === 'this') {
return `"this":this`;
}
return name;
});
const scopeFn = `()=>({${scopeEntries.join(',')}})`;

stringified = stringified.replace(`"${SCOPE_PLACEHOLDER}"`, scopeFn);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ export interface SerializedTemplateWithLazyBlock {
id?: Nullable<string>;
block: SerializedTemplateBlockJSON;
moduleName: string;
scope?: (() => unknown[]) | undefined | null;
scope?: (() => Record<string, unknown>) | undefined | null;
isStrictMode: boolean;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/@glimmer/interfaces/lib/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export interface LayoutWithContext {
readonly block: SerializedTemplateBlock;
readonly moduleName: string;
readonly owner: Owner | null;
readonly scope: (() => unknown[]) | undefined | null;
readonly scope: (() => Record<string, unknown>) | undefined | null;
readonly isStrictMode: boolean;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,15 @@ export function CompilePositional(

export function meta(layout: LayoutWithContext): BlockMetadata {
let [, locals, upvars, lexicalSymbols] = layout.block;
let scopeRecord = layout.scope?.() ?? null;

return {
symbols: {
locals,
upvars,
lexical: lexicalSymbols,
lexical: scopeRecord ? Object.keys(scopeRecord) : lexicalSymbols,
},
scopeValues: layout.scope?.() ?? null,
scopeValues: scopeRecord ? Object.values(scopeRecord) : null,
isStrictMode: layout.isStrictMode,
moduleName: layout.moduleName,
owner: layout.owner,
Expand Down
26 changes: 22 additions & 4 deletions packages/@glimmer/runtime/lib/compiled/opcodes/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,8 @@ APPEND_OPCODES.add(VM_PUSH_COMPONENT_DEFINITION_OP, (vm, { op1: handle }) => {

APPEND_OPCODES.add(VM_RESOLVE_DYNAMIC_COMPONENT_OP, (vm, { op1: _isStrict }) => {
let stack = vm.stack;
let component = check(
valueForRef(check(stack.pop(), CheckReference)),
CheckOr(CheckString, CheckCurriedComponentDefinition)
);
let ref = check(stack.pop(), CheckReference);
let component = check(valueForRef(ref), CheckOr(CheckString, CheckCurriedComponentDefinition));
let constants = vm.constants;
let owner = vm.getOwner();
let isStrict = constants.getValue<boolean>(_isStrict);
Expand All @@ -182,6 +180,13 @@ APPEND_OPCODES.add(VM_RESOLVE_DYNAMIC_COMPONENT_OP, (vm, { op1: _isStrict }) =>
definition = constants.component(component, owner);
}

if (DEBUG && !isCurriedValue(definition) && !definition.resolvedName && !definition.debugName) {
let debugLabel = ref.debugLabel;
if (debugLabel) {
definition.debugName = debugLabel;
}
}

stack.push(definition);
});

Expand Down Expand Up @@ -217,6 +222,19 @@ APPEND_OPCODES.add(VM_RESOLVE_CURRIED_COMPONENT_OP, (vm) => {
}
}

if (
DEBUG &&
definition &&
!isCurriedValue(definition) &&
!definition.resolvedName &&
!definition.debugName
) {
let debugLabel = ref.debugLabel;
if (debugLabel) {
definition.debugName = debugLabel;
}
}

stack.push(definition);
});

Expand Down
7 changes: 5 additions & 2 deletions packages/internal-test-helpers/lib/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ export default function compile(
): TemplateFactory {
options.locals = options.locals ?? Object.keys(scopeValues ?? {});
let [block, usedLocals] = precompileJSON(templateSource, compileOptions(options));
let reifiedScopeValues = usedLocals.map((key) => scopeValues[key]);
let reifiedScope: Record<string, unknown> = {};
for (let key of usedLocals) {
reifiedScope[key] = scopeValues[key];
}

let templateBlock: SerializedTemplateWithLazyBlock = {
block: JSON.stringify(block),
moduleName: options.moduleName ?? options.meta?.moduleName ?? '(unknown template module)',
scope: reifiedScopeValues.length > 0 ? () => reifiedScopeValues : null,
scope: usedLocals.length > 0 ? () => reifiedScope : null,
isStrictMode: options.strictMode ?? false,
};

Expand Down
35 changes: 35 additions & 0 deletions smoke-tests/scenarios/basic-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,41 @@ function basicTest(scenarios: Scenarios, appName: string) {

});
`,
'debug-render-tree-test.gjs': `
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import { captureRenderTree } from '@ember/debug';
import Component from '@glimmer/component';

function flattenTree(nodes) {
let result = [];
for (let node of nodes) {
result.push(node);
if (node.children) {
result.push(...flattenTree(node.children));
}
}
return result;
}

class HelloWorld extends Component {
<template>{{@arg}}</template>
}

module('Integration | captureRenderTree', function (hooks) {
setupRenderingTest(hooks);

test('scope-based components have correct names in debugRenderTree', async function (assert) {
await render(<template><HelloWorld @arg="first" /></template>);

let tree = captureRenderTree(this.owner);
let allNodes = flattenTree(tree);
let names = allNodes.filter(n => n.type === 'component').map(n => n.name);
assert.true(names.includes('HelloWorld'), 'HelloWorld component name is preserved in the render tree (found: ' + names.join(', ') + ')');
});
});
`,
},
},
});
Expand Down
Loading