Skip to content

fix(cdk/table): show a real error message instead of a crash when a table column is missing #33698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arturovt wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −4
Open

fix(cdk/table): show a real error message instead of a crash when a table column is missing #33698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/cdk/table/table.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,19 @@ describe('CdkTable', () => {
);
});

it('should throw a descriptive error (not a raw TypeError) for an unknown column in production mode', () => {
const originalNgDevMode = (globalThis as any).ngDevMode;
(globalThis as any).ngDevMode = false;

try {
expect(() =>
TestBed.createComponent(MissingColumnDefCdkTableApp).detectChanges(),
).toThrowError(getTableUnknownColumnError('column_a').message);
} finally {
(globalThis as any).ngDevMode = originalNgDevMode;
}
});

it('should pick up columns that are indirect descendants', () => {
expect(() =>
TestBed.createComponent(TableWithIndirectDescendantDefs).detectChanges(),
Expand Down
8 changes: 4 additions & 4 deletions src/cdk/table/table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1287,10 +1287,10 @@ export class CdkTable<T>
private _addStickyColumnStyles(rows: HTMLElement[], rowDef: BaseRowDef) {
const columnDefs = Array.from(rowDef?.columns || []).map(columnName => {
const columnDef = this._columnDefsByName.get(columnName);
if (!columnDef && (typeof ngDevMode === 'undefined' || ngDevMode)) {

@crisbeto crisbeto Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent here was that the columns likely aren't changing between dev and production so we don't need to do this assertion in production.

@arturovt arturovt Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah totally get that, and agree we don't want an assertion running on
every render just for something that "shouldn't" happen.

but fwiw this literally hit us in prod and we never once saw it in dev,
so the columns-dont-change-between-envs assumption kinda broke down for
us here lol. and when it does happen the error you get is just "cannot
read properties of undefined (reading headerCell)" with a stack that's
100% internal cdk frames - no column name, nothing. took us a while to
even figure out which table/column was the problem.

what if instead of bringing back the assertion we just wrap the actual
dereference in a try/catch and only build the nicer message if it
throws? like:

  try {
    return column.headerCell.template;
  } catch (error) {
    throw new error with some code (columnId);
  }

afaik try/catch is basically free in v8 as long as it doesn't actually
throw, so this shouldn't add the per-render cost you're worried about -
it only does anything in the case that was already about to blow up
anyway. just makes the blow up say something useful instead of a bare
TypeError.

happy to gate it behind prod-only too if you want dev to keep the exact
message it has now, just seemed silly to keep the error a total mystery
for anyone hitting in apps.

@arturovt arturovt Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is how it looks in prod which isn't very friendly:

image

if (!columnDef) {
throw getTableUnknownColumnError(columnName);
}
return columnDef!;
return columnDef;
});
const stickyStartStates = columnDefs.map(columnDef => columnDef.sticky);
const stickyEndStates = columnDefs.map(columnDef => columnDef.stickyEnd);
Expand Down Expand Up @@ -1415,11 +1415,11 @@ export class CdkTable<T>
return Array.from(rowDef.columns, columnId => {
const column = this._columnDefsByName.get(columnId);

if (!column && (typeof ngDevMode === 'undefined' || ngDevMode)) {
if (!column) {
throw getTableUnknownColumnError(columnId);
}

return rowDef.extractCellTemplate(column!);
return rowDef.extractCellTemplate(column);
});
}

Expand Down
Loading