Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
22 changes: 19 additions & 3 deletions renderers/angular/src/v0_9/core/component-binder.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('ComponentBinder', () => {
dataContext: mockDataContext,
} as unknown as ComponentContext;

const bound = binder.bind(mockContext);
const bound = binder.bind(mockContext, mockDestroyRef);

expect(bound['text']).toBeDefined();
expect(bound['visible']).toBeDefined();
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('ComponentBinder', () => {
dataContext: mockDataContext,
} as unknown as ComponentContext;

const bound = binder.bind(mockContext);
const bound = binder.bind(mockContext, mockDestroyRef);

expect(bound['value']).toBeDefined();
expect(bound['value'].value()).toBe('initial');
Expand Down Expand Up @@ -130,7 +130,7 @@ describe('ComponentBinder', () => {
dataContext: mockDataContext,
} as unknown as ComponentContext;

const bound = binder.bind(mockContext);
const bound = binder.bind(mockContext, mockDestroyRef);

expect(bound['text']).toBeDefined();
expect(bound['text'].value()).toBe('Literal String');
Expand All @@ -140,4 +140,20 @@ describe('ComponentBinder', () => {
bound['text'].onUpdate('new');
expect(mockDataContext.set).not.toHaveBeenCalled();
});

it('should dispose bound properties through disposeBoundProperties()', () => {
const disposeSpy = jasmine.createSpy('dispose');
const bound = {
value: {
value: () => 'v',
dispose: disposeSpy,
},
raw: 'v',
onUpdate: () => {},
} as any;

binder.disposeBoundProperties({ value: bound });

expect(disposeSpy).toHaveBeenCalled();
});
});
20 changes: 16 additions & 4 deletions renderers/angular/src/v0_9/core/component-binder.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { DestroyRef, Injectable, inject, NgZone } from '@angular/core';
import { DestroyRef, Injectable, NgZone, inject } from '@angular/core';
import { ComponentContext } from '@a2ui/web_core/v0_9';
import { toAngularSignal } from './utils';
import { BoundProperty } from './types';
Expand All @@ -31,7 +31,6 @@ import { BoundProperty } from './types';
providedIn: 'root',
})
export class ComponentBinder {
private destroyRef = inject(DestroyRef);
private ngZone = inject(NgZone);

/**
Expand All @@ -40,14 +39,14 @@ export class ComponentBinder {
* @param context The ComponentContext containing the model and data context.
* @returns An object where each key corresponds to a component prop and its value is an Angular Signal.
*/
bind(context: ComponentContext): Record<string, BoundProperty> {
bind(context: ComponentContext, destroyRef: DestroyRef): Record<string, BoundProperty> {
const props = context.componentModel.properties;
const bound: Record<string, any> = {};

for (const key of Object.keys(props)) {
const value = props[key];
const preactSig = context.dataContext.resolveSignal(value);
const angSig = toAngularSignal(preactSig as any, this.destroyRef, this.ngZone);
const angSig = toAngularSignal(preactSig as any, destroyRef, this.ngZone);

const isBoundPath = value && typeof value === 'object' && 'path' in value;

Expand All @@ -62,4 +61,17 @@ export class ComponentBinder {

return bound;
}

disposeBoundProperties(bound: Record<string, BoundProperty> | undefined): void {
if (!bound) {
return;
}

for (const prop of Object.values(bound)) {
const dispose = (prop.value as any)?.dispose;
if (typeof dispose === 'function') {
dispose();
}
}
}
}
36 changes: 34 additions & 2 deletions renderers/angular/src/v0_9/core/component-host.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,34 @@ describe('ComponentHostComponent', () => {
let mockBinder: jasmine.SpyObj<ComponentBinder>;
let mockSurface: any;
let mockSurfaceGroup: any;
let onUpdatedHandler: (() => void) | undefined;
let onUpdatedUnsubscribeSpy: jasmine.Spy;

beforeEach(async () => {
mockCatalog = {
id: 'test-catalog',
components: new Map([['TestType', { component: TestChildComponent }]]),
};

onUpdatedUnsubscribeSpy = jasmine.createSpy('onUpdated.unsubscribe');
mockSurface = {
componentsModel: new Map([
['comp1', { id: 'comp1', type: 'TestType', properties: { text: 'Hello' } }],
[
'comp1',
{
id: 'comp1',
type: 'TestType',
properties: { text: 'Hello' },
onUpdated: {
subscribe: jasmine
.createSpy('onUpdated.subscribe')
.and.callFake((handler: () => void) => {
onUpdatedHandler = handler;
return { unsubscribe: onUpdatedUnsubscribeSpy };
}),
},
},
],
]),
catalog: mockCatalog,
};
Expand All @@ -64,7 +82,7 @@ describe('ComponentHostComponent', () => {
surfaceGroup: mockSurfaceGroup,
};

mockBinder = jasmine.createSpyObj('ComponentBinder', ['bind']);
mockBinder = jasmine.createSpyObj('ComponentBinder', ['bind', 'disposeBoundProperties']);
mockBinder.bind.and.returnValue({
text: { value: () => 'bound-hello', onUpdate: () => {} } as any,
});
Expand Down Expand Up @@ -100,6 +118,7 @@ describe('ComponentHostComponent', () => {

expect(mockSurfaceGroup.getSurface).toHaveBeenCalledWith('surf1');
expect(mockBinder.bind).toHaveBeenCalled();
expect(mockBinder.bind.calls.mostRecent().args[1]).toBeDefined();

// Verify context creation implicitly by checking if bind was called with a ComponentContext
const bindArg = mockBinder.bind.calls.mostRecent().args[0];
Expand Down Expand Up @@ -157,9 +176,22 @@ describe('ComponentHostComponent', () => {
// Destroy fixture
fixture.destroy();

expect(onUpdatedUnsubscribeSpy).toHaveBeenCalled();
expect(mockBinder.disposeBoundProperties).toHaveBeenCalled();

// Implicitly verifies no crash on destroy
expect(component).toBeTruthy();
});

it('should refresh bindings when component properties are updated', () => {
fixture.detectChanges();
expect(mockBinder.bind).toHaveBeenCalledTimes(1);

onUpdatedHandler?.();

expect(mockBinder.disposeBoundProperties).toHaveBeenCalled();
expect(mockBinder.bind).toHaveBeenCalledTimes(2);
});
});

describe('Template rendering', () => {
Expand Down
19 changes: 16 additions & 3 deletions renderers/angular/src/v0_9/core/component-host.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,24 @@ export class ComponentHostComponent implements OnInit {

// Create context
this.context = new ComponentContext(surface, this.componentId(), this.dataContextPath());
this.props = this.binder.bind(this.context);
this.bindProps();

const updatedSubscription = componentModel.onUpdated.subscribe(() => {
this.bindProps();
});

this.destroyRef.onDestroy(() => {
// ComponentContext itself doesn't have a dispose, but its inner components might.
// However, SurfaceModel takes care of component disposal.
updatedSubscription.unsubscribe();
this.binder.disposeBoundProperties(this.props);
});
}

private bindProps(): void {
if (!this.context) {
return;
}

this.binder.disposeBoundProperties(this.props);
this.props = this.binder.bind(this.context, this.destroyRef);
}
}
30 changes: 25 additions & 5 deletions renderers/angular/src/v0_9/core/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
* limitations under the License.
*/

import { DestroyRef, Signal, signal as angularSignal } from '@angular/core';
import {
DestroyRef,
NgZone,
Signal,
signal as angularSignal,
} from '@angular/core';
import { Signal as PreactSignal, effect } from '@a2ui/web_core/v0_9';

/**
Expand All @@ -31,7 +36,10 @@ import { Signal as PreactSignal, effect } from '@a2ui/web_core/v0_9';
* (necessary for correct change detection in OnPush components).
* @returns A read-only Angular Signal.
*/
import { NgZone } from '@angular/core';

type ManagedAngularSignal<T> = Signal<T> & {
dispose?: () => void;
};

export function toAngularSignal<T>(
preactSignal: PreactSignal<T>,
Expand All @@ -48,15 +56,27 @@ export function toAngularSignal<T>(
}
});

destroyRef.onDestroy(() => {
let isDisposed = false;
const cleanup = () => {
if (isDisposed) {
return;
}
isDisposed = true;

dispose();

// Some signals returned by DataContext.resolveSignal have a custom unsubscribe for AbortControllers
if ((preactSignal as any).unsubscribe) {
(preactSignal as any).unsubscribe();
}
});
};

destroyRef.onDestroy(cleanup);

const readonlySignal = s.asReadonly() as ManagedAngularSignal<T>;
readonlySignal.dispose = cleanup;

return s.asReadonly();
return readonlySignal;
}

/**
Expand Down
Loading