diff --git a/packages/@glimmer-workspace/integration-tests/lib/compile.ts b/packages/@glimmer-workspace/integration-tests/lib/compile.ts index 012278e8765..ace69285e2c 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/compile.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/compile.ts @@ -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 = {}; + for (let key of usedLocals) { + reifiedScope[key] = scopeValues[key]; + } if ('emit' in options && options.emit?.debugSymbols) { block.push(usedLocals); @@ -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, }; diff --git a/packages/@glimmer-workspace/integration-tests/test/compiler/compile-options-test.ts b/packages/@glimmer-workspace/integration-tests/test/compiler/compile-options-test.ts index c166e06c1d0..7d2c5d2505e 100644 --- a/packages/@glimmer-workspace/integration-tests/test/compiler/compile-options-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/compiler/compile-options-test.ts @@ -57,7 +57,7 @@ module('[glimmer-compiler] precompile', ({ test }) => { ...WireFormat.Statement[], ]; - assert.deepEqual(wire.scope?.(), [hello]); + assert.deepEqual(wire.scope?.(), { hello }); assert.deepEqual( componentNameExpr, @@ -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']], @@ -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']]], ]); diff --git a/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts b/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts index 5cecd7d0852..81a594e7d49 100644 --- a/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts @@ -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'; @@ -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(``, { + 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 '() { + const HelloWorld = defComponent('{{@arg}}'); + + class Root extends GlimmerishComponent { + HelloWorld = HelloWorld; + } + + const RootDef = defComponent(``, { + 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 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 }); diff --git a/packages/@glimmer/compiler/lib/compiler.ts b/packages/@glimmer/compiler/lib/compiler.ts index 089608f65f5..a712196610a 100644 --- a/packages/@glimmer/compiler/lib/compiler.ts +++ b/packages/@glimmer/compiler/lib/compiler.ts @@ -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); } diff --git a/packages/@glimmer/interfaces/lib/compile/wire-format/api.d.ts b/packages/@glimmer/interfaces/lib/compile/wire-format/api.d.ts index 6db2138eb87..9032916b8d1 100644 --- a/packages/@glimmer/interfaces/lib/compile/wire-format/api.d.ts +++ b/packages/@glimmer/interfaces/lib/compile/wire-format/api.d.ts @@ -397,7 +397,7 @@ export interface SerializedTemplateWithLazyBlock { id?: Nullable; block: SerializedTemplateBlockJSON; moduleName: string; - scope?: (() => unknown[]) | undefined | null; + scope?: (() => Record) | undefined | null; isStrictMode: boolean; } diff --git a/packages/@glimmer/interfaces/lib/template.d.ts b/packages/@glimmer/interfaces/lib/template.d.ts index 5e008c8039c..2990176844a 100644 --- a/packages/@glimmer/interfaces/lib/template.d.ts +++ b/packages/@glimmer/interfaces/lib/template.d.ts @@ -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) | undefined | null; readonly isStrictMode: boolean; } 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 640b63494b7..634fd603510 100644 --- a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts +++ b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts @@ -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, diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index a8091130ec7..55baed8453a 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -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(_isStrict); @@ -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); }); @@ -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); }); diff --git a/packages/internal-test-helpers/lib/compile.ts b/packages/internal-test-helpers/lib/compile.ts index 7fd317a7fe8..2266b47457e 100644 --- a/packages/internal-test-helpers/lib/compile.ts +++ b/packages/internal-test-helpers/lib/compile.ts @@ -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 = {}; + 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, }; diff --git a/smoke-tests/scenarios/basic-test.ts b/smoke-tests/scenarios/basic-test.ts index 1e575338e0e..b58d060f79a 100644 --- a/smoke-tests/scenarios/basic-test.ts +++ b/smoke-tests/scenarios/basic-test.ts @@ -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 { + + } + + module('Integration | captureRenderTree', function (hooks) { + setupRenderingTest(hooks); + + test('scope-based components have correct names in debugRenderTree', async function (assert) { + await render(); + + 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(', ') + ')'); + }); + }); + `, }, }, });