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 3 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
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,33 @@ export interface DefineComponentOptions {

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

emit?: PrecompileOptionsWithLexicalScope['emit'];
}

export function defComponent(
templateSource: string,
options?: {
component?: object | undefined;
scope?: Record<string, unknown> | undefined;
emit?: {
moduleName?: string;
debugSymbols?: boolean;
};
}
) {
let definition = options?.component ?? templateOnlyComponent();
let templateFactory = createTemplate(
templateSource,
{
strictMode: true,
meta: { moduleName: options?.emit?.moduleName },
emit: { debugSymbols: options?.emit?.debugSymbols ?? true },
},
options?.scope ?? {}
);
setComponentTemplate(templateFactory, definition);
return definition;
}

export function defineComponent(
Expand All @@ -116,7 +144,11 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ import type { EmberishCurlyComponent } from '..';
import {
BaseEnv,
createTemplate,
defComponent,
defineSimpleModifier,
GlimmerishComponent,
JitRenderDelegate,
RenderTest,
suite,
test,
tracked,
trackedObj,
} from '..';

interface CapturedBounds {
Expand Down Expand Up @@ -74,6 +77,88 @@ class DebugRenderTreeTest extends RenderTest {

declare delegate: DebugRenderTreeDelegate;

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

const HelloWorld = defComponent('{{@arg}}');
const Root = defComponent(
`<HelloWorld @arg="first"/>{{#if state.showSecond}}<HelloWorld @arg="second"/>{{/if}}`,
{ scope: { HelloWorld, state }, emit: { moduleName: 'root.hbs' } }
);

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)',
wycats marked this conversation as resolved.
Show resolved Hide resolved
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
children: [],
},
],
},
]);
}

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

const HelloWorld = defComponent('<p ...attributes>{{@arg}}</p>');
const noopFn = () => {};
const noop = defineSimpleModifier(noopFn);
const Root = defComponent(
`<HelloWorld {{noop}} @arg="first"/>{{#if state.showSecond}}<HelloWorld @arg="second"/>{{/if}}`,
{ scope: { HelloWorld, state, noop }, emit: { moduleName: 'root.hbs' } }
);

this.renderComponent(Root);

const element = this.delegate.getInitialElement();

this.assertRenderTree([
{
type: 'component',
name: '{ROOT}',
args: { positional: [], named: {} },
instance: null,
template: 'root.hbs',
bounds: this.elementBounds(element),
children: [
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(element.firstChild),
children: [
{
type: 'modifier',
name: 'noop',
args: { positional: [], named: {} },
instance: (modifier: unknown) => modifier && Reflect.get(modifier, 'fn') === noopFn,
template: null,
bounds: this.nodeBounds(element.firstChild),
children: [],
},
],
},
],
},
]);
}

@test 'template-only components'() {
this.registerComponent('TemplateOnly', 'HelloWorld', '{{@arg}}');

Expand Down Expand Up @@ -313,7 +398,7 @@ class DebugRenderTreeTest extends RenderTest {
args: { positional: [this.element.firstChild], named: {} },
instance: (instance: GlimmerishComponent) => instance === null,
template: null,
bounds: this.elementBounds(this.element.firstChild! as unknown as Element),
bounds: this.elementBounds(this.element.firstChild! as unknown as SimpleElement),
children: [
{
type: 'component',
Expand Down Expand Up @@ -714,7 +799,7 @@ class DebugRenderTreeTest extends RenderTest {
};
}

elementBounds(element: Element): CapturedBounds {
elementBounds(element: SimpleElement): CapturedBounds {
return {
parentElement: element as unknown as SimpleElement,
firstNode: element.firstChild! as unknown as SimpleNode,
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
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 @@ -96,6 +96,7 @@ export interface NamedBlocks {
export interface ContainingMetadata {
evalSymbols: Nullable<string[]>;
upvars: Nullable<string[]>;
debugSymbols?: string[] | undefined;
scopeValues: unknown[] | null;
isStrictMode: boolean;
moduleName: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,23 @@ export function resolveComponent(
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')[
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 @@ -182,12 +183,12 @@ export function resolveModifier(
let type = expr[0];

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

then(constants.modifier(definition as object));
then(constants.modifier(definition as object, debugSymbols?.at(expr[1]) ?? undefined));
} else if (type === SexpOpcodes.GetStrictKeyword) {
let { upvars } = assertResolverInvariants(meta);
let name = unwrap(upvars[expr[1]]);
Expand Down Expand Up @@ -236,15 +237,16 @@ export function resolveComponentOrHelper(
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')[
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,7 +318,7 @@ export function resolveOptionalComponentOrHelper(
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')[
expr[1]
];
Expand All @@ -333,7 +335,8 @@ export function resolveOptionalComponentOrHelper(
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 @@ function lookupBuiltInHelper(
// 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
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,13 @@ export function CompilePositional(
}

export function meta(layout: LayoutWithContext): ContainingMetadata {
let [, symbols, , upvars] = layout.block;
let [, symbols, , upvars, debugSymbols] = layout.block;

return {
evalSymbols: evalSymbols(layout),
upvars: upvars,
scopeValues: layout.scope?.() ?? null,
debugSymbols,
isStrictMode: layout.isStrictMode,
moduleName: layout.moduleName,
owner: layout.owner,
Expand Down
Loading
Loading