Conversation
Includes: - v0.9 renderer components in v0_9 folder - a2ui_explorer app moved to samples/ - Fixed tsconfig (moduleResolution: bundler, module: esnext) and angular.json (preserveSymlinks: false) resolving for linked secondary entries
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed architectural change by adding a new v0.9 Angular renderer and refactoring the existing one into a v0.8 structure. The new v0.9 renderer's signal-based architecture with a ComponentHost is a great improvement. The inclusion of a comprehensive sample application is also very helpful for demonstrating the new capabilities. Most of the changes improve performance and type safety. I've identified one critical bug in the v0.8 catalog bindings, a high-severity issue in another binding, and a minor code quality issue in a test file that should be addressed.
| inputBinding('text', () => properties.text ?? null), | ||
| inputBinding('label', () => properties.label), | ||
| inputBinding('textFieldType', () => properties.textFieldType), | ||
| inputBinding('inputType', () => properties.textFieldType), |
There was a problem hiding this comment.
There's a mismatch between the input binding name in the catalog and the actual input property name in the TextField component. The catalog is attempting to bind to an input named inputType, but the component's input is named textFieldType. This will cause a runtime error as the inputType input does not exist on the component.
| inputBinding('inputType', () => properties.textFieldType), | |
| inputBinding('textFieldType', () => properties.textFieldType), |
| return [ | ||
| inputBinding('text', () => properties.text), | ||
| inputBinding('usageHint', () => properties.usageHint || null), | ||
| inputBinding('usageHint', () => properties.usageHint), |
There was a problem hiding this comment.
The binding for usageHint has been changed from () => properties.usageHint || null to () => properties.usageHint. The Text component's usageHint input is required and does not accept undefined. If properties.usageHint is not present on the node, this binding will pass undefined, leading to a runtime error. The previous implementation was safer by coercing undefined to null.
| inputBinding('usageHint', () => properties.usageHint), | |
| inputBinding('usageHint', () => properties.usageHint ?? null), |
| additionalStyles: {}, | ||
| }, | ||
| }, | ||
| { provide: MessageProcessor, useValue: mockMessageProcessor }, |
There was a problem hiding this comment.
The MessageProcessor is provided twice in this test configuration (here and on line 141). This is redundant and should be removed to keep the test setup clean.
1 participant
Description
This PR adds the v0.9 Angular renderer and packages it into secondary entry points with appropriate
package.jsonconfigurations. Additionally, it integrates thea2ui_explorersample app into thesamples/workspace and verifies application bundle generation fully compiles.Changes
renderers/angular/v0_9/: Added the complete v0.9 Angular code (components and core).renderers/angular/ng-package.json&package.json: Created subpath mappings (@a2ui/angular/v0_9) for standardexportslookup.samples/client/angular/projects/a2ui_explorer/: Imported and decoupleddemo-appfrom Lit, updating forA2uiRendererServicebindings and dynamic rendering setup.preserveSymlinksand configuringmoduleResolution: "bundler"insidesamplesworkspace configurations to fix subpath resolution for linked packages.Impact & Risks
@a2ui/angular/v0_9node subpaths.Testing
npx ng build a2ui_explorersuccessfully insamples/client/angular/workspace.