feat(dashboards-ng): support widgets as standalone component #1036
Conversation
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
chintankavathia
force-pushed
the
feat/dashboards/support-standalone-widgets
branch
4 times, most recently
from
35c191a to
df66eba
Compare
|
Documentation. Coverage Reports: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
projects/dashboards-demo/src/app/widgets/hello-widget/widget-descriptors.ts (1)
7-13: Migration to async component loading looks good.The loader function correctly implements dynamic component loading with appropriate error handling.
Consider two optional improvements:
Type safety: Replace
Promise<any>with a more specific return type to preserve type information.Code reuse: This loader pattern is duplicated in
contact-descriptors.tsandcharts/widget-descriptors.ts. Consider extracting a shared factory function:function createComponentLoader(modulePath: string, allowedComponents: string[]) { return async (name: string): Promise<any> => { if (allowedComponents.includes(name)) { return import(modulePath).then(m => m[name as keyof typeof m]); } throw new Error(`Unknown component to be loaded ${name}`); }; } const loaderFunction = createComponentLoader('./index', [ 'HelloWidgetComponent', 'HelloWidgetEditorComponent' ]);projects/dashboards-demo/src/app/widgets/contact-widget/contact-descriptors.ts (1)
7-13: Duplicate loader pattern detected.This loader function is nearly identical to the one in
hello-widget/widget-descriptors.ts(lines 7-13). Consider the shared factory approach suggested in that file's review to eliminate this duplication.projects/dashboards-demo/src/app/widgets/charts/widget-descriptors.ts (1)
7-21: Loader function follows established pattern but scales poorly.The implementation is correct, but this file demonstrates why the duplicated loader pattern creates maintenance overhead - the component list must be manually kept in sync with exports.
The shared factory approach suggested in
hello-widget/widget-descriptors.tswould be especially beneficial here given the larger number of components to maintain.projects/dashboards-ng/src/model/widgets.model.ts (1)
5-5: Widget factory typing for module vs standalone looks consistent; consider tightening componentLoader typingThe new
WidgetComponentTypeBaseConfig+ModuleOptions/StandaloneOptionsunion correctly enforces mutual exclusivity ofmoduleLoaderandcomponentLoaderand matches the loader logic. You could optionally:
- Narrow
componentLoader’s return type away fromPromise<Type<unknown>>/anytoward a more specific generic (e.g.,Type<WidgetInstance | WidgetInstanceEditor>) to improve editor/compile‑time help.- Reconsider the
[index: string]: anyon the base config if you want stronger type safety for future consumers; right now it hides many mistakes at compile time.These are refinements rather than blockers; the current shape is functionally fine.
Also applies to: 59-79
projects/dashboards-ng/src/widget-loader.ts (1)
117-151: Add a defensive guard when neither moduleLoader nor componentLoader is definedThe fallback
const loader = factory.moduleLoader ?? factory.componentLoader;is correct for well‑typed factories, but if a consumer misconfigures a widget (e.g. JS usage oranycasts),loadercan beundefined, causing a hard runtime error when invoked instead of a clear message.You can cheaply harden this with a guard and slightly simplify error formatting:
- if (factory[componentName]) { - const loader = factory.moduleLoader ?? factory.componentLoader; - loader(factory[componentName]!).then( + if (factory[componentName]) { + const loader = factory.moduleLoader ?? factory.componentLoader; + if (!loader) { + result.error( + `Provided component factory has neither moduleLoader nor componentLoader configured` + ); + return result; + } + loader(factory[componentName]!).then( @@ - const msg = rejection - ? `${errorMsg} with ${JSON.stringify(rejection.toString())}` - : `${errorMsg}`; + const msg = rejection + ? `${errorMsg} with ${String(rejection)}` + : errorMsg;Functionality stays the same for valid configs but failure modes become clearer and less brittle.
Also applies to: 137-144
projects/dashboards-ng/README.md (1)
167-175: Widget development section matches implementation; consider adding a tiny exampleThe description that widgets “have to be provided by an Angular module or as standalone” and that you “must provide either a module loader function with module name or a component loader function for standalone components” is in line with the new
WidgetComponentFactory/loader behavior. For extra clarity, you could add a short module‑based vs standalonecomponentFactorysnippet so users see both concrete shapes immediately.projects/dashboards-ng/test/test-widget/test-widget.ts (1)
15-23: Standalone loader aligns with new componentLoader contract; type could be made more specific
loaderFunctionStandalonecorrectly returns the concrete component export and matches thecomponentLoader(componentName: string) => Promise<...>usage inwidget-loader.ts. You could optionally tighten the signature fromPromise<any>to the sameType<unknown>(or a more specific generic) used inStandaloneOptionsto keep type information consistent end‑to‑end.projects/dashboards-ng/src/components/widget-host/si-widget-host.component.spec.ts (1)
35-160: Nice parametric tests over both widget variants; decouple config from TEST_WIDGET idLooping over
{ widget, name }to run the same suite forTEST_WIDGETandTEST_WIDGET_STANDALONEis a good way to guarantee behavior parity between module‑based and standalone widgets. The only small coupling is thatTEST_WIDGET_CONFIG_0.widgetIdis hard‑coded toTEST_WIDGET.id, so the standalone tests rely on both widgets sharing the same id string.You could make the tests more future‑proof by overriding
widgetIdper variant when setting up the input:- gridService.widgetCatalog.set([widget]); - fixture.componentRef.setInput('widgetConfig', TEST_WIDGET_CONFIG_0); + gridService.widgetCatalog.set([widget]); + fixture.componentRef.setInput('widgetConfig', { + ...TEST_WIDGET_CONFIG_0, + widgetId: widget.id + });That way, the tests stay correct even if you later give the standalone widget a distinct id.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
projects/dashboards-demo/src/app/widgets/charts/charts-widget.module.ts(0 hunks)projects/dashboards-demo/src/app/widgets/charts/index.ts(0 hunks)projects/dashboards-demo/src/app/widgets/charts/widget-descriptors.ts(9 hunks)projects/dashboards-demo/src/app/widgets/contact-widget/contact-descriptors.ts(2 hunks)projects/dashboards-demo/src/app/widgets/contact-widget/contact-widget.module.ts(0 hunks)projects/dashboards-demo/src/app/widgets/contact-widget/index.ts(0 hunks)projects/dashboards-demo/src/app/widgets/hello-widget/hello-widget.module.ts(0 hunks)projects/dashboards-demo/src/app/widgets/hello-widget/index.ts(0 hunks)projects/dashboards-demo/src/app/widgets/hello-widget/widget-descriptors.ts(2 hunks)projects/dashboards-ng/README.md(2 hunks)projects/dashboards-ng/src/components/widget-host/si-widget-host.component.spec.ts(2 hunks)projects/dashboards-ng/src/model/widgets.model.ts(2 hunks)projects/dashboards-ng/src/widget-loader.ts(1 hunks)projects/dashboards-ng/test/test-widget/test-widget.ts(2 hunks)
💤 Files with no reviewable changes (6)
- projects/dashboards-demo/src/app/widgets/hello-widget/hello-widget.module.ts
- projects/dashboards-demo/src/app/widgets/charts/index.ts
- projects/dashboards-demo/src/app/widgets/hello-widget/index.ts
- projects/dashboards-demo/src/app/widgets/contact-widget/contact-widget.module.ts
- projects/dashboards-demo/src/app/widgets/contact-widget/index.ts
- projects/dashboards-demo/src/app/widgets/charts/charts-widget.module.ts
🧰 Additional context used
🧬 Code graph analysis (2)
projects/dashboards-ng/src/components/widget-host/si-widget-host.component.spec.ts (1)
projects/dashboards-ng/test/test-widget/test-widget.ts (3)
TEST_WIDGET(25-43)TEST_WIDGET_STANDALONE(45-62)TEST_WIDGET_CONFIG_0(64-74)
projects/dashboards-ng/test/test-widget/test-widget.ts (1)
projects/dashboards-ng/src/model/widgets.model.ts (1)
Widget(16-44)
🔇 Additional comments (6)
projects/dashboards-demo/src/app/widgets/hello-widget/widget-descriptors.ts (1)
20-25: LGTM! ComponentFactory correctly migrated to standalone pattern.The switch from
moduleName/moduleLoadertocomponentLoaderaligns with the PR's objective to support widgets as standalone components.projects/dashboards-demo/src/app/widgets/contact-widget/contact-descriptors.ts (1)
20-24: ComponentFactory migration looks correct.The transition to
componentLoadermaintains consistency with the standalone component pattern.projects/dashboards-demo/src/app/widgets/charts/widget-descriptors.ts (1)
23-179: All widget descriptors correctly migrated to componentLoader.All seven widget descriptors (LINE_CHART_DESC, BAR_CHART_DESC, CIRCLE_CHART_DESC, GAUGE_CHART_DESC, VALUE_WIDGET, LIST_WIDGET, TIMELINE_WIDGET) have been consistently updated to use
componentLoader, maintaining the standalone component pattern across the board.projects/dashboards-ng/README.md (1)
19-19: Docs note about standalone and modules is accurate“The library supports standalone components and modules.” correctly reflects the current API surface; no change needed.
projects/dashboards-ng/test/test-widget/test-widget.ts (1)
45-62: TEST_WIDGET_STANDALONE definition mirrors TEST_WIDGET and looks correctThe standalone test widget reuses the same id, defaults, and payload as
TEST_WIDGETwhile switching tocomponentLoader: loaderFunctionStandalone, which is exactly what you need to exercise both loading paths without duplicating test data. No issues from a test perspective.projects/dashboards-ng/src/components/widget-host/si-widget-host.component.spec.ts (1)
17-21: Import of TEST_WIDGET_STANDALONE is consistent with new test widgetAdding
TEST_WIDGET_STANDALONEalongsideTEST_WIDGETandTEST_WIDGET_CONFIG_0is consistent with the new standalone widget definition and keeps all test fixtures in one place.
Support widgets as standalone Angular components. This change maintains backward compatibility with existing module-based widgets while enabling modern standalone component architecture.
Usage:
For standalone components, use `componentLoader` in your widget descriptor:
```ts
const loaderFunction = async (name: string): Promise<any> => {
if (name === 'MyWidgetComponent' || name === 'MyWidgetEditorComponent') {
return import('./index').then(m => m[name as keyof typeof m]);
}
throw new Error(`Unknown component: ${name}`);
};
export const MY_WIDGET: Widget = {
name: 'My Widget',
id: '@myapp/my-widget',
componentFactory: {
componentName: 'MyWidgetComponent',
editorComponentName: 'MyWidgetEditorComponent',
componentLoader: loaderFunction
},
...
};
```
chintankavathia
force-pushed
the
feat/dashboards/support-standalone-widgets
branch
from
df66eba to
4f86474
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
projects/dashboards-demo/src/app/widgets/charts/widget-descriptors.ts (1)
7-21: Consider simplifying the async function and addressing code duplication.The
asyncfunction uses.then()instead ofawait, which is less idiomatic. Additionally, this pattern is duplicated intest-widget.ts(loaderFunctionStandalone). Consider:
- Using
awaitfor cleaner async code- Creating a shared utility function to reduce duplication
Apply this diff to simplify the function:
-const loaderFunction = async (name: string): Promise<any> => { +const loaderFunction = async (name: string): Promise<any> => { if ( name === 'CircleComponent' || name === 'CartesianComponent' || name === 'GaugeComponent' || name === 'ValueWidgetComponent' || name === 'ValueWidgetEditorComponent' || name === 'ListWidgetComponent' || name === 'TimelineWidgetComponent' ) { - return import('../../widgets/charts/index').then(m => m[name as keyof typeof m]); + const m = await import('../../widgets/charts/index'); + return m[name as keyof typeof m]; } else { throw new Error(`Unknown component to be loaded ${name}`); } };projects/dashboards-ng/test/test-widget/test-widget.ts (1)
15-23: Code duplication with widget-descriptors.ts loaderFunction.The
loaderFunctionStandalonefollows the same pattern as theloaderFunctioninprojects/dashboards-demo/src/app/widgets/charts/widget-descriptors.ts. While the duplication is currently limited to test code, consider creating a shared utility function if this pattern is used in additional places to improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
projects/dashboards-demo/src/app/widgets/charts/charts-widget.module.ts(0 hunks)projects/dashboards-demo/src/app/widgets/charts/index.ts(0 hunks)projects/dashboards-demo/src/app/widgets/charts/widget-descriptors.ts(9 hunks)projects/dashboards-demo/src/app/widgets/contact-widget/contact-descriptors.ts(2 hunks)projects/dashboards-demo/src/app/widgets/contact-widget/contact-widget.module.ts(0 hunks)projects/dashboards-demo/src/app/widgets/contact-widget/index.ts(0 hunks)projects/dashboards-demo/src/app/widgets/hello-widget/hello-widget.module.ts(0 hunks)projects/dashboards-demo/src/app/widgets/hello-widget/index.ts(0 hunks)projects/dashboards-demo/src/app/widgets/hello-widget/widget-descriptors.ts(2 hunks)projects/dashboards-ng/README.md(2 hunks)projects/dashboards-ng/src/components/widget-host/si-widget-host.component.spec.ts(2 hunks)projects/dashboards-ng/src/model/widgets.model.ts(2 hunks)projects/dashboards-ng/src/widget-loader.ts(1 hunks)projects/dashboards-ng/test/test-widget/test-widget.ts(2 hunks)
💤 Files with no reviewable changes (6)
- projects/dashboards-demo/src/app/widgets/charts/index.ts
- projects/dashboards-demo/src/app/widgets/hello-widget/index.ts
- projects/dashboards-demo/src/app/widgets/hello-widget/hello-widget.module.ts
- projects/dashboards-demo/src/app/widgets/contact-widget/contact-widget.module.ts
- projects/dashboards-demo/src/app/widgets/contact-widget/index.ts
- projects/dashboards-demo/src/app/widgets/charts/charts-widget.module.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- projects/dashboards-demo/src/app/widgets/contact-widget/contact-descriptors.ts
- projects/dashboards-ng/src/widget-loader.ts
- projects/dashboards-demo/src/app/widgets/hello-widget/widget-descriptors.ts
- projects/dashboards-ng/src/model/widgets.model.ts
🧰 Additional context used
🧬 Code graph analysis (2)
projects/dashboards-ng/src/components/widget-host/si-widget-host.component.spec.ts (1)
projects/dashboards-ng/test/test-widget/test-widget.ts (3)
TEST_WIDGET(25-43)TEST_WIDGET_STANDALONE(45-62)TEST_WIDGET_CONFIG_0(64-74)
projects/dashboards-ng/test/test-widget/test-widget.ts (1)
projects/dashboards-ng/src/model/widgets.model.ts (1)
Widget(16-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
projects/dashboards-ng/README.md (2)
19-19: LGTM!The updated documentation clearly states that the library supports both standalone components and modules, which accurately reflects the PR changes.
167-174: Documentation accurately reflects type definitions.The type system enforces the documented requirement perfectly.
WidgetComponentTypeFactoryuses a discriminated union ofModuleOptions | StandaloneOptions, where each option forbids the other's fields via?: never. This prevents providing both loaders simultaneously and guarantees compliance with the documented "either/or" requirement.projects/dashboards-demo/src/app/widgets/charts/widget-descriptors.ts (1)
23-179: LGTM!The widget descriptors have been consistently migrated from module-based loading (
moduleName+moduleLoader) to component-based loading (componentLoader). The pattern is uniform across all seven descriptors, maintaining backward compatibility with existing descriptor properties.projects/dashboards-ng/src/components/widget-host/si-widget-host.component.spec.ts (1)
35-160: Excellent test coverage for both widget loading strategies!The test suite has been properly refactored to use a data-driven approach that validates both module-based (
TEST_WIDGET) and standalone (TEST_WIDGET_STANDALONE) widget loading. All test cases are executed for both variants, ensuring backward compatibility while validating the new standalone component support.projects/dashboards-ng/test/test-widget/test-widget.ts (1)
45-62: LGTM!The
TEST_WIDGET_STANDALONEprovides essential test infrastructure for validating standalone component loading. The descriptor correctly usescomponentLoaderinstead ofmoduleName/moduleLoader, enabling the test suite to verify both loading strategies work correctly.
|
|
Support widgets as standalone Angular components. This change maintains backward compatibility with existing module-based widgets while enabling modern standalone component architecture.
Usage:
For standalone components, use
componentLoaderin your widget descriptor:Summary by CodeRabbit
New Features
Refactor
Documentation
Tests