-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat(angular): add support to bindings and directives in FlexRender #6152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds creation-time Angular Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant flexRenderComponent
participant Factory
participant AngularCreateComponent
participant RenderedComponent
Caller->>flexRenderComponent: request component config (component, bindings, directives, injector)
flexRenderComponent->>Factory: map config -> create options (injector, directives, bindings)
Factory->>AngularCreateComponent: createComponent(options)
AngularCreateComponent->>RenderedComponent: instantiate with directives/bindings applied
RenderedComponent-->>Caller: mounted instance (via FlexRenderComponentInstance)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
View your CI Pipeline Execution ↗ for commit 78ee0ef
☁️ Nx Cloud last updated this comment at |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/angular-table/src/flex-render/flexRenderComponent.ts`:
- Around line 33-51: Typo in the JSDoc example import — change the package name
in the example import from '@tantack/angular-table' to the correct
'@tanstack/angular-table' in the comment block near the flexRenderComponent
example so the documented import (flexRenderComponent) matches the actual
package name.
- Around line 65-83: The example import path contains a typo: update the import
in the JSDoc example for flexRenderComponent from "@tantack/angular-table" to
the correct "@tanstack/angular-table" so the documentation matches the actual
package name; locate the JSDoc block near the flexRenderComponent declaration
and change the string in the import statement.
🧹 Nitpick comments (1)
packages/angular-table/src/flex-render/flexRenderComponent.ts (1)
86-107: Consider adding a warning about mutually exclusive options.The JSDoc mentions that
bindingsshouldn't be used together withinputs/outputs, but there's no runtime validation. Consider whether a development-mode warning would be helpful if users accidentally combine both approaches.💡 Optional: Add dev-mode warning
export function flexRenderComponent<TComponent = any>( component: Type<TComponent>, options?: FlexRenderOptions<Inputs<TComponent>, Outputs<TComponent>>, ): FlexRenderComponent<TComponent> { const { inputs, injector, outputs, directives, bindings } = options ?? {} if (typeof ngDevMode === 'undefined' || ngDevMode) { if (bindings?.length && (inputs || outputs)) { console.warn( '[`@tanstack/angular-table`] Using `bindings` together with `inputs`/`outputs` is not recommended. ' + 'Choose one approach for consistency.' ) } } return new FlexRenderComponentInstance(/* ... */) }
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/angular-table/src/flex-render/flexRenderComponent.ts`:
- Around line 245-251: The interface declares mutable properties "bindings?:
CreateComponentBindings" and "directives?: CreateComponentDirectives" but the
implementation type FlexRenderComponentInstance marks them as readonly; update
the interface to declare both properties as readonly (e.g., "readonly bindings?:
CreateComponentBindings" and "readonly directives?: CreateComponentDirectives")
so the interface matches the implementation and the TypeScript "implements"
check passes.
- Around line 88-101: The JSDoc links reference the non-existent option
"FlexRenderOptions#binding" — update both occurrences that appear near the
inputs?: TInputs and the outputs documentation (the paragraph referencing
OutputEmitterRef#subscribe) to point to the correct option name
"FlexRenderOptions#bindings" so the {`@link` ...} tags read {`@link`
FlexRenderOptions#bindings}.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
afe86ba to
267c718
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/angular-table/src/flex-render/flexRenderComponent.ts`:
- Around line 123-124: The JSDoc comment for the helper uses square-bracket tag
syntax "[`@link` FlexRenderComponent]" which won't render; update the JSDoc to use
the correct inline tag syntax "{`@link` FlexRenderComponent}" in the comment above
the helper function that creates a FlexRenderComponent instance (the comment
referencing FlexRenderComponent in flexRenderComponent.ts) so IDE/tooling will
render a clickable link.
♻️ Duplicate comments (1)
packages/angular-table/src/flex-render/flexRenderComponent.ts (1)
245-251: Addreadonlymodifier for consistency.The
bindingsanddirectivesproperties lack thereadonlymodifier, unlike all other properties in this interface (lines 209-239). The implementing class (FlexRenderComponentInstance) declares these asreadonlyat lines 271-272. For API consistency, align the interface.📝 Proposed fix
- bindings?: CreateComponentBindings + readonly bindings?: CreateComponentBindings /** * Directives that should be applied to the component. * * `@see` {FlexRenderOptions#directives} */ - directives?: CreateComponentDirectives + readonly directives?: CreateComponentDirectives
| /** | ||
| * Helper function to create a [@link FlexRenderComponent] instance, with better type-safety. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix JSDoc link syntax.
JSDoc inline tags use curly braces, not square brackets. The current syntax won't render as a clickable link in IDE tooltips or generated documentation.
📝 Proposed fix
-/**
- * Helper function to create a [`@link` FlexRenderComponent] instance, with better type-safety.
+/**
+ * Helper function to create a {`@link` FlexRenderComponent} instance, with better type-safety.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Helper function to create a [@link FlexRenderComponent] instance, with better type-safety. | |
| /** | |
| * Helper function to create a {`@link` FlexRenderComponent} instance, with better type-safety. |
🤖 Prompt for AI Agents
In `@packages/angular-table/src/flex-render/flexRenderComponent.ts` around lines
123 - 124, The JSDoc comment for the helper uses square-bracket tag syntax
"[`@link` FlexRenderComponent]" which won't render; update the JSDoc to use the
correct inline tag syntax "{`@link` FlexRenderComponent}" in the comment above the
helper function that creates a FlexRenderComponent instance (the comment
referencing FlexRenderComponent in flexRenderComponent.ts) so IDE/tooling will
render a clickable link.
The following PR will update
flexRenderComponenthelper in order to passbindingsanddirectivesoptions into theviewContainerRef.createComponentcall.Angular 20 has introduced those new options to set inputs in a more convenient way. It also allows to set directives at component creation time.
https://angular.dev/guide/components/programmatic-rendering#binding-inputs-outputs-and-setting-host-directives-at-creation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.