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

Plumb debug symbols when using lexical scope #1634

Merged
merged 5 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 6 additions & 2 deletions packages/@glimmer-workspace/integration-tests/lib/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {
Template,
TemplateFactory,
} from '@glimmer/interfaces';
import type { PrecompileOptions } from '@glimmer/syntax';
import type { PrecompileOptions, PrecompileOptionsWithLexicalScope } from '@glimmer/syntax';
import { precompileJSON } from '@glimmer/compiler';
import { templateFactory } from '@glimmer/opcode-compiler';

Expand All @@ -19,13 +19,17 @@ let templateId = 0;

export function createTemplate(
templateSource: Nullable<string>,
options: PrecompileOptions = {},
options: PrecompileOptions | PrecompileOptionsWithLexicalScope = {},
scopeValues: Record<string, unknown> = {}
): TemplateFactory {
options.locals = options.locals ?? Object.keys(scopeValues ?? {});
let [block, usedLocals] = precompileJSON(templateSource, options);
let reifiedScopeValues = usedLocals.map((key) => scopeValues[key]);

if ('emit' in options && options.emit?.debugSymbols) {
block.push(usedLocals);
}

let templateBlock: SerializedTemplateWithLazyBlock = {
id: String(templateId++),
block: JSON.stringify(block),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
ModifierManager,
Owner,
} from '@glimmer/interfaces';
import type { PrecompileOptionsWithLexicalScope } from '@glimmer/syntax';
import { registerDestructor } from '@glimmer/destroyable';
import {
helperCapabilities,
Expand Down Expand Up @@ -99,6 +100,8 @@ export interface DefineComponentOptions {

// additional strict-mode keywords
keywords?: string[];

emit?: PrecompileOptionsWithLexicalScope['emit'];
}

export function defineComponent(
Expand All @@ -116,7 +119,7 @@ export function defineComponent(
let keywords = options.keywords ?? [];

let definition = options.definition ?? templateOnlyComponent();
let templateFactory = createTemplate(templateSource, { strictMode, keywords }, scopeValues ?? {});
let templateFactory = createTemplate(templateSource, { strictMode, keywords, emit: options.emit }, scopeValues ?? {});
setComponentTemplate(templateFactory, definition);
return definition;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/@glimmer/compiler/lib/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ export function precompile(
): TemplateJavascript {
const [block, usedLocals] = precompileJSON(source, options);

if ('emit' in options && options.emit?.debugSymbols && usedLocals.length > 0) {
block.push(usedLocals);
}

const moduleName = options.meta?.moduleName;
const idFn = options.id || defaultId;
const blockJSON = JSON.stringify(block);
Expand Down
15 changes: 8 additions & 7 deletions packages/@glimmer/interfaces/lib/compile/wire-format/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ import type {
YieldOpcode,
} from './opcodes.js';

export * from './opcodes.js';
export * from './resolution.js';
export type * from './opcodes.js';
export type * from './resolution.js';

export type TupleSyntax = Statement | TupleExpression;

Expand All @@ -66,7 +66,7 @@ export type ExpressionSexpOpcodeMap = {
[TSexpOpcode in TupleExpression[0]]: Extract<TupleExpression, { 0: TSexpOpcode }>;
};

export interface SexpOpcodeMap extends ExpressionSexpOpcodeMap, StatementSexpOpcodeMap {}
export interface SexpOpcodeMap extends ExpressionSexpOpcodeMap, StatementSexpOpcodeMap { }
export type SexpOpcode = keyof SexpOpcodeMap;

export namespace Core {
Expand Down Expand Up @@ -365,13 +365,14 @@ export type SerializedInlineBlock = [statements: Statements.Statement[], paramet
*/
export type SerializedTemplateBlock = [
// statements
Statements.Statement[],
statements: Statements.Statement[],
// symbols
string[],
symbols: string[],
// hasDebug
boolean,
hasDebug: boolean,
// upvars
string[],
upvars: string[],
lexicalSymbols?: string[]
];

/**
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/interfaces/lib/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export interface ComponentDefinition<
manager: M;
capabilities: CapabilityMask;
compilable: CompilableProgram | null;
debugName?: string | undefined;
}

export interface ComponentInstance<
Expand Down
6 changes: 4 additions & 2 deletions packages/@glimmer/interfaces/lib/program.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,14 @@ export interface ResolutionTimeConstants {
component(
definitionState: ComponentDefinitionState,
owner: object,
isOptional?: false
isOptional?: false,
debugName?: string
): ComponentDefinition;
component(
definitionState: ComponentDefinitionState,
owner: object,
isOptional?: boolean
isOptional?: boolean,
debugName?: string
): ComponentDefinition | null;

resolvedComponent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,23 @@
assert(!meta.isStrictMode, 'Strict mode errors should already be handled at compile time');

throw new Error(
`Attempted to resolve a component in a strict mode template, but that value was not in scope: ${
meta.upvars![expr[1]] ?? '{unknown variable}'
`Attempted to resolve a component in a strict mode template, but that value was not in scope: ${meta.upvars![expr[1]] ?? '{unknown variable}'
}`
);
}

if (type === SexpOpcodes.GetLexicalSymbol) {
let { scopeValues, owner } = meta;
let { scopeValues, owner, debugSymbols } = meta;
let definition = expect(scopeValues, 'BUG: scopeValues must exist if template symbol is used')[

Check failure on line 97 in packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/resolution.ts

View workflow job for this annotation

GitHub Actions / Linting

Property 'debugSymbols' does not exist on type 'ContainingMetadata'.
expr[1]
];

then(
constants.component(
definition as object,
expect(owner, 'BUG: expected owner when resolving component definition')
expect(owner, 'BUG: expected owner when resolving component definition'),
false,
debugSymbols?.at(expr[1])
)
);
} else {
Expand Down Expand Up @@ -236,15 +237,16 @@
let type = expr[0];

if (type === SexpOpcodes.GetLexicalSymbol) {
let { scopeValues, owner } = meta;
let { scopeValues, owner, debugSymbols } = meta;
let definition = expect(scopeValues, 'BUG: scopeValues must exist if template symbol is used')[

Check failure on line 241 in packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/resolution.ts

View workflow job for this annotation

GitHub Actions / Linting

Property 'debugSymbols' does not exist on type 'ContainingMetadata'.
expr[1]
];

let component = constants.component(
definition as object,
expect(owner, 'BUG: expected owner when resolving component definition'),
true
true,
debugSymbols?.at(expr[1])
);

if (component !== null) {
Expand Down Expand Up @@ -316,8 +318,8 @@
let type = expr[0];

if (type === SexpOpcodes.GetLexicalSymbol) {
let { scopeValues, owner } = meta;
let { scopeValues, owner, debugSymbols } = meta;
let definition = expect(scopeValues, 'BUG: scopeValues must exist if template symbol is used')[

Check failure on line 322 in packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/resolution.ts

View workflow job for this annotation

GitHub Actions / Linting

Property 'debugSymbols' does not exist on type 'ContainingMetadata'.
expr[1]
];

Expand All @@ -333,7 +335,8 @@
let component = constants.component(
definition,
expect(owner, 'BUG: expected owner when resolving component definition'),
true
true,
debugSymbols?.at(expr[1])
);

if (component !== null) {
Expand Down Expand Up @@ -390,8 +393,7 @@
// Keyword helper did not exist, which means that we're attempting to use a
// value of some kind that is not in scope
throw new Error(
`Attempted to resolve a ${type} in a strict mode template, but that value was not in scope: ${
meta.upvars![expr[1]] ?? '{unknown variable}'
`Attempted to resolve a ${type} in a strict mode template, but that value was not in scope: ${meta.upvars![expr[1]] ?? '{unknown variable}'
}`
);
}
Expand Down
8 changes: 7 additions & 1 deletion packages/@glimmer/program/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ export class ConstantsImpl
component(
definitionState: ComponentDefinitionState,
owner: object,
isOptional?: true
isOptional?: true,
debugName?: string
): ComponentDefinition | null {
let definition = this.componentDefinitionCache.get(definitionState);

Expand Down Expand Up @@ -256,7 +257,12 @@ export class ConstantsImpl
this.componentDefinitionCount++;
}

if (definition && debugName !== undefined) {
return { ...definition, debugName };
} else {

return definition;
}
}

resolvedComponent(
Expand Down
5 changes: 3 additions & 2 deletions packages/@glimmer/runtime/lib/compiled/opcodes/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import { ConcreteBounds } from '../../bounds';
import { hasCustomDebugRenderTreeLifecycle } from '../../component/interfaces';
import { resolveComponent } from '../../component/resolve';
import { isCurriedType, isCurriedValue, resolveCurriedValue } from '../../curried-value';
import { getDebugName } from '../../debug-render-tree';
import { APPEND_OPCODES } from '../../opcodes';
import createClassListRef from '../../references/class-list';
import { ARGS, CONSTANTS } from '../../symbols';
Expand Down Expand Up @@ -417,7 +418,7 @@ APPEND_OPCODES.add(Op.BeginComponentTransaction, (vm, { op1: _state }) => {
if (import.meta.env.DEV) {
let { definition, manager } = check(vm.fetchValue(_state), CheckComponentInstance);

name = definition.resolvedName ?? manager.getDebugName(definition.state);
name = getDebugName(definition, manager);
}

vm.beginCacheGroup(name);
Expand Down Expand Up @@ -674,7 +675,7 @@ APPEND_OPCODES.add(Op.GetComponentSelf, (vm, { op1: _state, op2: _names }) => {
vm.updateWith(new DebugRenderTreeUpdateOpcode(bucket));
});
} else {
let name = definition.resolvedName ?? manager.getDebugName(definition.state);
let name = getDebugName(definition, manager);

vm.env.debugRenderTree.create(instance, {
type: 'component',
Expand Down
7 changes: 6 additions & 1 deletion packages/@glimmer/runtime/lib/debug-render-tree.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {
Bounds,
CapturedRenderNode,
ComponentDefinition,
DebugRenderTree,
Nullable,
RenderNode,
Expand Down Expand Up @@ -37,7 +38,7 @@ export class Ref<T extends object> {
this.value = null;
}

toString(): String {
toString(): string {
let label = `Ref ${this.id}`;

if (this.value === null) {
Expand Down Expand Up @@ -196,3 +197,7 @@ export default class DebugRenderTreeImpl<TBucket extends object>
return { parentElement, firstNode, lastNode };
}
}

export function getDebugName(definition: ComponentDefinition, manager = definition.manager): string {
return definition.resolvedName ?? definition.debugName ?? manager.getDebugName(definition.state);
}
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,14 @@ export interface PrecompileOptions extends PreprocessOptions {

export interface PrecompileOptionsWithLexicalScope extends PrecompileOptions {
lexicalScope: (variable: string) => boolean;

/**
* If `emit.debugSymbols` is set to `true`, the name of lexical local variables
* will be included in the wire format.
*/
emit?: {
debugSymbols?: boolean;
},
}

export interface PreprocessOptions {
Expand Down
Loading