Skip to content

Commit 17ca94f

Browse files
Merge pull request #1492 from patricklx/patch-2
improve arguments error detection for inspector
2 parents 1cb493b + 91731da commit 17ca94f

File tree

6 files changed

+101
-6
lines changed

6 files changed

+101
-6
lines changed

packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts

+34-4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
RenderTest,
2424
suite,
2525
test,
26+
tracked,
2627
} from '..';
2728

2829
interface CapturedBounds {
@@ -135,33 +136,62 @@ class DebugRenderTreeTest extends RenderTest {
135136

136137
@test 'emberish curly components'() {
137138
this.registerComponent('Curly', 'HelloWorld', 'Hello World');
139+
let error: Error | null = null;
140+
class State {
141+
@tracked doFail = false;
142+
get getterWithError() {
143+
if (!this.doFail) return;
144+
error = new Error('error');
145+
throw error;
146+
}
147+
}
148+
const obj = new State();
138149

139150
this.render(
140-
`<HelloWorld @arg="first"/>{{#if this.showSecond}}<HelloWorld @arg="second"/>{{/if}}`,
151+
`<HelloWorld @arg="first" @arg2={{this.obj.getterWithError}}/>{{#if this.showSecond}}<HelloWorld @arg="second"/>{{/if}}`,
141152
{
142153
showSecond: false,
154+
obj,
143155
}
144156
);
145157

158+
obj.doFail = true;
159+
160+
this.delegate.getCapturedRenderTree();
161+
162+
this.assert.ok(error !== null, 'expecting an Error');
163+
146164
this.assertRenderTree([
147165
{
148166
type: 'component',
149167
name: 'HelloWorld',
150-
args: { positional: [], named: { arg: 'first' } },
168+
args: (actual) => {
169+
const args = { positional: [], named: { arg: 'first', arg2: { error } } };
170+
this.assert.deepEqual(actual, args);
171+
this.assert.ok(
172+
!this.delegate.context.runtime.env.isArgumentCaptureError!(actual.named['arg'])
173+
);
174+
this.assert.ok(
175+
this.delegate.context.runtime.env.isArgumentCaptureError!(actual.named['arg2'])
176+
);
177+
return true;
178+
},
151179
instance: (instance: EmberishCurlyComponent) => (instance as any).arg === 'first',
152180
template: '(unknown template module)',
153181
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
154182
children: [],
155183
},
156184
]);
157185

186+
obj.doFail = false;
187+
158188
this.rerender({ showSecond: true });
159189

160190
this.assertRenderTree([
161191
{
162192
type: 'component',
163193
name: 'HelloWorld',
164-
args: { positional: [], named: { arg: 'first' } },
194+
args: { positional: [], named: { arg: 'first', arg2: undefined } },
165195
instance: (instance: EmberishCurlyComponent) => (instance as any).arg === 'first',
166196
template: '(unknown template module)',
167197
bounds: this.nodeBounds(this.element.firstChild),
@@ -184,7 +214,7 @@ class DebugRenderTreeTest extends RenderTest {
184214
{
185215
type: 'component',
186216
name: 'HelloWorld',
187-
args: { positional: [], named: { arg: 'first' } },
217+
args: { positional: [], named: { arg: 'first', arg2: undefined } },
188218
instance: (instance: EmberishCurlyComponent) => (instance as any).arg === 'first',
189219
template: '(unknown template module)',
190220
bounds: this.nodeBounds(this.element.firstChild),

packages/@glimmer/interfaces/lib/runtime/arguments.d.ts

+11
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,14 @@ export interface Arguments {
6060
positional: readonly unknown[];
6161
named: Record<string, unknown>;
6262
}
63+
64+
export interface ArgumentsDebug {
65+
positional: readonly unknown[];
66+
named: Record<string, unknown>;
67+
}
68+
69+
export interface ArgumentError {
70+
error: any;
71+
}
72+
73+
export function isArgumentError(arg: unknown): arg is ArgumentError;

packages/@glimmer/interfaces/lib/runtime/environment.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,5 @@ export interface Environment {
4545

4646
isInteractive: boolean;
4747
debugRenderTree?: DebugRenderTree | undefined;
48+
isArgumentCaptureError?: ((error: any) => boolean) | undefined;
4849
}

packages/@glimmer/runtime/lib/debug-render-tree.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type {
77
} from '@glimmer/interfaces';
88
import { assign, expect, Stack } from '@glimmer/util';
99

10-
import { reifyArgs } from './vm/arguments';
10+
import { reifyArgsDebug } from './vm/arguments';
1111

1212
interface InternalRenderNode<T extends object> extends RenderNode {
1313
bounds: Nullable<Bounds>;
@@ -181,7 +181,7 @@ export default class DebugRenderTreeImpl<TBucket extends object>
181181
let template = this.captureTemplate(node);
182182
let bounds = this.captureBounds(node);
183183
let children = this.captureRefs(refs);
184-
return { id, type, name, args: reifyArgs(args), instance, template, bounds, children };
184+
return { id, type, name, args: reifyArgsDebug(args), instance, template, bounds, children };
185185
}
186186

187187
private captureTemplate({ template }: InternalRenderNode<TBucket>): Nullable<string> {

packages/@glimmer/runtime/lib/environment.ts

+3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { track, updateTag } from '@glimmer/validator';
1818

1919
import DebugRenderTree from './debug-render-tree';
2020
import { DOMChangesImpl, DOMTreeConstruction } from './dom/helper';
21+
import { isArgumentError } from './vm/arguments';
2122

2223
export const TRANSACTION: TransactionSymbol = Symbol('TRANSACTION') as TransactionSymbol;
2324

@@ -101,6 +102,7 @@ export class EnvironmentImpl implements Environment {
101102
// Delegate methods and values
102103
public isInteractive: boolean;
103104

105+
isArgumentCaptureError: ((error: any) => boolean) | undefined;
104106
debugRenderTree: DebugRenderTree<object> | undefined;
105107

106108
constructor(
@@ -109,6 +111,7 @@ export class EnvironmentImpl implements Environment {
109111
) {
110112
this.isInteractive = delegate.isInteractive;
111113
this.debugRenderTree = this.delegate.enableDebugTooling ? new DebugRenderTree() : undefined;
114+
this.isArgumentCaptureError = this.delegate.enableDebugTooling ? isArgumentError : undefined;
112115
if (options.appendOperations) {
113116
this.appendOperations = options.appendOperations;
114117
this.updateOperations = options.updateOperations;

packages/@glimmer/runtime/lib/vm/arguments.ts

+50
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type {
2+
ArgumentError,
23
BlockArguments,
34
BlockSymbolTable,
45
BlockValue,
@@ -501,6 +502,55 @@ export function reifyArgs(args: CapturedArguments) {
501502
};
502503
}
503504

505+
const ARGUMENT_ERROR = Symbol('ARGUMENT_ERROR');
506+
507+
export function isArgumentError(arg: unknown): arg is ArgumentError {
508+
return (
509+
arg !== null &&
510+
typeof arg === 'object' &&
511+
(arg as { [ARGUMENT_ERROR]: boolean })[ARGUMENT_ERROR]
512+
);
513+
}
514+
515+
function ArgumentErrorImpl(error: any) {
516+
return {
517+
[ARGUMENT_ERROR]: true,
518+
error,
519+
};
520+
}
521+
522+
export function reifyNamedDebug(named: CapturedNamedArguments) {
523+
let reified = dict();
524+
for (const [key, value] of Object.entries(named)) {
525+
try {
526+
reified[key] = valueForRef(value);
527+
} catch (e) {
528+
reified[key] = ArgumentErrorImpl(e);
529+
}
530+
}
531+
532+
return reified;
533+
}
534+
535+
export function reifyPositionalDebug(positional: CapturedPositionalArguments) {
536+
return positional.map((p) => {
537+
try {
538+
return valueForRef(p);
539+
} catch (e) {
540+
return ArgumentErrorImpl(e);
541+
}
542+
});
543+
}
544+
545+
export function reifyArgsDebug(args: CapturedArguments) {
546+
let named = reifyNamedDebug(args.named);
547+
let positional = reifyPositionalDebug(args.positional);
548+
return {
549+
named,
550+
positional,
551+
};
552+
}
553+
504554
export const EMPTY_NAMED = Object.freeze(Object.create(null)) as CapturedNamedArguments;
505555
export const EMPTY_POSITIONAL = EMPTY_REFERENCES as CapturedPositionalArguments;
506556
export const EMPTY_ARGS = createCapturedArgs(EMPTY_NAMED, EMPTY_POSITIONAL);

0 commit comments

Comments
 (0)