feat(angular): Add angular table composability #6144
Conversation
|
📝 WalkthroughWalkthroughAdds a signal-driven FlexRenderDirective and helper directives (FlexRenderCell, TanStackTable/TanStackTableHeader/TanStackTableCell), a createTableHook API for pre-bound table/component wiring, removes the public enableExperimentalReactivity flag, introduces memo metadata, many Angular example projects/components, and corresponding test updates. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Component
participant Hook as createTableHook / injectAppTable
participant DI as TanStackTable Token (DI)
participant Directive as FlexRenderDirective / FlexRenderCell
participant View as Signal-driven View
App->>Hook: call injectAppTable({ columns, data, components })
Hook->>Hook: register features, row models, component mappings
Hook-->>App: return table signal instance
App->>DI: provide table signal via TanStackTable directive
Directive->>DI: injectTableContext() / injectTableCellContext()
DI-->>Directive: supply table/header/cell signal
Directive->>View: create and mount signal-driven view (effect)
View-->>Directive: on state change -> re-render via signals
Directive->>View: unmount on destroy
sequenceDiagram
participant Table as Table instance
participant Subscribe as table.Subscribe(selector)
participant Signal as Selected Signal
participant Component as UI component
Component->>Table: call table.Subscribe({ selector })
Table->>Subscribe: create selected signal
Subscribe-->>Component: return Signal
Table->>Signal: update on state change
Signal-->>Component: component re-evaluates computed/display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 92f0d47 ☁️ Nx Cloud last updated this comment at |
…y in table subscription
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
examples/angular/row-selection/src/app/selection-column.component.ts (1)
48-54: Inconsistent generic constraint and unused properties.Two issues identified:
Inconsistent generic constraint:
TableHeadSelectionComponentuses<T extends RowData>butTableRowSelectionComponentstill uses<T>. Apply the same constraint for consistency.Unused properties: The new
cellandrowproperties are defined but not used in the template. The template still referencescontext.row. Either remove the unused properties or update the template to use the new pattern.Option A: Remove unused properties if not needed
-export class TableRowSelectionComponent<T> { - readonly cell = injectTableCellContext() - readonly row = computed(() => this.cell().row) +export class TableRowSelectionComponent<T extends RowData> { context = // @ts-expect-error TODO: Should fix types injectFlexRenderContext<CellContext<{ rowSelectionFeature: {} }, unknown>>() }Option B: Use the new properties and remove context (preferred if aligning with new pattern)
-export class TableRowSelectionComponent<T> { +export class TableRowSelectionComponent<T extends RowData> { readonly cell = injectTableCellContext() readonly row = computed(() => this.cell().row) - context = - // @ts-expect-error TODO: Should fix types - injectFlexRenderContext<CellContext<{ rowSelectionFeature: {} }, unknown>>() }And update the template to use
row()instead ofcontext.row:template: ` <input type="checkbox" - [checked]="context.row.getIsSelected()" - (change)="context.row.getToggleSelectedHandler()($event)" + [checked]="row().getIsSelected()" + (change)="row().getToggleSelectedHandler()($event)" /> `,packages/angular-table/src/flexRender.ts (1)
236-242: Verify bitwise logic for view recreation check.The expression
FlexRenderFlags.ContentChanged & FlexRenderFlags.ViewFirstRenderperforms a bitwise AND between the two flag constants, not betweenthis.renderFlagsand the flags. If the intent is to check whether either flag is set, this should use OR (|) and includethis.renderFlags:🐛 Proposed fix (if intent is OR)
#shouldRecreateEntireView() { return ( this.renderFlags & - FlexRenderFlags.ContentChanged & - FlexRenderFlags.ViewFirstRender + (FlexRenderFlags.ContentChanged | FlexRenderFlags.ViewFirstRender) ) }If the intent is to check that both flags are set simultaneously, the current logic is also incorrect and would need:
return (this.renderFlags & FlexRenderFlags.ContentChanged) && (this.renderFlags & FlexRenderFlags.ViewFirstRender)packages/angular-table/src/angularReactivityFeature.ts (3)
109-121: Bug: Copy-paste error - usescellinstead ofcolumn.
assignColumnPrototypeincorrectly referencestable.options.reactivity?.cellinstead oftable.options.reactivity?.column:🐛 Proposed fix
assignColumnPrototype: (prototype, table) => { if (table.options.reactivity?.column === false) { return } markReactive(prototype) setReactiveProps(table.get, prototype, { skipProperty: getUserSkipPropertyFn( - table.options.reactivity?.cell, + table.options.reactivity?.column, skipBaseProperties, ), overridePrototype: true, }) },
123-135: Bug: Copy-paste error - usescellinstead ofheader.
assignHeaderPrototypeincorrectly referencestable.options.reactivity?.cellinstead oftable.options.reactivity?.header:🐛 Proposed fix
assignHeaderPrototype: (prototype, table) => { if (table.options.reactivity?.header === false) { return } markReactive(prototype) setReactiveProps(table.get, prototype, { skipProperty: getUserSkipPropertyFn( - table.options.reactivity?.cell, + table.options.reactivity?.header, skipBaseProperties, ), overridePrototype: true, }) },
137-149: Bug: Copy-paste error - usescellinstead ofrow.
assignRowPrototypeincorrectly referencestable.options.reactivity?.cellinstead oftable.options.reactivity?.row:🐛 Proposed fix
assignRowPrototype: (prototype, table) => { if (table.options.reactivity?.row === false) { return } markReactive(prototype) setReactiveProps(table.get, prototype, { skipProperty: getUserSkipPropertyFn( - table.options.reactivity?.cell, + table.options.reactivity?.row, skipBaseProperties, ), overridePrototype: true, }) },packages/angular-table/src/flex-render/flags.ts (1)
5-34: Update outdated comment referencing removed flag.The code consolidation of
DirtyCheckandDirtySignalinto a singleDirtyflag is properly implemented—no actual code references the removed flags. However, the comment atpackages/angular-table/src/flexRender.ts:218still mentions the oldDirtySignalflag. Update it to reference the newDirtyflag for accuracy:- // to react to changes. If the current fn uses signals, we will set the DirtySignal flag + // to react to changes. If the current fn uses signals, we will set the Dirty flagpackages/angular-table/tests/injectTable.test.ts (1)
20-49: ThewhenRenderingDone()promise should be awaited.
fixture.whenRenderingDone()returns a Promise that should be awaited to ensure the test waits for rendering to complete. Withoutawait, the test may complete before rendering finishes, potentially causing flaky tests.🔧 Proposed fix
- test('should support required signal inputs', () => { + test('should support required signal inputs', async () => { @Component({ selector: 'app-table', template: ``, standalone: true, changeDetection: ChangeDetectionStrategy.OnPush, }) class TableComponent { data = input.required<Array<any>>() table = injectTable(() => ({ data: this.data(), _features: stockFeatures, columns: [], })) } @Component({ selector: 'app-root', imports: [TableComponent], template: ` <app-table [data]="[]" /> `, changeDetection: ChangeDetectionStrategy.OnPush, }) class RootComponent {} const fixture = TestBed.createComponent(RootComponent) fixture.detectChanges() - fixture.whenRenderingDone() + await fixture.whenRenderingDone() })
🤖 Fix all issues with AI agents
In @examples/angular/composable-tables/src/app/components/cell-components.ts:
- Around line 59-60: The issue is duplicate Angular component selectors: both
ProgressCell and RowActionsCell use selector 'table-progress-cell', causing a
conflict; update the RowActionsCell @Component selector to a unique value (e.g.,
'table-row-actions-cell') and then update any template references/usages of the
old selector to the new selector so Angular can distinguish the components;
ensure the identifier referenced in the component decorator is changed (the
@Component selector in RowActionsCell) and verify imports/consumers reflect the
new selector.
- Line 24: The template currently calls cell.getValue().toLocaleString() which
will throw if getValue() returns null/undefined; change the template to guard
the value (e.g., use optional chaining and a fallback like
cell.getValue()?.toLocaleString() || '' or compute a safe string in the
component) so that null/undefined values render safely; update the template
expression referencing cell.getValue() and, if needed, add a helper method in
the component to format values before binding.
- Around line 91-105: The PriceCell double-formats the value: the price computed
currently returns a formatted string via toLocaleString while the template
applies the CurrencyPipe; change the computed `price` (in class PriceCell) to
return the raw numeric value (e.g., `() => this.cell().getValue()`) and keep the
`| currency` pipe in the template (remove the toLocaleString formatting),
ensuring `CurrencyPipe` remains in `imports`; alternatively, if you prefer
preformatted strings, remove the `| currency` pipe and `CurrencyPipe` from
`imports` and keep the existing `toLocaleString` in `price`.
In @examples/angular/composable-tables/src/app/components/header-components.ts:
- Around line 26-36: Fix the typo in the FooterColumnId component's host
attribute: change the host key 'tantack-footer-column-id' to the correct
'tanstack-footer-column-id' in the @Component decorator for class FooterColumnId
so the host attribute matches the intended "tanstack" prefix.
- Around line 38-58: The host attribute in FooterSum has a typo and a potential
reactivity hole: change the host key value from "tantack-footer-sum" to
"tanstack-footer-sum", and ensure the computed that builds sum properly depends
on the column id if it can change by referencing it reactively (e.g. capture
this.header().column.id into the computed or use a reactive getter) so that the
sum computed (sum) recalculates when the column id changes; update FooterSum to
reference the corrected host attribute and make the column id a reactive
dependency inside the sum computed.
In @examples/angular/composable-tables/src/app/components/table-components.ts:
- Around line 4-17: The @Component decorator for the table toolbar component is
missing a required selector; add a unique kebab-case selector (e.g.,
"app-table-toolbar") to the @Component metadata so the component can be used in
templates, update any template usages to match that selector, and ensure the
symbol that defines the component (the class tied to this @Component in
table-components.ts) has the selector present in its decorator.
- Around line 24-26: The constructor in the toolbar component is invoking
this.table().resetColumnFilters(), which causes filters to be cleared every time
the component is instantiated; remove that call from the constructor to avoid
the unintended side effect and instead only reset filters via an explicit method
or lifecycle hook if initialization is truly required. Locate the constructor in
table-components.ts and delete the this.table().resetColumnFilters() invocation
(or move it into a clearly named method like resetFilters() or an intentional
init routine that callers invoke).
In @examples/angular/composable-tables/src/app/makeData.ts:
- Around line 45-77: The file defines an unused Product type and the
makeProductData function (symbols: Product, makeProductData, newProduct) which
are not used anywhere in this example; remove the newProduct function, the
Product type and the makeProductData export, or if you intend to keep them for
future use, add a clear comment above newProduct/makeProductData explaining they
are intentionally retained as example helpers for generating product fixtures
and why they are unused now. Ensure any removed exports are also deleted from
the module surface so there are no unused symbol exports.
In @examples/angular/composable-tables/src/app/table.ts:
- Around line 78-80: The default getRowId function (getRowId) used in the table
options assumes a row.id field but Person (from makeData.ts) has no id, causing
undefined row IDs when injectAppTable is called from app.component.ts; update
the default getRowId to safely handle missing ids (for example return row.id ??
index) or make getRowId required so callers must provide one—modify the table
options where getRowId is defined to use a fallback based on the row index (or
add a guard) and ensure app.component.ts either supplies an explicit getRowId or
relies on the new safe default.
In @examples/angular/composable-tables/src/index.html:
- Line 5: The page title is incorrect; update the HTML <title> element in
index.html (currently "Basic") to reflect the example name (e.g., "Composable
Tables" or "composable-tables") so the document title matches the
composable-tables example.
- Line 9: Update the Twind shim import so builds are reproducible by pinning the
CDN version: replace the unpinned script src
"https://cdn.skypack.dev/twind/shim" with the versioned URL
"https://cdn.skypack.dev/[email protected]/shim" in the HTML script tag that
currently loads the Twind shim.
In @packages/angular-table/src/flexRender.ts:
- Line 220: The current condition checks the linkedSignal function object
(this.content) instead of its value, so replace the check in the if statement
that reads `if (!this.#currentEffectRef && typeof this.content === 'function')`
with a check that evaluates the signal's value (e.g., call the signal and test
the returned value: obtain const value = this.content?.(); then use typeof value
=== 'function' or value !== undefined as appropriate) so the branch runs based
on the signal's current value rather than the signal function itself.
In @packages/angular-table/src/helpers/table.ts:
- Around line 5-7: The injection token description for TanStackTableToken is
mislabeled as "HeaderContext"; update the description string used when creating
TanStackTableToken to "[TanStack Table] TableContext" so it correctly reflects
the TanStackTableContext['table'] purpose; locate the token declaration
(TanStackTableToken and TanStackTableContext) and replace the quoted description
accordingly.
In @packages/angular-table/src/lazySignalInitializer.ts:
- Line 9: The constant addedPropsDuringInitialization is declared but never
used; either remove this dead variable or implement property-tracking by adding
a set trap to the Proxy that populates addedPropsDuringInitialization when new
properties are added during initialization. Locate the Proxy creation in
lazySignalInitializer.ts, add a set(target, prop, value) { if (!(prop in
target)) addedPropsDuringInitialization[prop as string] = true; target[prop] =
value; return true; } (or remove the const entirely if tracking is not required)
and ensure any downstream code that expected addedPropsDuringInitialization is
updated accordingly.
🧹 Nitpick comments (36)
examples/angular/composable-tables/.editorconfig (1)
1-16: LGTM! Standard EditorConfig for Angular projects.The configuration follows EditorConfig conventions and includes appropriate settings for an Angular/TypeScript project. The Markdown exception for
trim_trailing_whitespace = falseis correct, as trailing spaces are significant in Markdown.✨ Optional: Add line-ending enforcement
Consider adding
end_of_line = lfto the global section to prevent cross-platform line-ending issues:[*] charset = utf-8 indent_style = space indent_size = 2 insert_final_newline = true trim_trailing_whitespace = true +end_of_line = lfThis ensures consistent Unix-style line endings across all development environments.
examples/angular/grouping/src/app/app.component.ts (1)
46-53: Consider typing the event parameters.The
event: anytyping works but loses type safety. For example code, this is acceptable, but in production code you might useEventand castevent.targetappropriately.Optional type improvement
- onPageInputChange(event: any): void { + onPageInputChange(event: Event): void { - const page = event.target.value ? Number(event.target.value) - 1 : 0 + const page = (event.target as HTMLInputElement).value ? Number((event.target as HTMLInputElement).value) - 1 : 0 this.table.setPageIndex(page) } - onPageSizeChange(event: any) { + onPageSizeChange(event: Event) { - this.table.setPageSize(Number(event.target.value)) + this.table.setPageSize(Number((event.target as HTMLSelectElement).value)) }examples/angular/row-selection/src/app/app.component.ts (1)
72-72: Remove unusedpropsparameter.The
propsparameter is declared but never used. For consistency with line 78 (header: () => 'Last Name'), either use a static string or remove the parameter.♻️ Suggested fix
- header: (props) => `First name`, + header: () => 'First name',Or if dynamic rendering isn't required, use a static string like line 102:
- header: (props) => `First name`, + header: 'First name',examples/angular/composable-tables/README.md (1)
1-3: Consider updating the README title and adding example-specific documentation.The title "Basic" appears to be leftover from the Angular CLI scaffold generator and doesn't reflect the "composable-tables" example purpose. Consider renaming it and adding a brief description of what this example demonstrates (e.g., the new composable table APIs,
FlexRenderCelldirectives, injectable tokens, etc.).📝 Suggested improvement
-# Basic +# Composable Tables Example + +This example demonstrates the Angular TanStack Table composable API, including: +- `FlexRenderCell`, `FlexRenderHeader`, and `FlexRenderFooter` directives +- Injectable table context tokens (`CellContextToken`, `HeaderContextToken`) +- Pre-bound table configuration with `createTableHook` This project was generated with [Angular CLI](https://github.com/angular/angular-cli) version 17.1.2.examples/angular/composable-tables/.devcontainer/devcontainer.json (1)
1-4: Consider upgrading to Node.js 20 or 22 LTS.Node.js 18 LTS reached end-of-life on April 30, 2025. For better long-term support, Node.js 22 LTS (supported until April 2027) is recommended, or Node.js 20 LTS (supported until April 2026) if you prefer to stay closer to the current stable release.
📝 Suggested update
{ "name": "Node.js", - "image": "mcr.microsoft.com/devcontainers/javascript-node:18" + "image": "mcr.microsoft.com/devcontainers/javascript-node:22" }examples/angular/composable-tables/src/styles.scss (1)
120-141: Status badges rely solely on color for differentiation.For accessibility, consider adding additional visual indicators (icons, patterns, or text prefixes) alongside color for status badges. Users with color vision deficiencies may have difficulty distinguishing between relationship/complicated/single statuses based on color alone.
This is acceptable for an example project, but worth noting for production use.
examples/angular/basic/src/app/app.component.html (3)
6-10: Variable shadowing may cause confusion.The template variable
let headershadows the outer loop variableheaderfrom line 6. The innerheadercontains the rendered content (string/HTML), while the outerheaderis the header object. This works correctly but may confuse maintainers.Consider using distinct names like
let headerContentfor clarity:Suggested naming for clarity
- <th *flexRenderHeader="header; let header"> - <div [innerHTML]="header"></div> + <th *flexRenderHeader="header; let headerContent"> + <div [innerHTML]="headerContent"></div>
19-22: Same shadowing pattern withcell.Similar to headers, the
let cellshadows the outer loop variable. For consistency, consider renaming tolet cellContent.
30-33: Shadowing pattern withfooter.Same observation applies here. Consider
let footerContentfor consistency.Overall, the new directive-based approach is cleaner and aligns well with the PR's composable tables goal.
examples/angular/column-resizing-performant/src/app/app.component.ts (1)
85-85: Remove unnecessary type assertion.The
as ColumnResizeModecast is unnecessary since'onChange'is a valid literal of theColumnResizeModetype. Type inference works correctly without it, as demonstrated in other framework examples (React, Lit) that use the same value without casting.Remove the cast
- columnResizeMode: 'onChange' as ColumnResizeMode, + columnResizeMode: 'onChange',examples/angular/composable-tables/tsconfig.json (1)
1-31: Configuration is consistent with most Angular examples, though some newer examples use the recommended "bundler" strategy.The file matches the majority of Angular examples in the repository that use
"moduleResolution": "node". However, verification shows that 2 newer examples (row-dnd, remote-data) use"moduleResolution": "bundler", which is the recommended approach for modern Angular apps with bundlers. While the current "node" configuration works correctly, consider aligning with the newer pattern for consistency with these recent examples.packages/angular-table/src/helpers/flexRenderCell.ts (2)
60-81: Consider early-return pattern for mutual exclusivity.The effect reads all three signals even when only one should be set. If multiple inputs are accidentally provided, the last truthy one silently wins. Consider an early-return pattern for clarity and to avoid redundant signal reads:
♻️ Suggested refactor
effect(() => { const cell = this.cell() + if (cell) { + this.#flexRender.content.set(cell.column.columnDef.cell) + this.#flexRender.props.set(cell.getContext()) + return + } const header = this.header() + if (header) { + this.#flexRender.content.set(header.column.columnDef.header) + this.#flexRender.props.set(header.getContext()) + return + } const footer = this.footer() - const { content, props } = this.#flexRender - - if (cell) { - content.set(cell.column.columnDef.cell) - props.set(cell.getContext()) - } - - if (header) { - content.set(header.column.columnDef.header) - props.set(header.getContext()) - } - if (footer) { - content.set(footer.column.columnDef.footer) - props.set(footer.getContext()) + this.#flexRender.content.set(footer.column.columnDef.footer) + this.#flexRender.props.set(footer.getContext()) } })
39-46: Verify directive class naming convention.The class is named
FlexRenderCellbut handles cells, headers, and footers. Consider renaming toFlexRenderContextor similar to better reflect its multi-purpose nature, or split into three separate directives for clarity.packages/angular-table/src/flexRender.ts (1)
322-355: Consider lazy proxy creation for performance.The
#getInjectormethod creates a newProxyandInjectoron every call. If called frequently during rendering, this could create unnecessary object allocations. Consider caching the injector when props reference hasn't changed.examples/angular/composable-tables/src/app/app.component.html (1)
14-21: Minor inconsistency inisPlaceholderhandling between header and footer.In the header section (line 15), the
@if (!header.isPlaceholder)wraps the entire<th>element, preventing placeholder headers from rendering. In the footer section (line 45), the@if (!footer.isPlaceholder)is placed inside the<th>, meaning placeholder footers still render an empty<th>withcolSpanapplied.If this is intentional (e.g., to preserve table column structure in the footer), consider adding a brief comment. Otherwise, consider aligning the patterns:
Option to align footer with header pattern
@for (footer of footerGroup.headers; track footer.id) { - <th [colSpan]="footer.colSpan" [tanStackTableHeader]="footer"> - @if (!footer.isPlaceholder) { + @if (!footer.isPlaceholder) { + <th [colSpan]="footer.colSpan" [tanStackTableHeader]="footer"> <ng-container *flexRenderFooter="footer; let footer"> {{ footer }} </ng-container> - } - </th> + </th> + } }Also applies to: 43-51
packages/angular-table/tests/test-utils.ts (2)
72-99: Consider adding a brief comment explaining the-1offset.The logic is correct, but the
-1ingetFnArgsLength(fn) - 1on line 95 may not be immediately obvious to future maintainers. A brief comment explaining that this accounts for the implicit table/context argument would improve clarity.💡 Suggested improvement
if (propertyName.startsWith('get')) { // Only properties with no arguments are computed const fn = testObj[propertyName] // Cannot test if is lazy computed since we return the unwrapped value + // Subtract 1 to account for the implicit table/context first argument const args = Math.max(0, getFnArgsLength(fn) - 1) return fn instanceof Function && args === 0 }
101-103: Accessing private property for testing purposes.This helper accesses
_reactiveCache, which appears to be a private/internal property. While acceptable for testing utilities, be aware this creates a coupling to internal implementation details that may break if the underlying implementation changes.examples/angular/composable-tables/src/app/components/header-components.ts (1)
1-10: Consider removing commented-out code.Commented-out React-style code provides no value in this Angular file and adds noise. If this is intended as a reference for future implementation, consider adding a TODO or tracking it in an issue instead.
examples/angular/composable-tables/src/app/app.component.ts (1)
70-79: Consider usingreadonlyfor the table property.For consistency with other properties in this class,
tableshould be markedreadonly.♻️ Suggested change
- table = injectAppTable(() => ({ + readonly table = injectAppTable(() => ({ columns: this.columns, data: this.data(), debugTable: true, // more table options }))examples/angular/composable-tables/src/app/makeData.ts (1)
22-28: Consider usingArray.fromfor cleaner implementation.This is a minor style preference - the current implementation works correctly.
♻️ Alternative implementation
-const range = (len: number) => { - const arr: Array<number> = [] - for (let i = 0; i < len; i++) { - arr.push(i) - } - return arr -} +const range = (len: number) => Array.from({ length: len }, (_, i) => i)packages/angular-table/tests/angularReactivityFeature.test.ts (2)
44-45: Shared table instance may cause test pollution.The
tableinstance is created once at module level and shared across all tests in the suite. If any test mutates table state, it could affect subsequent tests. Consider creating the table inside each test or usingbeforeEach.♻️ Consider using beforeEach for test isolation
describe('angularReactivityFeature', () => { // ... type and column definitions ... let table: ReturnType<typeof createTestTable> beforeEach(() => { table = createTestTable() }) // ... tests ... })
93-95: Variableheadersshadows the outer variable.The variable
headerson line 94 shadows theheadersvariable from line 78. While this works correctly due to scoping, consider renaming for clarity.♻️ Suggested rename
- const headers = headerGroup.headers - headers.forEach((header, cellIndex) => { + const groupHeaders = headerGroup.headers + groupHeaders.forEach((header, headerIndex) => {examples/angular/composable-tables/src/app/table.ts (1)
33-43: Remove commented-out import block.These commented-out imports add noise. Remove them or add a TODO if they're planned additions.
packages/angular-table/src/index.ts (2)
6-11: Remove commented-out exports or document deprecation.The commented-out exports (
angularReactivityFeature,lazySignalInitializer,reactivityUtils) suggest these were removed from the public API. Either remove the comments entirely or add a migration note for consumers who may have been using these.
20-20: Document theFlexRendertuple usage pattern.The
FlexRenderconstant as a tuple of[FlexRenderDirective, FlexRenderCell]is an interesting pattern for Angular imports, but its usage may not be immediately clear to consumers. Consider adding a JSDoc comment explaining how to use it.📝 Suggested documentation
+/** + * FlexRender is a convenience export combining FlexRenderDirective and FlexRenderCell. + * Use in component imports: `imports: [...FlexRender]` or `imports: FlexRender` + */ export const FlexRender = [FlexRenderDirective, FlexRenderCell] as constpackages/angular-table/tests/reactivityUtils.test.ts (1)
121-126: Inconsistent use offlushEffects()vstick().This test still uses
TestBed.flushEffects()while other tests in the same file (lines 29, 33, 37, 171, 173) have been updated to useTestBed.tick(). Consider updating for consistency.Suggested fix
- TestBed.flushEffects() + TestBed.tick() expect(mockSumBy3Fn).toHaveBeenLastCalledWith(4) expect(mockSumBy2Fn).toHaveBeenLastCalledWith(3) notifier.set(2) - TestBed.flushEffects() + TestBed.tick() expect(mockSumBy3Fn).toHaveBeenLastCalledWith(5)packages/angular-table/src/helpers/header.ts (1)
17-35: Samestandaloneconsideration ascell.ts.For consistency with
FlexRenderDirectiveand to explicitly support consumers on Angular versions before 19, consider addingstandalone: trueto the directive metadata.Suggested fix
@Directive({ selector: '[tanStackTableHeader]', exportAs: 'header', + standalone: true, providers: [packages/angular-table/src/helpers/cell.ts (1)
17-35: Addstandalone: trueto the directive decorator for consistency.The
TanStackTableCelldirective lacksstandalone: true, while theFlexRenderDirectiveinsrc/flexRender.tsexplicitly includes it. All helper directives (cell.ts,header.ts,table.ts,flexRenderCell.ts) are currently missing this configuration, which should be added for consistency across the codebase and to ensure compatibility with Angular versions prior to 19.Suggested fix
@Directive({ selector: '[tanStackTableCell]', exportAs: 'cell', + standalone: true, providers: [packages/angular-table/src/injectTable.ts (1)
117-119: Consider addingwritable: falsefor immutability clarity.Using
Object.definePropertywithout specifyingwritabledefaults tofalse, which is correct here. However, explicitly settingwritable: falsewould improve readability and make the intent clearer.Suggested change
Object.defineProperty(table, 'state', { value: injectStore(table.store, selector, { injector }), + writable: false, })packages/angular-table/src/helpers/createTableHook.ts (4)
312-318: Type safety loss withanyfor TData parameter.Both
injectTableHeaderContextandinjectTableCellContextpassanyas theTDatatype parameter, which loses type safety. Consider using a generic or keeping the type parameter open for consumers to specify.Suggested improvement
- function injectTableHeaderContext<TValue extends CellData = CellData>() { - return _injectTableHeaderContext<TFeatures, any, TValue>() + function injectTableHeaderContext< + TData extends RowData = RowData, + TValue extends CellData = CellData, + >() { + return _injectTableHeaderContext<TFeatures, TData, TValue>() } - function injectTableCellContext<TValue extends CellData = CellData>() { - return _injectTableCellContext<TFeatures, any, TValue>() + function injectTableCellContext< + TData extends RowData = RowData, + TValue extends CellData = CellData, + >() { + return _injectTableCellContext<TFeatures, TData, TValue>() }
360-368: Overly broad type cast loses type information.The return cast
as AngularTable<any, any>discards the type parameters that were carefully constructed. This could lead to type errors downstream.Suggested fix
return injectTable<TFeatures, TData, TSelected>(() => { const options = { ...defaultTableOptions, ...tableOptions(), } as TableOptions<TFeatures, TData> options._features = { ...options._features, appTableFeatures } return options - }, selector) as AngularTable<any, any> + }, selector) as AppAngularTable< + TFeatures, + TData, + TSelected, + TTableComponents, + TCellComponents, + THeaderComponents + > }
53-54: Consider a more specific return type forFlexRender.The
FlexRenderproperty on bothcellandheaderhas return typeunknown. If possible, provide a more specific type to improve developer experience.Also applies to: 73-74
376-384: Document the type-only nature of this wrapper.The comment on line 376-377 explains the runtime behavior, but a JSDoc comment on the function would help API consumers understand that this is primarily a type enhancement over the core column helper.
Suggested documentation
+ /** + * Creates a column helper with enhanced types for pre-bound components. + * At runtime, this delegates to the core column helper - the enhancement is purely at the type level. + */ function createAppColumnHelper<TData extends RowData>(): AppColumnHelper< TFeatures, TData, TCellComponents, THeaderComponents > {packages/angular-table/src/reactivityUtils.ts (1)
173-187: Cache key collision risk and repeated metadata assignment.Two concerns:
- The cache key
${self.id}${fnName}could collide ifidcontains characters that matchfnName(e.g.,id="getState"andfnName="Value"vsid="get"andfnName="StateValue").markReactiveand$internalMemoFnMetaassignment happen on every property access, not just initial creation.Suggested improvements
- self._reactiveCache ??= {} - const cached = (self._reactiveCache[`${self.id}${fnName}`] ??= computed( + const cacheKey = `${fnName}:${self.id}` + self._reactiveCache ??= {} + let cached = self._reactiveCache[cacheKey] + if (!cached) { + cached = computed( () => { notifier() return fn.apply(self) }, {}, - )) - markReactive(cached) - cached[$internalMemoFnMeta] = { - originalArgsLength, - } satisfies MemoFnMeta + ) + markReactive(cached) + cached[$internalMemoFnMeta] = { + originalArgsLength, + } satisfies MemoFnMeta + self._reactiveCache[cacheKey] = cached + } return cachedexamples/angular/composable-tables/src/app/components/cell-components.ts (2)
72-88: Consider extracting repeated row access to a computed property.The
firstNameandlastNameaccess pattern is repeated acrossview(),edit(), anddelete(). Extracting to a computed would be cleaner.Suggested refactor
export class RowActionsCell { readonly cell = injectTableCellContext<number>() + readonly fullName = computed( + () => `${this.cell().row.original.firstName} ${this.cell().row.original.lastName}` + ) + view() { - alert( - `View: ${this.cell().row.original.firstName} ${this.cell().row.original.lastName}`, - ) + alert(`View: ${this.fullName()}`) } edit() { - alert( - `Edit: ${this.cell().row.original.firstName} ${this.cell().row.original.lastName}`, - ) + alert(`Edit: ${this.fullName()}`) } delete() { - alert( - `Delete: ${this.cell().row.original.firstName} ${this.cell().row.original.lastName}`, - ) + alert(`Delete: ${this.fullName()}`) } }
7-17: Inconsistent context injection patterns.
TextCellandNumberCelluseinjectFlexRenderContext<CellContext<...>>()whileStatusCelland others useinjectTableCellContext<...>(). This inconsistency could confuse developers. Consider documenting when to use which, or standardizing on one approach.Also applies to: 31-44
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/angular/composable-tables/src/favicon.icois excluded by!**/*.icopnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (64)
examples/angular/basic/src/app/app.component.htmlexamples/angular/column-ordering/src/app/app.component.tsexamples/angular/column-pinning-sticky/src/app/app.component.tsexamples/angular/column-pinning/src/app/app.component.tsexamples/angular/column-resizing-performant/src/app/app.component.tsexamples/angular/column-visibility/src/app/app.component.tsexamples/angular/composable-tables/.devcontainer/devcontainer.jsonexamples/angular/composable-tables/.editorconfigexamples/angular/composable-tables/.gitignoreexamples/angular/composable-tables/README.mdexamples/angular/composable-tables/angular.jsonexamples/angular/composable-tables/package.jsonexamples/angular/composable-tables/src/app/app.component.htmlexamples/angular/composable-tables/src/app/app.component.tsexamples/angular/composable-tables/src/app/app.config.tsexamples/angular/composable-tables/src/app/app.routes.tsexamples/angular/composable-tables/src/app/components/cell-components.tsexamples/angular/composable-tables/src/app/components/header-components.tsexamples/angular/composable-tables/src/app/components/table-components.tsexamples/angular/composable-tables/src/app/makeData.tsexamples/angular/composable-tables/src/app/table.tsexamples/angular/composable-tables/src/assets/.gitkeepexamples/angular/composable-tables/src/index.htmlexamples/angular/composable-tables/src/main.tsexamples/angular/composable-tables/src/styles.scssexamples/angular/composable-tables/tsconfig.app.jsonexamples/angular/composable-tables/tsconfig.jsonexamples/angular/composable-tables/tsconfig.spec.jsonexamples/angular/editable/src/app/app.component.htmlexamples/angular/expanding/src/app/app.component.tsexamples/angular/filters/src/app/app.component.tsexamples/angular/grouping/src/app/app.component.tsexamples/angular/grouping/src/app/columns.tsexamples/angular/remote-data/src/app/app.component.tsexamples/angular/remote-data/src/app/app.config.server.tsexamples/angular/remote-data/src/app/app.routes.server.tsexamples/angular/remote-data/src/server.tsexamples/angular/remote-data/tsconfig.jsonexamples/angular/row-selection-signal/src/app/app.component.tsexamples/angular/row-selection/src/app/app.component.htmlexamples/angular/row-selection/src/app/app.component.tsexamples/angular/row-selection/src/app/selection-column.component.tsexamples/angular/signal-input/src/app/person-table/person-table.component.tspackages/angular-table/package.jsonpackages/angular-table/src/angularReactivityFeature.tspackages/angular-table/src/flex-render/flags.tspackages/angular-table/src/flex-render/flex-render-component-ref.tspackages/angular-table/src/flex-render/view.tspackages/angular-table/src/flexRender.tspackages/angular-table/src/helpers/cell.tspackages/angular-table/src/helpers/createTableHook.tspackages/angular-table/src/helpers/flexRenderCell.tspackages/angular-table/src/helpers/header.tspackages/angular-table/src/helpers/table.tspackages/angular-table/src/index.tspackages/angular-table/src/injectTable.tspackages/angular-table/src/lazySignalInitializer.tspackages/angular-table/src/reactivityUtils.tspackages/angular-table/tests/angularReactivityFeature.test.tspackages/angular-table/tests/flex-render/flex-render.unit.test.tspackages/angular-table/tests/injectTable.test.tspackages/angular-table/tests/reactivityUtils.test.tspackages/angular-table/tests/test-utils.tspackages/table-core/src/utils.ts
💤 Files with no reviewable changes (3)
- examples/angular/row-selection-signal/src/app/app.component.ts
- examples/angular/signal-input/src/app/person-table/person-table.component.ts
- examples/angular/filters/src/app/app.component.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array, so `subRows` is guaranteed to never be undefined.
Applied to files:
packages/angular-table/src/helpers/table.tspackages/angular-table/src/helpers/header.tspackages/angular-table/src/helpers/cell.tsexamples/angular/row-selection/src/app/selection-column.component.tspackages/angular-table/src/flexRender.tsexamples/angular/grouping/src/app/app.component.ts
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array using `subRows ?? []`, so `subRows` is guaranteed to never be undefined.
Applied to files:
packages/angular-table/src/helpers/table.tspackages/angular-table/src/helpers/cell.tsexamples/angular/row-selection/src/app/selection-column.component.tspackages/angular-table/src/flexRender.tsexamples/angular/grouping/src/app/app.component.ts
🧬 Code graph analysis (22)
examples/angular/composable-tables/src/app/app.config.ts (1)
examples/angular/composable-tables/src/app/app.routes.ts (1)
routes(3-3)
examples/angular/composable-tables/src/main.ts (1)
examples/angular/composable-tables/src/app/app.config.ts (1)
appConfig(5-7)
packages/angular-table/src/helpers/flexRenderCell.ts (3)
packages/angular-table/src/helpers/cell.ts (1)
Directive(17-35)packages/angular-table/src/helpers/header.ts (1)
Directive(17-35)packages/angular-table/src/flex-render/view.ts (2)
content(56-58)content(60-63)
examples/angular/composable-tables/src/app/components/header-components.ts (2)
examples/angular/composable-tables/src/app/app.component.ts (1)
Component(17-80)packages/angular-table/src/helpers/header.ts (1)
injectTableHeaderContext(37-43)
examples/angular/remote-data/src/app/app.config.server.ts (1)
examples/angular/remote-data/src/app/app.routes.server.ts (1)
serverRoutes(4-9)
packages/angular-table/tests/angularReactivityFeature.test.ts (4)
packages/table-core/src/types/ColumnDef.ts (1)
ColumnDef(202-209)packages/table-core/src/features/stockFeatures.ts (1)
stockFeatures(33-48)packages/angular-table/src/injectTable.ts (1)
injectTable(40-123)packages/angular-table/tests/test-utils.ts (2)
testShouldBeComputedProperty(72-99)getFnReactiveCache(101-103)
packages/angular-table/tests/test-utils.ts (1)
packages/table-core/src/utils.ts (1)
getMemoFnMeta(76-78)
packages/angular-table/src/flex-render/flex-render-component-ref.ts (1)
packages/angular-table/src/flex-render/flex-render-component.ts (1)
FlexRenderComponent(94-119)
examples/angular/composable-tables/src/app/table.ts (6)
packages/table-core/src/helpers/tableFeatures.ts (1)
tableFeatures(14-18)packages/table-core/src/features/row-sorting/createSortedRowModel.ts (1)
createSortedRowModel(12-30)packages/table-core/src/fns/sortFns.ts (1)
sortFns(164-171)packages/table-core/src/features/column-filtering/createFilteredRowModel.ts (1)
createFilteredRowModel(22-44)packages/table-core/src/fns/filterFns.ts (1)
filterFns(358-371)packages/table-core/src/features/row-pagination/createPaginatedRowModel.ts (1)
createPaginatedRowModel(10-30)
examples/angular/composable-tables/src/app/components/cell-components.ts (1)
examples/angular/composable-tables/src/app/components/table-components.ts (1)
Component(4-27)
packages/angular-table/src/helpers/header.ts (4)
packages/table-core/src/types/Header.ts (1)
Header(23-37)packages/angular-table/src/flexRender.ts (1)
Directive(65-356)packages/angular-table/src/helpers/cell.ts (1)
Directive(17-35)packages/angular-table/src/helpers/table.ts (1)
Directive(16-33)
packages/angular-table/tests/injectTable.test.ts (2)
examples/angular/basic/src/app/app.component.ts (1)
Component(78-99)packages/vue-table/src/merge-proxy.ts (1)
keys(72-77)
packages/angular-table/src/helpers/cell.ts (6)
packages/table-core/src/types/TableFeatures.ts (1)
TableFeatures(42-43)packages/table-core/src/types/type-utils.ts (2)
RowData(5-5)CellData(7-7)packages/table-core/src/types/Cell.ts (1)
Cell(22-33)packages/angular-table/src/flexRender.ts (1)
Directive(65-356)packages/angular-table/src/helpers/header.ts (1)
Directive(17-35)packages/angular-table/src/helpers/table.ts (1)
Directive(16-33)
examples/angular/row-selection/src/app/selection-column.component.ts (3)
packages/table-core/src/types/type-utils.ts (1)
RowData(5-5)packages/table-core/src/core/headers/coreHeadersFeature.types.ts (1)
HeaderContext(30-47)packages/angular-table/src/helpers/cell.ts (1)
injectTableCellContext(37-43)
examples/angular/row-selection/src/app/app.component.ts (2)
packages/angular-table/src/index.ts (1)
FlexRender(20-20)packages/angular-table/src/flexRender.ts (1)
flexRenderComponent(293-306)
packages/angular-table/tests/reactivityUtils.test.ts (1)
packages/angular-table/src/reactivityUtils.ts (1)
toComputed(83-145)
packages/angular-table/src/helpers/createTableHook.ts (6)
packages/angular-table/src/flexRender.ts (1)
FlexRenderContent(48-56)packages/table-core/src/core/cells/coreCellsFeature.types.ts (1)
CellContext(8-19)packages/table-core/src/types/TableOptions.ts (1)
TableOptions(51-98)packages/angular-table/src/helpers/table.ts (1)
injectTableContext(35-40)packages/angular-table/src/helpers/header.ts (1)
injectTableHeaderContext(37-43)packages/angular-table/src/helpers/cell.ts (1)
injectTableCellContext(37-43)
examples/angular/column-resizing-performant/src/app/app.component.ts (2)
packages/angular-table/src/injectTable.ts (1)
injectTable(40-123)packages/table-core/src/features/column-resizing/columnResizingFeature.types.ts (1)
ColumnResizeMode(16-16)
packages/angular-table/src/flexRender.ts (7)
packages/angular-table/src/helpers/cell.ts (2)
Directive(17-35)TanStackTableCellToken(13-15)packages/angular-table/src/helpers/header.ts (2)
Directive(17-35)TanStackTableHeaderToken(5-7)packages/angular-table/src/helpers/table.ts (2)
Directive(16-33)TanStackTableToken(5-7)packages/table-core/src/types/type-utils.ts (2)
RowData(5-5)CellData(7-7)packages/table-core/src/core/cells/coreCellsFeature.types.ts (1)
CellContext(8-19)packages/table-core/src/core/headers/coreHeadersFeature.types.ts (1)
HeaderContext(30-47)packages/angular-table/src/flex-render/view.ts (4)
previousContent(52-54)content(56-58)content(60-63)FlexRenderComponentView(109-164)
examples/angular/composable-tables/src/app/components/table-components.ts (3)
examples/angular/composable-tables/src/app/app.component.ts (1)
Component(17-80)examples/angular/composable-tables/src/app/components/header-components.ts (2)
Component(26-36)Component(38-58)packages/angular-table/src/helpers/table.ts (1)
injectTableContext(35-40)
packages/angular-table/src/reactivityUtils.ts (1)
packages/table-core/src/utils.ts (3)
getMemoFnMeta(76-78)$internalMemoFnMeta(63-63)MemoFnMeta(64-64)
packages/angular-table/tests/flex-render/flex-render.unit.test.ts (1)
packages/angular-table/src/index.ts (1)
FlexRender(20-20)
examples/angular/composable-tables/src/app/components/cell-components.ts
Outdated
Show resolved
Hide resolved
examples/angular/composable-tables/src/app/components/cell-components.ts
Show resolved
Hide resolved
examples/angular/composable-tables/src/app/components/header-components.ts
Show resolved
Hide resolved
examples/angular/composable-tables/src/app/components/header-components.ts
Show resolved
Hide resolved
examples/angular/composable-tables/src/app/components/table-components.ts
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/angular-table/src/flexRender.ts (1)
239-245: Potential logic bug: Bitwise AND may not match intended behavior.The condition uses bitwise AND across three values:
this.renderFlags & FlexRenderFlags.ContentChanged & FlexRenderFlags.ViewFirstRenderThis evaluates as
(renderFlags & ContentChanged) & ViewFirstRender, which only returns truthy if the resulting bits overlap withViewFirstRender. IfContentChangedandViewFirstRenderare different bit positions (e.g.,0b01and0b10), this will always be0.Compare with
update()at line 176 which uses OR:ContentChanged | ViewFirstRender.If the intent is to check whether either flag is set, use OR:
Proposed fix
#shouldRecreateEntireView() { return ( this.renderFlags & - FlexRenderFlags.ContentChanged & - FlexRenderFlags.ViewFirstRender + (FlexRenderFlags.ContentChanged | FlexRenderFlags.ViewFirstRender) ) }
🤖 Fix all issues with AI agents
In @examples/angular/basic-app-table/package.json:
- Line 10: The package.json "lint" script invokes ESLint but the project lacks
ESLint packages in devDependencies; update package.json devDependencies to
include eslint plus the Angular ESLint packages (e.g., eslint,
@angular-eslint/eslint-plugin, @angular-eslint/eslint-plugin-template,
@angular-eslint/template-parser and any peer packages needed) and then run
npm/yarn install so the "lint" script succeeds; ensure the versions align with
the project's Angular/TypeScript versions and the existing "lint" script remains
unchanged.
- Around line 14-21: The package.json pins Angular 21 packages (e.g.,
"@angular/core", "@angular/common", etc.) while using an alpha of TanStack
Angular Table ("@tanstack/angular-table": "^9.0.0-alpha.10") which lacks
official Angular 21 support; either align versions by downgrading Angular
packages to a supported major (e.g., Angular 17) or bump the table adapter to a
stable release that documents Angular 21 compatibility, and update the
"dependencies" entries for "@angular/*" or "@tanstack/angular-table" accordingly
after validating compatibility in local testing.
In @examples/angular/basic-app-table/src/app/app.component.ts:
- Around line 116-118: The rerender method mutates the module-level defaultData
because defaultData.sort(() => -1) sorts in-place and uses an invalid
comparator; change rerender to operate on a copy and use a stable operation —
e.g. make a shallow copy of defaultData (via slice or spread) and then reverse
or sort with a proper comparator before calling this.data.set; update the
rerender function reference accordingly to avoid mutating defaultData.
In @examples/angular/basic/src/app/app.component.ts:
- Around line 5-6: The top-of-file comment incorrectly refers to the `useTable`
hook; update that comment to reference `injectTable` so it matches the actual
API used in this file (the code calls `injectTable`). Locate the comment near
the file header and change the text from "useTable" to "injectTable", keeping
the rest of the explanatory sentence intact for consistency.
In @examples/angular/composable-tables/src/app/components/cell-components.ts:
- Line 24: The template calls cell.getValue().toLocaleString() which can throw
if getValue() is null/undefined; guard the call by checking the value first (use
optional chaining and nullish coalescing or a ternary) so you only call
toLocaleString() when cell.getValue() != null and render an empty string (or
fallback) otherwise; update the template in cell-components.ts around the
template where cell.getValue() is used to use something like
cell.getValue()?.toLocaleString() ?? '' or an equivalent null check/ternary.
In @packages/angular-table/src/flexRender.ts:
- Around line 311-323: The #renderCustomComponent method uses this.injector()
directly when calling #flexRenderComponentFactory.createComponent, but it builds
a merged injector via this.#getInjector(this.injector()) (like #renderComponent
does); update #renderCustomComponent to create a local componentInjector (const
componentInjector = this.#getInjector(this.injector())) and pass that
componentInjector to createComponent instead of this.injector(), ensuring the
merged injector with context tokens is used when instantiating the component for
FlexRenderComponentView.
🧹 Nitpick comments (8)
examples/angular/composable-tables/src/app/components/header-components.ts (2)
1-24: Remove commented-out code before merging.These blocks contain dead code: React-style JSX (lines 1-10) and a commented-out Angular component (lines 15-24). If
SortIndicatoris planned for future implementation, consider tracking it via an issue instead of leaving commented code in the codebase.
26-36: Consider using a more specific component selector.Using
selector: 'span'matches any<span>element in the DOM. While the component is rendered dynamically viaflexRenderComponent(which mitigates accidental matching), a specific selector like'app-footer-column-id'or an attribute selector'[tanstackFooterColumnId]'would be more explicit and follow Angular best practices.💡 Suggested selector change
@Component({ - selector: 'span', + selector: 'app-footer-column-id', host: { 'tanstack-footer-column-id': '', class: 'footer-column-id', }, template: `{{ header().column.id }}`, })examples/angular/composable-tables/src/index.html (1)
9-9: Pin the Twind version for reproducibility.Loading
twind/shimwithout a version specifier means this example could break if Twind releases incompatible changes. For example projects that users may reference later, pinning to a specific version improves reliability.💡 Suggested improvement
- <script type="module" src="https://cdn.skypack.dev/twind/shim"></script> + <script type="module" src="https://cdn.skypack.dev/[email protected]/shim"></script>examples/angular/basic-app-table/.devcontainer/devcontainer.json (1)
1-4: Update to a current Node.js LTS version.Node.js 18 reached end-of-life on April 30, 2025. Update to Node.js 20 or 22 for continued security updates and support.
♻️ Suggested update
{ "name": "Node.js", - "image": "mcr.microsoft.com/devcontainers/javascript-node:18" + "image": "mcr.microsoft.com/devcontainers/javascript-node:20" }examples/angular/composable-tables/src/app/components/cell-components.ts (1)
69-89: Consider typing the row data for better type safety.The generic parameter
<number>ininjectTableCellContext<number>()only types the cell value. The access torow.original.firstNameandrow.original.lastNameis not type-checked. For production code, consider defining a proper row data type.Example with typed row data
// Define a type for your row data interface Person { firstName: string lastName: string // ... other fields } // Then use it in the component (if the API supports it) // Note: This depends on how injectTableCellContext is typedexamples/angular/basic-app-table/src/app/app.component.html (1)
6-11: Consider renaming the template variable to avoid shadowing.The
let headerdeclaration shadows the outer loop variableheader. While this works correctly (the inner scope takes precedence), using a distinct name likelet contentorlet headerContentwould improve readability.This same pattern appears for
cell(line 20) andfooter(line 31).Example for header
- <th *flexRenderHeader="header; let header"> - <div [innerHTML]="header"></div> + <th *flexRenderHeader="header; let content"> + <div [innerHTML]="content"></div> </th>packages/angular-table/src/flexRender.ts (2)
86-96: Consider whetherlinkedSignalis the appropriate primitive here.
linkedSignalcreates a writable signal that resets when its source changes. If the intent is simply to track the input values reactively without allowing external writes,computed()would be more appropriate and semantically clearer.The current implementation allows calling
.set()oncontent,props, andinjector, which could lead to subtle bugs if they're mutated independently of their inputs.If writability is intentional for some use case, consider documenting why.
159-171: Consider renamingdoCheckto avoid confusion with Angular'sngDoCheck.The method name
doCheckmight be mistaken for Angular's lifecycle hook. A name likecheckAndUpdateorhandleContentChangewould be clearer.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/angular/basic-app-table/src/favicon.icois excluded by!**/*.icopnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
examples/angular/basic-app-table/.devcontainer/devcontainer.jsonexamples/angular/basic-app-table/.editorconfigexamples/angular/basic-app-table/.gitignoreexamples/angular/basic-app-table/README.mdexamples/angular/basic-app-table/angular.jsonexamples/angular/basic-app-table/package.jsonexamples/angular/basic-app-table/src/app/app.component.htmlexamples/angular/basic-app-table/src/app/app.component.tsexamples/angular/basic-app-table/src/app/app.config.tsexamples/angular/basic-app-table/src/assets/.gitkeepexamples/angular/basic-app-table/src/index.htmlexamples/angular/basic-app-table/src/main.tsexamples/angular/basic-app-table/src/styles.scssexamples/angular/basic-app-table/tsconfig.app.jsonexamples/angular/basic-app-table/tsconfig.jsonexamples/angular/basic-app-table/tsconfig.spec.jsonexamples/angular/basic/src/app/app.component.tsexamples/angular/composable-tables/src/app/components/cell-components.tsexamples/angular/composable-tables/src/app/components/header-components.tsexamples/angular/composable-tables/src/index.htmlpackages/angular-table/src/flexRender.tspackages/angular-table/src/helpers/table.ts
✅ Files skipped from review due to trivial changes (4)
- examples/angular/basic-app-table/.gitignore
- examples/angular/basic-app-table/tsconfig.app.json
- examples/angular/basic-app-table/README.md
- examples/angular/basic-app-table/angular.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/angular-table/src/helpers/table.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array, so `subRows` is guaranteed to never be undefined.
Applied to files:
packages/angular-table/src/flexRender.ts
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array using `subRows ?? []`, so `subRows` is guaranteed to never be undefined.
Applied to files:
packages/angular-table/src/flexRender.ts
🧬 Code graph analysis (4)
examples/angular/composable-tables/src/app/components/header-components.ts (2)
examples/angular/composable-tables/src/app/app.component.ts (1)
Component(17-80)packages/angular-table/src/helpers/header.ts (1)
injectTableHeaderContext(37-43)
examples/angular/basic-app-table/src/main.ts (1)
examples/angular/basic-app-table/src/app/app.config.ts (1)
appConfig(3-5)
examples/angular/basic-app-table/src/app/app.component.ts (2)
examples/angular/grouping/src/app/columns.ts (1)
columns(36-84)examples/angular/basic/src/app/app.component.ts (1)
Component(86-107)
examples/angular/composable-tables/src/app/components/cell-components.ts (4)
packages/angular-table/src/flexRender.ts (1)
injectFlexRenderContext(44-44)packages/table-core/src/core/cells/coreCellsFeature.types.ts (1)
CellContext(8-19)packages/table-core/src/types/TableFeatures.ts (1)
TableFeatures(42-43)packages/angular-table/src/helpers/cell.ts (1)
injectTableCellContext(37-43)
🔇 Additional comments (29)
examples/angular/basic-app-table/.editorconfig (1)
1-16: LGTM!The EditorConfig settings are standard and appropriate for an Angular project. The configuration correctly defines UTF-8 encoding, 2-space indentation (Angular convention), single quotes for TypeScript, and proper Markdown handling.
examples/angular/basic-app-table/tsconfig.json (1)
1-31: Configuration looks good for Angular project.The TypeScript and Angular compiler options are well-configured with strict type checking enabled. The
useDefineForClassFields: falsesetting is correctly paired withexperimentalDecorators: trueto ensure proper decorator behavior in Angular.examples/angular/basic-app-table/tsconfig.spec.json (1)
1-9: LGTM!Standard Angular test configuration that correctly extends the base config and includes Jasmine types for testing.
examples/angular/basic/src/app/app.component.ts (3)
45-50: Well-structured documentation comments.The step-by-step comments (steps 3-4) clearly explain the V9 features and provide helpful guidance about
tableFeaturesand type-safe column definition alternatives. This improves the example's educational value.
95-102: Verify intentional removal ofdebugTableoption.The
debugTable: trueoption was removed and replaced with a placeholder comment. While this simplifies the example, having debug output enabled in a "basic" example could be helpful for developers learning the library, as it exposes table state in the console.Was this removal intentional, or should the debug option be retained for educational purposes?
92-107: Component implementation looks good.The component correctly uses:
- Signal-based state management for reactive data
- OnPush change detection for performance
- The new
injectTableAPI with proper feature and row model configurationThe implementation aligns well with modern Angular patterns.
examples/angular/composable-tables/src/app/components/header-components.ts (1)
38-58: LGTM!The computed signal chain is well-structured:
header → table → rows → sum. The reduce logic correctly handles non-numeric values by treating them as 0, and the template appropriately displays a dash for non-positive sums.Same selector concern applies here as noted for
FooterColumnId.examples/angular/composable-tables/src/index.html (2)
1-8: LGTM!Standard HTML5 boilerplate with proper charset, viewport meta tag, and base href configuration for an Angular SPA.
11-13: LGTM!Clean body structure with the standard Angular bootstrap element.
examples/angular/composable-tables/src/app/components/cell-components.ts (1)
7-17: LGTM on the cell rendering pattern.The use of
injectFlexRenderContextforTextCell/NumberCellandinjectTableCellContext(signal-based) for the other components is consistent with the library's API design. The host attribute bindings for CSS class injection inStatusCellandCategoryCellare a clean approach.Also applies to: 19-29, 31-44
examples/angular/basic-app-table/package.json (2)
1-12: LGTM!Package metadata is well-configured for an example project with appropriate naming, initial version, and
private: trueto prevent accidental publishing.
32-32: TypeScript 5.9.3 is compatible with Angular 21 (which requires TypeScript >= 5.9.0 and < 6.0.0). The exact version pinning ensures reproducible builds, which is a standard practice.examples/angular/basic-app-table/src/app/app.config.ts (1)
1-5: LGTM!The minimal application configuration with an empty providers array is appropriate for this basic example that doesn't require routing or additional services.
Note: The AI summary mentions
provideRouterandroutesimports, but the actual implementation has an empty providers array. The code is correct as-is for this simple example.examples/angular/basic-app-table/src/styles.scss (1)
1-32: LGTM!Clean and minimal styling appropriate for the example application. The table styles are consistent and the pagination layout is well-structured.
examples/angular/basic-app-table/src/app/app.component.html (1)
1-42: LGTM overall!The template correctly demonstrates the new composable table directives (
*flexRenderHeader,*flexRenderCell,*flexRenderFooter) with the modern Angular control flow syntax. The structure is clean and follows the patterns established in the PR.examples/angular/basic-app-table/src/main.ts (1)
1-5: LGTM!Standard Angular standalone application bootstrap with proper error handling. This correctly initializes the application using the modern
bootstrapApplicationAPI.examples/angular/basic-app-table/src/index.html (2)
1-14: LGTM otherwise!The HTML structure is correct with proper meta tags, viewport configuration, and the Angular app root element.
9-9: > Likely an incorrect or invalid review comment.examples/angular/basic-app-table/src/app/app.component.ts (2)
72-77: Verify HTML string rendering behavior.The cell and header functions return HTML strings (e.g.,
<i>${info.getValue()}</i>). In Angular with FlexRender, raw HTML strings are typically rendered as text content, not parsed HTML. This would display literal<i>value</i>instead of italicized text.If HTML rendering is intended, consider using Angular components or templates with
[innerHTML]binding. If this is intentional for demonstration purposes, a brief comment clarifying the expected output would help.
99-114: Well-structured component demonstrating the new composable table pattern.The component follows Angular best practices with
ChangeDetectionStrategy.OnPush, uses signals for reactive data, and correctly integrates the newinjectAppTablehook. The pattern of defining features/row models centrally viacreateTableHookand then using the returned helpers throughout the application is clean and promotes consistency.packages/angular-table/src/flexRender.ts (9)
58-63: LGTM!The new
FlexRenderInputContenttype correctly represents the allowed input values including the function form that can return dynamic content.
119-157: LGTM! Clean signal-driven lifecycle management.The constructor properly sets up:
- Cleanup via
DestroyRef.onDestroyfor effect and view disposal- An effect that tracks
props()andcontent()changes with appropriate flags- First-render flag management
The use of
untracked(() => this.update())correctly prevents the update call from being tracked as a dependency.
173-191: LGTM!The update method correctly prioritizes render flags: full render for content changes, props update for reference changes, and dirty check for signal-driven updates.
247-261: LGTM!The method correctly dispatches to the appropriate renderer based on content kind.
263-278: LGTM!The getter pattern for
$implicitensures the template always receives the latest context value reactively.
280-294: LGTM!The template rendering correctly injects the custom injector for context token availability.
296-309: LGTM!Correctly merges the FlexRenderComponent's injector with the context injector.
325-358: LGTM! Reactive injector pattern with dynamic providers.The proxy pattern correctly provides reactive access to context properties. The
useValue: () => proxy.tablefunction form ensures consumers get the latest value when accessed.Note: The
'in'checks (lines 332, 338, 344) evaluate against the current props snapshot. Since a new injector is created on each render cycle, this correctly reflects the current props structure.
14-14: Angular version compatibility forlinkedSignalis appropriate. The package requires Angular ≥19, wherelinkedSignalwas introduced as a developer preview and is now stable since Angular v20. The peer dependency is correctly configured.
examples/angular/composable-tables/src/app/components/cell-components.ts
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/angular-table/src/flexRender.ts (1)
239-245: Incorrect bitwise operation in#shouldRecreateEntireView.The condition uses bitwise AND (
&) across all three values, which will only be truthy when all flags are simultaneously set:this.renderFlags & FlexRenderFlags.ContentChanged & FlexRenderFlags.ViewFirstRenderBased on the method name and usage context (determining when to destroy the current effect and recreate the view), the logic should check if either
ContentChangedorViewFirstRenderis set. However, reviewing the call site at Line 199, this is invoked to decide whether to destroy the current effect—which should happen on content change but NOT on first render (since there's no prior effect to destroy).Please verify the intended logic. If the goal is to recreate the view when content changes (but not necessarily on first render), consider:
Possible fix
#shouldRecreateEntireView() { return ( - this.renderFlags & - FlexRenderFlags.ContentChanged & - FlexRenderFlags.ViewFirstRender + (this.renderFlags & FlexRenderFlags.ContentChanged) !== 0 && + (this.renderFlags & FlexRenderFlags.ViewFirstRender) === 0 ) }Or if the intent is to check either flag:
#shouldRecreateEntireView() { return ( - this.renderFlags & - FlexRenderFlags.ContentChanged & - FlexRenderFlags.ViewFirstRender + this.renderFlags & + (FlexRenderFlags.ContentChanged | FlexRenderFlags.ViewFirstRender) ) }
🤖 Fix all issues with AI agents
In @examples/angular/composable-tables/src/app/app.component.html:
- Around line 2-5: The description references a non-existent hook name
"useAppTable"; update the copy to reference the actual exported hook
"injectAppTable" used across the example (replace "useAppTable" with
"injectAppTable" in the paragraph in app.component.html) so the docs match the
implementation and avoid confusion.
In @examples/angular/composable-tables/src/app/components/table-components.ts:
- Around line 109-113: The onPageChange handler can produce a negative index
when parsing user input (e.g., "0"); update the onPageChange method to parse the
input into a numeric page, then clamp it to a valid range before calling
this.table().setPageIndex(page). Use the table API to determine the max index
(e.g., this.table().getPageCount() - 1 or this.table().getPageOptions().length -
1) and ensure the final page is Math.max(0, Math.min(parsedPage, maxIndex)) so
you never call setPageIndex with a negative or out-of-range value.
- Around line 30-32: The constructor currently calls
this.table().resetColumnFilters(), causing filters to be cleared on every
instantiation; remove that call from the constructor and either delete it
entirely or move it to a lifecycle hook (e.g., implement ngOnInit on the toolbar
component and call this.table().resetColumnFilters() there) with a clear comment
like "Intentionally reset filters on initial toolbar init" if the reset-on-init
behavior is desired; ensure you remove the constructor side-effect or replace it
with the explicit ngOnInit implementation referencing the same
this.table().resetColumnFilters() call.
In @examples/angular/composable-tables/src/app/table.ts:
- Around line 42-52: Remove the leftover commented-out blocks in table.ts:
delete the duplicate import comment block for table-level components (the
commented import lines referencing PaginationControls, RowCount, TableToolbar),
the empty comment placeholders for cell/header/footer components, and the
React-style hook name comments (e.g., references to
useHeaderContext/useFooterContext) so the file contains only active, relevant
Angular code; locate these comments in the area around the existing import
section and the header/footer comment block and remove them prior to merging.
- Line 88: The default getRowId implementation (getRowId: (row) => row.id)
assumes every row has an id and causes runtime errors for UsersTable because
Person lacks id; fix by either adding an id to the Person type, or overriding
getRowId in UsersTable to return a stable fallback (e.g., use the row index or a
unique Person field), or replace the default getRowId with a safer function that
checks for row.id and falls back to an index/other key; update the UsersTable
component (or the shared table config where getRowId is defined) to use the
chosen approach so ProductsTable can keep its override and UsersTable no longer
throws.
In @packages/angular-table/src/helpers/createTableHook.ts:
- Around line 367-373: Change the types for appHeader and appFooter in
createTableHook.ts from Cell to Header: update both the parameter types and
return casts to use Header<TFeatures, TData, any> & TCellComponents (instead of
Cell<...>), and ensure Header is imported from @tanstack/table-core; keep the
function names appHeader and appFooter intact so they match the AppAngularTable
definitions.
🧹 Nitpick comments (10)
examples/angular/composable-tables/src/app/components/products-table/products-table.html (1)
13-14: Variable shadowing may cause confusion.The template uses
_headeras the loop variable (Line 13), then creates a newheadervia@let(Line 14), and then shadows it again withlet headerin theflexRenderHeaderdirective (Line 19). While functional, this multi-level shadowing can make the template harder to follow.Consider using distinct names like
rawHeader,appHeader, andheaderValueto improve clarity.Also applies to: 19-21
examples/angular/composable-tables/src/app/components/users-table/users-table.html (1)
1-81: Consider extracting shared table template.This template is nearly identical to
products-table.html, differing only in the toolbar title. Both templates share the same structure for headers, body, footer, pagination, and row count rendering.Consider extracting a shared table template component that accepts configuration inputs (like title and refresh handler), reducing duplication and maintenance burden. This would align with the DRY principle and the composable design philosophy of this PR.
packages/angular-table/src/flexRender.ts (1)
159-171: Consider renamingdoCheckto avoid lifecycle hook confusion.The private
doCheck()method name suggests it implements Angular'sDoChecklifecycle interface, but it's a regular method called from the render effect. Consider renaming tocheckAndUpdate()orperformDirtyCheck()for clarity.packages/angular-table/src/helpers/createTableHook.ts (1)
327-333: Type safety lost withanyforTDataparameter.Both
injectTableHeaderContextandinjectTableCellContextuseanyfor theTDatatype parameter, which loses type safety for the row data type. Consider makingTDataa generic parameter of these functions or documenting whyanyis intentionally used here.Proposed improvement
- function injectTableHeaderContext<TValue extends CellData = CellData>() { - return _injectTableHeaderContext<TFeatures, any, TValue>() + function injectTableHeaderContext< + TData extends RowData = RowData, + TValue extends CellData = CellData, + >() { + return _injectTableHeaderContext<TFeatures, TData, TValue>() } - function injectTableCellContext<TValue extends CellData = CellData>() { - return _injectTableCellContext<TFeatures, any, TValue>() + function injectTableCellContext< + TData extends RowData = RowData, + TValue extends CellData = CellData, + >() { + return _injectTableCellContext<TFeatures, TData, TValue>() }examples/angular/composable-tables/src/app/components/users-table/users-table.ts (1)
29-29: Consider documenting the large initial dataset.Initializing with 5000 rows is intentional for demonstrating pagination, but a comment explaining this would help future maintainers understand it's a deliberate choice for the example, not a production pattern.
- readonly data = signal(makeData(5000)) + // Initialize with large dataset to demonstrate pagination and performance + readonly data = signal(makeData(5000))examples/angular/composable-tables/src/app/components/header-components.ts (3)
1-10: Remove commented-out code.This commented-out React-style reference implementation should be removed to keep the codebase clean. If needed for documentation, consider moving it to a separate doc or README.
52-62: Genericspanselector may cause unintended style/behavior conflicts.Using
selector: 'span'is overly generic and could match unintended elements. Consider using a more specific selector like'footer-column-id'or'[tanstack-footer-column-id]'.♻️ Suggested change
@Component({ - selector: 'span', + selector: 'footer-column-id', host: { 'tanstack-footer-column-id': '', class: 'footer-column-id', }, template: `{{ header().column.id }}`, })
64-84: Same genericspanselector issue; also consider edge case for zero sums.
Same selector concern as
FooterColumnId- use a more specific selector.The condition
sum() > 0will display '—' when the sum is exactly zero, which may be a valid computed value (e.g., all values are 0 or positive/negative values cancel out). Consider using a different check if zero should be displayed.♻️ Suggested changes
@Component({ - selector: 'span', + selector: 'footer-sum', host: { 'tanstack-footer-sum': '', class: 'footer-sum', }, - template: `{{ sum() > 0 ? sum().toLocaleString() : '—' }}`, + template: `{{ sum().toLocaleString() }}`, })Or if you want to distinguish "no numeric values" from "sum is zero":
readonly sum = computed(() => { let hasNumericValue = false const total = this.rows().reduce((acc, row) => { const value = row.getValue(this.header().column.id) if (typeof value === 'number') { hasNumericValue = true return acc + value } return acc }, 0) return hasNumericValue ? total : null })Then in template:
{{ sum() !== null ? sum()!.toLocaleString() : '—' }}packages/angular-table/src/injectTable.ts (1)
117-119: Consider making thestateproperty explicitly read-only.
Object.definePropertywithout specifyingwritabledefaults tofalse, which is correct for a read-only property. However, being explicit improves clarity and ensures the intent is preserved during future refactoring.♻️ Optional explicit property descriptor
Object.defineProperty(table, 'state', { value: injectStore(table.store, selector, { injector }), + writable: false, + configurable: false, })examples/angular/composable-tables/src/app/components/table-components.ts (1)
103-107: Inconsistent number formatting betweenpageIndexandpageCount.
pageCountusestoLocaleString()butpageIndexdoes not. For consistency, either format both or neither. Since page counts can be large, the current approach forpageCountis reasonable, but the display{{ pageIndex() + 1 }} of {{ pageCount() }}will look inconsistent for large datasets (e.g., "1 of 1,000").♻️ Optional: consistent formatting
- readonly pageCount = computed(() => - this.table().getPageCount().toLocaleString(), - ) + readonly pageCount = computed(() => this.table().getPageCount())Then in template:
{{ (pageIndex() + 1).toLocaleString() }} of {{ pageCount().toLocaleString() }}Or keep both unformatted for simplicity since page counts typically don't need locale formatting.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
examples/angular/composable-tables/src/app/app.component.htmlexamples/angular/composable-tables/src/app/app.component.tsexamples/angular/composable-tables/src/app/components/header-components.tsexamples/angular/composable-tables/src/app/components/products-table/products-table.htmlexamples/angular/composable-tables/src/app/components/products-table/products-table.tsexamples/angular/composable-tables/src/app/components/table-components.tsexamples/angular/composable-tables/src/app/components/users-table/users-table.htmlexamples/angular/composable-tables/src/app/components/users-table/users-table.tsexamples/angular/composable-tables/src/app/table.tsexamples/angular/composable-tables/src/index.htmlpackages/angular-table/src/flexRender.tspackages/angular-table/src/helpers/createTableHook.tspackages/angular-table/src/helpers/table.tspackages/angular-table/src/injectTable.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/angular/composable-tables/src/app/app.component.ts
- examples/angular/composable-tables/src/index.html
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array using `subRows ?? []`, so `subRows` is guaranteed to never be undefined.
Applied to files:
examples/angular/composable-tables/src/app/table.tspackages/angular-table/src/helpers/table.tspackages/angular-table/src/flexRender.ts
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array, so `subRows` is guaranteed to never be undefined.
Applied to files:
examples/angular/composable-tables/src/app/table.tspackages/angular-table/src/helpers/table.tspackages/angular-table/src/flexRender.ts
🧬 Code graph analysis (5)
examples/angular/composable-tables/src/app/components/header-components.ts (3)
packages/angular-table/src/helpers/header.ts (1)
injectTableHeaderContext(37-43)packages/angular-table/src/flex-render/flex-render-component.ts (1)
FlexRenderComponent(94-119)packages/angular-table/src/flexRender.ts (1)
flexRenderComponent(296-309)
examples/angular/composable-tables/src/app/components/users-table/users-table.ts (1)
examples/angular/composable-tables/src/app/components/header-components.ts (3)
Component(33-50)Component(52-62)Component(64-84)
examples/angular/composable-tables/src/app/components/products-table/products-table.ts (1)
examples/angular/composable-tables/src/app/makeData.ts (2)
Product(13-20)makeProductData(75-77)
packages/angular-table/src/helpers/createTableHook.ts (8)
packages/angular-table/src/flexRender.ts (1)
FlexRenderContent(48-56)packages/table-core/src/core/cells/coreCellsFeature.types.ts (1)
CellContext(8-19)packages/table-core/src/types/Header.ts (1)
Header(23-37)packages/angular-table/src/injectTable.ts (2)
AngularTable(22-38)injectTable(40-123)packages/table-core/src/types/TableOptions.ts (1)
TableOptions(51-98)packages/angular-table/src/helpers/table.ts (1)
injectTableContext(30-36)packages/angular-table/src/helpers/header.ts (1)
injectTableHeaderContext(37-43)packages/angular-table/src/helpers/cell.ts (1)
injectTableCellContext(37-43)
examples/angular/composable-tables/src/app/components/table-components.ts (1)
packages/angular-table/src/helpers/table.ts (1)
injectTableContext(30-36)
🔇 Additional comments (14)
examples/angular/composable-tables/src/app/components/products-table/products-table.html (2)
1-81: LGTM overall structure.The template correctly integrates TanStack table directives with Angular's control flow syntax (
@for,@if,@let). The use oftrackexpressions for performance optimization and the composition of toolbar, pagination, and row count components demonstrate good practices.
29-31: No XSS risk.SortIndicatorreturns a hardcoded HTML string containing only static emoji characters (🔼/🔽) based on sort state—no user input is interpolated.ColumnFilterreturns a component reference (FlexRenderComponent), not HTML, so the flexRender directive handles rendering safely.Likely an incorrect or invalid review comment.
examples/angular/composable-tables/src/app/components/users-table/users-table.html (1)
29-31: SameinnerHTMLconcern applies here.As noted in the products-table review, the
[innerHTML]bindings forSortIndicatorandColumnFiltershould be verified for safe content.Also applies to: 39-41
packages/angular-table/src/flexRender.ts (2)
119-156: Effect-based change tracking is well-structured.The constructor effect properly tracks
props()andcontent()signals, sets appropriate flags for changes, and triggers updates viauntracked(). The cleanup logic inDestroyRef.onDestroycorrectly handles both the effect reference and render view.
325-358: Theinoperator checks the stale Proxy target, not through the get trap.The
Proxyis created withthis.props()as the target (evaluated once at line 327), but thegettrap dynamically callsgetContext()to access the current props. However, theinoperator (lines 332, 338, 344) checks property presence directly against the stale target without invoking thegettrap. This means the presence checks ('table' in proxy, etc.) are evaluated against the props snapshot captured at Proxy creation time, not the current props.Since each
#getInjector()call is made fresh during rendering and receives the latest props reference at that moment, this is more of a synchronous quirk within each invocation rather than a cumulative staleness issue. The registered provider callbacks remain reactive via thegettrap. Still worth noting for maintainability—if the logic around provider registration ever changes, this distinction could become problematic.packages/angular-table/src/helpers/createTableHook.ts (1)
243-263: Well-designedAppAngularTabletype with helper methods.The
AppAngularTabletype cleanly extendsAngularTableand adds typed helper methods (appCell,appHeader,appFooter) that return the base types intersected with the registered components. The use ofNoInferprevents unwanted type inference widening.examples/angular/composable-tables/src/app/app.component.html (1)
7-11: LGTM - Clean component composition.The template demonstrates the composable pattern well by rendering two different table types side-by-side with a divider.
examples/angular/composable-tables/src/app/components/users-table/users-table.ts (2)
31-67: Well-structured column definitions.The column definitions cleanly demonstrate the composable pattern:
- Accessor columns use appropriate cell components (
TextCell,NumberCell,StatusCell,ProgressCell)- Footer components (
FooterColumnId,FooterSum) are appropriately matched to column types- The display column for actions shows how to add non-data columns
69-74: Table options function is reactive.Using
this.data()inside the options factory function ensures the table reacts to data changes. ThedebugTable: trueflag is appropriate for an example but should typically be disabled in production.examples/angular/composable-tables/src/app/table.ts (1)
62-115: Well-organized hook configuration.The
createTableHookcall cleanly separates concerns:
- Features and row models are centrally defined
- Components are categorized by scope (table, cell, header)
- Default options like
getRowIdare shared across all tablesThis pattern enables consistent table behavior across the application while maintaining flexibility for individual table configurations.
examples/angular/composable-tables/src/app/components/products-table/products-table.ts (1)
1-68: LGTM! Well-structured component following established patterns.The component follows the composable table pattern correctly with signal-based data, OnPush change detection, and proper column helper usage. The
onRefreshmethod correctly creates a new array reference to trigger reactivity.Minor note:
NgComponentOutletis imported but may not be used directly in this component's template (assuming it's used in the HTML template file).packages/angular-table/src/helpers/table.ts (1)
1-36: LGTM! Clean DI pattern for table context.The implementation correctly uses Angular's dependency injection to provide table context to child components:
TanStackTableTokenproperly defines the injection token- The directive's provider factory correctly exposes the input signal
injectTableContext()provides a clean API for consumersThe use of
anyin the token type is acceptable since the actual type is parameterized at the injection site.packages/angular-table/src/injectTable.ts (1)
20-47: LGTM! API improvements for signal-based state subscriptions.The changes improve the API:
- Adding
ValueEqualityFnsupport enables custom equality checks for fine-grained reactivity control- Defaulting
TSelectedtoTableState<TFeatures>provides better ergonomics for the common case- The selector default aligns with the new type default
examples/angular/composable-tables/src/app/components/table-components.ts (1)
42-52: LGTM! Clean row count component.The
RowCountcomponent correctly uses computed signals andtoLocaleString()for user-friendly number formatting.
examples/angular/composable-tables/src/app/components/table-components.ts
Show resolved
Hide resolved
examples/angular/composable-tables/src/app/components/table-components.ts
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @examples/angular/composable-tables/src/app/table.ts:
- Around line 52-64: Fix the doc typo and sync the exports with the
documentation: change the documentation line referencing
injectFlexRenderCellContext to say "cell-level typings" instead of "header-level
typings", and either remove the two doc bullets for
injectFlexRenderHeaderContext/injectFlexRenderCellContext or uncomment the
corresponding exports (injectFlexRenderHeaderContext and
injectFlexRenderCellContext) in the exported destructuring from createTableHook
so the public API matches the docs.
🧹 Nitpick comments (2)
packages/angular-table/src/helpers/createTableHook.ts (2)
52-60: Consider clarifying theFlexRenderproperty or removing if unused.The
FlexRender: () => unknownproperty on the enhanced cell type appears undocumented and its purpose is unclear. If this is a placeholder for future functionality, consider adding a doc comment. If it's unused, consider removing it to keep the API surface clean.
445-453: Loose type cast obscures type safety.The cast to
AngularTable<any, any>at line 452 discards the precise generic typing established throughout this module. While this may be necessary due to TypeScript limitations with complex mapped types, it would be safer to use the exact return type or add a comment explaining why the cast is required.💡 Suggestion
Consider adding a comment explaining the cast, or attempting a more precise cast:
return injectTable<TFeatures, TData, TSelected>(() => { const options = { ...defaultTableOptions, ...tableOptions(), } as TableOptions<TFeatures, TData> options._features = { ...options._features, appTableFeatures } return options - }, selector) as AngularTable<any, any> + }, selector) as AppAngularTable< + TFeatures, + TData, + TSelected, + TTableComponents, + TCellComponents, + THeaderComponents + >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/angular/basic/src/app/app.component.tsexamples/angular/composable-tables/src/app/app.component.htmlexamples/angular/composable-tables/src/app/table.tspackages/angular-table/src/helpers/createTableHook.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/angular/composable-tables/src/app/app.component.html
- examples/angular/basic/src/app/app.component.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array using `subRows ?? []`, so `subRows` is guaranteed to never be undefined.
Applied to files:
examples/angular/composable-tables/src/app/table.ts
📚 Learning: 2025-09-15T15:05:14.117Z
Learnt from: stevenwjy
Repo: TanStack/table PR: 6094
File: packages/table-core/src/utils/getGroupedRowModel.ts:74-78
Timestamp: 2025-09-15T15:05:14.117Z
Learning: In TanStack Table's Row interface, the `subRows` property is typed as `Row<TData>[]` (not optional) and the `createRow` function always initializes it as an empty array, so `subRows` is guaranteed to never be undefined.
Applied to files:
examples/angular/composable-tables/src/app/table.ts
🧬 Code graph analysis (1)
packages/angular-table/src/helpers/createTableHook.ts (9)
packages/table-core/src/types/Cell.ts (1)
Cell(22-33)packages/table-core/src/core/cells/coreCellsFeature.types.ts (1)
CellContext(8-19)packages/table-core/src/types/Header.ts (1)
Header(23-37)packages/table-core/src/types/TableOptions.ts (1)
TableOptions(51-98)packages/table-core/src/core/headers/coreHeadersFeature.types.ts (1)
HeaderContext(30-47)packages/table-core/src/types/TableState.ts (1)
TableState(23-65)packages/angular-table/src/helpers/table.ts (1)
injectTableContext(30-36)packages/angular-table/src/helpers/header.ts (1)
injectTableHeaderContext(37-43)packages/angular-table/src/helpers/cell.ts (1)
injectTableCellContext(37-43)
🔇 Additional comments (6)
examples/angular/composable-tables/src/app/table.ts (1)
65-107: LGTM!The hook configuration cleanly centralizes features, row models, default options, and component registrations. This pattern enables consistent table setups across the application.
packages/angular-table/src/helpers/createTableHook.ts (5)
168-238: LGTM!The
AppColumnHelpertype comprehensively mirrors the core column helper while injecting enhanced context types. The generics flow correctly from features through to cell/header contexts.
240-263: LGTM!Good use of
NoInferon component types to prevent unwanted type inference widening. The helper methods (appCell,appHeader,appFooter) provide a clean API for consumers to access typed cell/header objects.
433-443: LGTM!The
appTableFeaturesTableFeature elegantly leverages the existing feature hook system (constructTableAPIs,assignCellPrototype,assignHeaderPrototype) to inject components at the appropriate levels. This avoids runtime wrapper overhead while maintaining type safety through the augmented types.
455-469: LGTM!Clean approach — zero runtime overhead with the type cast providing enhanced context types. The comment clearly explains the rationale.
471-479: LGTM!The returned object correctly exposes all documented helpers, matching the
CreateTableHookResulttype contract.
Here's the notable changes:
TLDR
createTableHookhelper to provide a strongly typed, Angular-friendly table APIinjectAppTablenow returns an enhanced table instance with:tableComponents,cellComponents,headerComponentsappCell,appHeader,appFooter) for typed templatescreateTableHook(e.g.cell.TextCell)tanStackTable,tanStackTableCell,tanStackTableHeader,tanStackTableFooterdirectives to enable context injection in rendered componentsFlexRenderDirectivewas refactored to rely entirely on signals. No manual dirty checking anymore!*flexRenderCell,*flexRenderHeader, and*flexRenderFooterAPIsflexRenderFlexRenderCell simplified directive
You can now pass directly a
cell,headerorfooterinstance while using *flexRenderCell, *flexRenderHeader or *flexRenderFooter.*flexRenderdirective still works and can be used when you need to specifiy different props.New
createTableHookhelperAngular doesn’t support higher-order components in the same way React does, but we can still provide a strong, typed authoring experience by leveraging a few Angular-friendly patterns.
In React, context can be injected via components such as
AppCellorAppHeader, which act as typed wrappers around render logic:In Angular, the only way to provide contextual bindings to templates is through structural directives. However:
createTableHookBecause of this, directly emulating the React pattern is not feasible in Angular.
Instead of introducing new directives, we augment the table instance itself with typed helper functions.
This works because:
Object.assignis needed during renderingThis allow us to just call
*flexRenderand pass that cell in the context, like we did in <V9.As a recap:
cellComponentsheaderComponentstableComponentsColumn definitions work the same way as before. If a component class or the result of
flexRenderComponentis provided, the system renders that component with the correct table context. As a plus, thanks to the new types, you are able to just pass the components defined increateTableHooksUsing cell/header components in template
What if you need to use
header.FooterColumnIdin the template? In the react example we have something like this:In Angular, we achieve the same result by using typed helpers exposed on the table instance.
These helpers (
appCell,appHeader,appFooter) simply return casted versions of the original TanStack objects, enriched with the app-specific typings. They are cheap at runtime and do not introduce additional directives or wrappers.@for (_footer of footerGroup.headers; track _footer.id) { @let footer = table.appFooter(_footer); --> this will return an AppHeader type <!-- using flexRender --> <ng-container *flexRender="footer.FooterSum; props: footer.getContext()"/>; <!-- using angular built-in directive --> <ng-container [tanStackTableFooter]="footer"> <!-- injectable directive that provides footer --> <ng-container *ngComponentOutlet="footer.FooterSum"/> <!-- if you want to call it like before, you may want to access that context without passing inputs --> <app-footer-sum/> </ng-container> }Context injection via
tanStackTableCell / Header / FooterdirectivesIn addition to the typed helpers, this PR introduces the following injectable directives:
tanStackTableCelltanStackTableHeadertanStackTableFooterThese directives exist for one specific reason: to allow components rendered inside the table (via
ngComponentOutletor just using the component tag) to inject the correct TanStack Table context. If you use*flexRender, this is already done internally via that directive. This will act like a context in React.FlexRenderDirective rendering refactor
Since the new
injectTablewithangularReactivityFeaturewill wraps every method of row, cells etc to a cached computed, the *flexRender directive doesn't need to handle anymore the "mark as dirty" using bothngDoCheckand effects.The refactored implementation removes
ngOnChangesandngDoCheckusing a singleeffectcalled during class initialization, which is responsible to handle the entire flow:effectwithcontent(props)as a dependencyDirtyflag in order to update the component on the next`FlexRender#update`callrenderflow will start from scratch. We don't need here to handle component manual dirty checking like before because it will be automatically marked as dirty when you call a computed method inside.FlexRender new injectable tokens
While using flexRender, based on the given context, components and template and rendered with a custom injector whith those tokens as providers:
While passing a cell context:
CellContextToken->injectTableCellContext()-> returns a signal of a cell instanceWhile passing an header or footer context
HeaderContextToken->injectTableHeaderContext()-> returns a signal of an header or footer instance