Skip to content

Commit

Permalink
Merge pull request #1460 from patricklx/patch-3
Browse files Browse the repository at this point in the history 
fix capture render tree fails when errors in args
  • Loading branch information
@NullVoxPopuli
NullVoxPopuli authored Nov 2, 2023
2 parents 77faa4d + 626b0e7 commit 71f1b70
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 27 deletions.
63 changes: 41 additions & 22 deletions packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
JitRenderDelegate,
RenderTest,
suite,
test,
test, tracked,
} from '..';

interface CapturedBounds {
Expand Down Expand Up @@ -90,7 +90,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
Expand All @@ -104,7 +104,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
Expand All @@ -113,7 +113,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'second' } },
args: { positional: [], named: { arg: 'second' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().lastChild),
Expand All @@ -127,7 +127,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
Expand All @@ -138,33 +138,52 @@ class DebugRenderTreeTest extends RenderTest {

@test 'emberish curly components'() {
this.registerComponent('Curly', 'HelloWorld', 'Hello World');
let error: Error|null = null;
class State {
@tracked doFail = false;
get getterWithError() {
if (!this.doFail) return;
error = new Error('error');
throw error;
}
}
const obj = new State();

this.render(
`<HelloWorld @arg="first"/>{{#if this.showSecond}}<HelloWorld @arg="second"/>{{/if}}`,
`<HelloWorld @arg="first" @arg2={{this.obj.getterWithError}}/>{{#if this.showSecond}}<HelloWorld @arg="second"/>{{/if}}`,
{
showSecond: false,
obj,
}
);

obj.doFail = true;

this.delegate.getCapturedRenderTree();

this.assert.ok(error !== null, 'expecting an Error');

this.assertRenderTree([
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first', arg2: error }, errors: { arg2: error! } },
instance: (instance: EmberishCurlyComponent) => (instance as any).arg === 'first',
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
children: [],
},
]);

obj.doFail = false;

this.rerender({ showSecond: true });

this.assertRenderTree([
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first', arg2: undefined }, errors: {} },
instance: (instance: EmberishCurlyComponent) => (instance as any).arg === 'first',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand All @@ -173,7 +192,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'second' } },
args: { positional: [], named: { arg: 'second' }, errors: {} },
instance: (instance: EmberishCurlyComponent) => (instance as any).arg === 'second',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.lastChild),
Expand All @@ -187,7 +206,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first', arg2: undefined }, errors: {} },
instance: (instance: EmberishCurlyComponent) => (instance as any).arg === 'first',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand All @@ -210,7 +229,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: (instance: GlimmerishComponent) => instance.args['arg'] === 'first',
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
Expand All @@ -224,7 +243,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: (instance: GlimmerishComponent) => instance.args['arg'] === 'first',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand All @@ -233,7 +252,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'second' } },
args: { positional: [], named: { arg: 'second' }, errors: {} },
instance: (instance: GlimmerishComponent) => instance.args['arg'] === 'second',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.lastChild),
Expand All @@ -247,7 +266,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: (instance: GlimmerishComponent) => instance.args['arg'] === 'first',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand Down Expand Up @@ -307,7 +326,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld2',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
Expand All @@ -321,7 +340,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld2',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand All @@ -330,15 +349,15 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'route-template',
name: 'foo',
args: { positional: [], named: { arg: 'second' } },
args: { positional: [], named: { arg: 'second' }, errors: {} },
instance: instance1,
template: null,
bounds: this.nodeBounds(this.element.lastChild),
children: [
{
type: 'engine',
name: 'bar',
args: { positional: [], named: {} },
args: { positional: [], named: {}, errors: {} },
instance: instance2,
template: null,
bounds: this.nodeBounds(this.element.lastChild),
Expand All @@ -354,7 +373,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld2',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand Down Expand Up @@ -387,7 +406,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld2',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.delegate.getInitialElement().firstChild),
Expand All @@ -401,7 +420,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld2',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand All @@ -415,7 +434,7 @@ class DebugRenderTreeTest extends RenderTest {
{
type: 'component',
name: 'HelloWorld2',
args: { positional: [], named: { arg: 'first' } },
args: { positional: [], named: { arg: 'first' }, errors: {} },
instance: null,
template: '(unknown template module)',
bounds: this.nodeBounds(this.element.firstChild),
Expand Down
6 changes: 6 additions & 0 deletions packages/@glimmer/interfaces/lib/runtime/arguments.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,9 @@ export interface Arguments {
positional: readonly unknown[];
named: Record<string, unknown>;
}

export interface ArgumentsDebug {
positional: readonly unknown[];
named: Record<string, unknown>;
errors: Record<string, Error>;
}
4 changes: 2 additions & 2 deletions packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { SimpleElement, SimpleNode } from '@simple-dom/interface';

import type { Bounds } from '../dom/bounds';
import type { Arguments, CapturedArguments } from './arguments';
import type { ArgumentsDebug, CapturedArguments } from './arguments';

export type RenderNodeType = 'outlet' | 'engine' | 'route-template' | 'component';

Expand All @@ -17,7 +17,7 @@ export interface CapturedRenderNode {
id: string;
type: RenderNodeType;
name: string;
args: Arguments;
args: ArgumentsDebug;
instance: unknown;
template: string | null;
bounds: null | {
Expand Down
4 changes: 2 additions & 2 deletions packages/@glimmer/runtime/lib/debug-render-tree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {
} from "@glimmer/interfaces";
import { assign, expect, Stack } from '@glimmer/util';

import { reifyArgs } from './vm/arguments';
import { reifyArgsDebug } from './vm/arguments';

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

private captureTemplate({ template }: InternalRenderNode<TBucket>): Nullable<string> {
Expand Down
42 changes: 42 additions & 0 deletions packages/@glimmer/runtime/lib/vm/arguments.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,48 @@ export function reifyArgs(args: CapturedArguments) {
};
}

export function reifyNamedDebug(named: CapturedNamedArguments) {
let reified = dict();
let errors: Record<string, Error> = dict();

for (const [key, value] of Object.entries(named)) {
try {
reified[key] = valueForRef(value);
} catch (e) {
reified[key] = e;
errors[key] = e as Error;
}
}

return { reified, errors };
}

export function reifyPositionalDebug(positional: CapturedPositionalArguments) {
let errors: Error[] = [];
let reified = positional.map((p) => {
try {
return valueForRef(p);
} catch(e) {
errors.push(e as Error);
return e;
}
});
return {
reified,
errors
}
}

export function reifyArgsDebug(args: CapturedArguments) {
let named = reifyNamedDebug(args.named);
let positional = reifyPositionalDebug(args.positional);
return {
named: named.reified,
positional: positional.reified,
errors: {...named.errors, ...positional.errors} as unknown as Record<string, Error >
};
}

export const EMPTY_NAMED = Object.freeze(Object.create(null)) as CapturedNamedArguments;
export const EMPTY_POSITIONAL = EMPTY_REFERENCES as CapturedPositionalArguments;
export const EMPTY_ARGS = createCapturedArgs(EMPTY_NAMED, EMPTY_POSITIONAL);
2 changes: 1 addition & 1 deletion packages/@glimmer/vm-babel-plugins/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default function generateVmPlugins(
return [
[
__loadPlugins
?
?
require('babel-plugin-debug-macros')
: require.resolve('babel-plugin-debug-macros'),
{
Expand Down

0 comments on commit 71f1b70

Please sign in to comment.