chore: Remove unnecessary console.error log from table.getColumn(id) implementation - #5964
Conversation
…lementation The `getColumn` method is already typed to return `Column<TData, unknown> | undefined`, indicating to callers that the result might be undefined. The current `console.error` logging when a column is not found: 1. Adds noise in development environments 2. Can mask actual issues 3. Forces users to implement workarounds (as discussed in TanStack#5505) This change removes the error logging while maintaining the expected return type, allowing consumers to handle undefined columns as already expected by the API.
console.error log from table.getColumn(id) implementationconsole.error log from table.getColumn(id) implementation
|
Is anybody available to review this? |
deepan-ashby
left a comment
There was a problem hiding this comment.
Just ran into this myself, this seems like a reasonable fix
|
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
|
I'm following this pr carefully 😅 |
|
It looks like there was some internal refactoring from the time this pull request was opened til now. It doesn't appear at first glance that the public facing API has changed, but the underlying log is still present in a slightly varied form. table/packages/table-core/src/core/columns/coreColumnsFeature.utils.ts Lines 263 to 288 in b9c46a7 So it looks like the logs will still be flooded with |
|
You shouldn't have code that can try to call invalid column ids. We even used to throw an error here |
The
getColumnmethod is already typed to returnColumn<TData, unknown> | undefined, indicating to callers that the result might be undefined. The currentconsole.errorlogging when a column is not found:This change removes the error logging while maintaining the expected return type, allowing consumers to handle undefined columns as already expected by the API.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.