fix(angular): [v9] fix conditional flexRenderComponent rendering#6219
fix(angular): [v9] fix conditional flexRenderComponent rendering#6219riccardoperra wants to merge 3 commits intoalphafrom
Conversation
📝 WalkthroughWalkthroughThis PR refactors the Angular flex-render system to add stricter generic type constraints and an equality comparison mechanism for typed content. The change detection logic in the renderer is updated to compare content kinds and equality before updating. New tests verify dynamic component switching via reactive signals. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
riccardoperra
changed the title
|
View your CI Pipeline Execution ↗ for commit df7d6bc
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This resolve a rendering issue with flexRenderDirective that doesn't re-render component while using conditional flexRenderComponent return in column configuration
riccardoperra
force-pushed
the
fix/angular-component-conditional-rendering-v9
branch
from
010323e to
94aa6eb
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We reordered the imports in plugin.tsx to place the regular ./TableDevtools import before the import type declaration, resolving the import/order ESLint violation. This aligns with the workspace-wide convention (also enforced in the angular-table changes in this PR) that requires all value imports to precede type imports.
Tip
✅ We verified this fix by re-running @tanstack/solid-table-devtools:test:eslint.
diff --git a/packages/solid-table-devtools/src/production/plugin.tsx b/packages/solid-table-devtools/src/production/plugin.tsx
index 39f442aa..94a8e230 100644
--- a/packages/solid-table-devtools/src/production/plugin.tsx
+++ b/packages/solid-table-devtools/src/production/plugin.tsx
@@ -1,6 +1,6 @@
import { createSolidPlugin } from '@tanstack/devtools-utils/solid'
-import type { TanStackDevtoolsPlugin } from '@tanstack/devtools'
import { TableDevtoolsPanel } from './TableDevtools'
+import type { TanStackDevtoolsPlugin } from '@tanstack/devtools'
const [tableDevtoolsPluginFn] = createSolidPlugin({
name: 'TanStack Table',
Or Apply changes locally with:
npx nx-cloud apply-locally TsDX-tI6j
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/angular-table/tests/flex-render/flex-render.unit.test.ts (1)
1-17: Import ordering should be fixed per ESLint rules.ESLint flags several import sorting issues:
ViewChildshould come beforesignalalphabetically (line 5)- Type-only imports (
TemplateRef,ComponentFixture) should use top-level type-only import syntaxFlexRenderDirectiveshould come beforeflexRenderComponentalphabetically (line 14)🔧 Proposed fix for import ordering
-import { - Component, - input, - signal, - ViewChild, - type TemplateRef, -} from '@angular/core' -import { TestBed, type ComponentFixture } from '@angular/core/testing' +import { + Component, + input, + signal, + ViewChild, +} from '@angular/core' +import type { TemplateRef } from '@angular/core' +import { TestBed } from '@angular/core/testing' +import type { ComponentFixture } from '@angular/core/testing' import { createColumnHelper } from '@tanstack/table-core' import { describe, expect, test } from 'vitest' import { FlexRender, + FlexRenderDirective, flexRenderComponent, - FlexRenderDirective, injectFlexRenderContext, } from '../../src'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/angular-table/tests/flex-render/flex-render.unit.test.ts` around lines 1 - 17, Reorder and adjust the imports to satisfy ESLint: place ViewChild before signal in the Angular import list, move type-only imports (TemplateRef and ComponentFixture) to top-level type-only import syntax (using `import type { ... } from '...'`), and alphabetize the named imports from '../../src' so FlexRenderDirective comes before flexRenderComponent (ensure overall groups remain logically ordered: Angular core, Angular testing, third-party, then local). Ensure you update the import statements referencing TemplateRef and ComponentFixture to use `import type` and reorganize the named imports to the required alphabetical order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/angular-table/tests/flex-render/flex-render.unit.test.ts`:
- Around line 1-17: Reorder and adjust the imports to satisfy ESLint: place
ViewChild before signal in the Angular import list, move type-only imports
(TemplateRef and ComponentFixture) to top-level type-only import syntax (using
`import type { ... } from '...'`), and alphabetize the named imports from
'../../src' so FlexRenderDirective comes before flexRenderComponent (ensure
overall groups remain logically ordered: Angular core, Angular testing,
third-party, then local). Ensure you update the import statements referencing
TemplateRef and ComponentFixture to use `import type` and reorganize the named
imports to the required alphabetical order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c787a37-e3a2-4ae2-a490-467599f6f0c3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
examples/react/row-selection/package.jsonpackages/angular-table/src/flex-render/renderer.tspackages/angular-table/src/flex-render/view.tspackages/angular-table/tests/flex-render/flex-render-table.test.tspackages/angular-table/tests/flex-render/flex-render.unit.test.tspackages/solid-table-devtools/src/production/plugin.tsx
1 participant
#6218 for v9 branch
This resolve a rendering issue with flexRenderDirective that doesn't re-render component while using conditional flexRenderComponent return in the same cell column configuration (same cell reference in template, so it's a case where you are not updating table state but relies on external data outside of table scope)
Example:
Results
before: no-rerender since we were checking only A and B content type (which is always flexRenderComponent)
after: A will be destroyed and B will be created (since component type class is different)
Summary by CodeRabbit
Refactor
Tests
Chores