Skip to content

feat: Export Configuration Overhaul & Build Stability Fixes#21

Merged
jacksonkasi1 merged 12 commits intomainfrom
dev
Feb 24, 2026
Merged

feat: Export Configuration Overhaul & Build Stability Fixes#21
jacksonkasi1 merged 12 commits intomainfrom
dev

Conversation

@jacksonkasi1
Copy link
Copy Markdown
Owner

@jacksonkasi1 jacksonkasi1 commented Feb 24, 2026

Overview

This PR overhauls the Export System for better Developer Experience and type-safety, alongside crucial fixes for monorepo build stability.

Highlights & Features

  • Simplified Export Configuration (removeHeaders):
    Replaced the inclusion-based headers array with an exclusion-based removeHeaders. This makes configuring exports for tables with many columns significantly simpler.
  • Type-Safe Autocomplete via KnownStringKeys<T>:
    Resolved an autocomplete bug where index signatures (like Record<string, unknown>) swallowed column suggestions. Autocomplete for removeHeaders and columnMapping now perfectly suggests generated *Column keys!
  • Cross-Page Export Support:
    Added queryByIds support to the adapter for fetching user selections that span across multiple paginated pages during export operations.

Fixes

  • Build Stability (Bun TSC Cache Bug): Removed the --bun flag from package tsc build scripts. When the Vite app rebuilt, Bun's tsc implementation would aggressively clear dist folders without successfully regenerating all files, leading to persistent local TS errors. Standard tsc is now enforced.

Documentation

  • Updated docs/export-config.md to reflect the removeHeaders shift and fully explain export options.

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 24, 2026

Deployment failed with the following error:

Resource is limited - try again in 11 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/jacksonkasi1s-projects?upgradeToPro=build-rate-limit

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adapters and types extended to support sorted bulk-ID queries (sortBy, sortOrder). TableCraft adapter gains idField and a queryByIds method. DataTable now uses adapter.queryByIds for cross-page selection retrieval. ExportConfig typing and defineExportConfig helper added, with docs and example.

Changes

Cohort / File(s) Summary
Type definitions & exports
packages/table/src/types.ts, packages/table/src/index.ts
Extended DataAdapter.queryByIds to accept `options?: { sortBy?: string; sortOrder?: "asc"
REST adapter
packages/table/src/auto/rest-adapter.ts
Updated RestAdapterOptions.queryByIdsFn signature to accept the new optional options (sortBy, sortOrder); adapter maps queryByIds to options.queryByIdsFn.
TableCraft adapter
packages/table/src/auto/tablecraft-adapter.ts
Added idField?: string option and implemented queryByIds(ids, sortOptions?) that constructs an IN-filter query (pageSize = ids.length), applies optional sort and dateRange metadata, and returns API data array.
Data table behavior
packages/table/src/data-table.tsx
getSelectedItems now calls adapter.queryByIds(selectedIds, { sortBy, sortOrder }) to fetch all selected items in-order, falls back to current page items on failure, and added sortBy/sortOrder to dependency list. Two exportConfig usages cast via unknown for typing.
Export runtime & toolbar
packages/table/src/export.tsx, packages/table/src/toolbar.tsx
Simplified header computation using removeHeaders, tightened internal mapping typing to Record<string,string>, and removed an explicit runtime cast when forwarding exportConfig.
Docs & example
docs/export-config.md, docs/SUMMARY.md, apps/vite-web-example/src/pages/orders-page.tsx
Added export-config documentation and SUMMARY entry; example page imports defineExportConfig and passes exportConfig into DataTable with a type-safe export config.
Package metadata
packages/table/package.json, apps/vite-web-example/package.json
packages/table version bumped (to 0.2.16); build/dev scripts simplified to tsc/tsc --watch. Example app dependency updated to workspace reference for @tablecraft/table.
Misc
packages/table/src/auto/..., packages/table/src/...
Minor formatting/whitespace adjustments across adapter and export files; no behavioral changes beyond above features.

Sequence Diagram(s)

sequenceDiagram
  participant UI as DataTable (UI)
  participant Adapter as DataAdapter (TableCraft / REST)
  participant API as Remote API

  UI->>Adapter: queryByIds(selectedIds, { sortBy, sortOrder })
  Adapter->>API: GET /items?filter=id_in(selectedIds)&pageSize=ids.length&sort=...
  API-->>Adapter: { data: [...] }
  Adapter-->>UI: return data[] (ordered per sort)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 I hop through rows with joyful cheer,

IDs gathered, sorted, tidy and clear,
Across pages I fetch each tasty bit,
Rows aligned — a tidy rabbit fit,
Hooray for exporters — carrot-shaped writ!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately covers the main themes of the changeset: export configuration overhaul (new export config types, removeHeaders, columnMapping, defineExportConfig helper) and build stability fixes (tsc script changes).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Feb 24, 2026

Reviewer's Guide

Adds a sortable queryByIds API to table adapters and updates DataTable’s cross-page export to use it so that exported rows match the current table sort order, along with minor typing and formatting tweaks.

Sequence diagram for cross page export using sortable queryByIds

sequenceDiagram
  actor User
  participant DataTable
  participant DataAdapter
  participant TableCraftAdapter
  participant Backend

  User->>DataTable: triggerExport()
  DataTable->>DataTable: computeSelectedIdsAndSort(sortBy, sortOrder)
  DataTable->>DataAdapter: queryByIds(selectedIds, { sortBy, sortOrder })
  activate DataAdapter
  DataAdapter->>TableCraftAdapter: queryByIds(selectedIds, { sortBy, sortOrder })
  activate TableCraftAdapter
  TableCraftAdapter->>TableCraftAdapter: buildQueryParams(ids, sortBy, sortOrder, idField)
  TableCraftAdapter->>Backend: GET /table?filters[id].operator=in&filters[id].value=ids&sort=sortBy&sortOrder=sortOrder
  activate Backend
  Backend-->>TableCraftAdapter: sortedRowsById
  deactivate Backend
  TableCraftAdapter-->>DataAdapter: sortedRowsById
  deactivate TableCraftAdapter
  DataAdapter-->>DataTable: sortedRowsById
  deactivate DataAdapter
  DataTable->>DataTable: useFetchedRowsForExport(sortedRowsById)
  DataTable-->>User: downloadExportedFile
Loading

Class diagram for updated table adapters and DataAdapter interface

classDiagram
  class QueryParams {
  }

  class QueryResult_T_ {
  }

  class TableMetadata {
  }

  class DataAdapter_T_ {
    <<interface>>
    +query(params QueryParams) Promise~QueryResult_T_~
    +queryByIds(ids Array~string_or_number~, options QueryByIdsOptions) Promise~Array~T~~
    +meta() Promise~TableMetadata~
    +export(format string, params QueryParams) Promise~string~
  }

  class QueryByIdsOptions {
    +sortBy string
    +sortOrder string
  }

  class TableCraftAdapterOptions_TFilters_ {
    +baseUrl string
    +table string
    +idField string
    +headers Record~string_string~
    +fetch(url string, options RequestInit) Promise~Response~
    +axios any
  }

  class RestAdapterOptions_T_ {
    +queryFn(params QueryParams) Promise~QueryResult_T_~
    +queryByIdsFn(ids Array~string_or_number~, options QueryByIdsOptions) Promise~Array~T~~
    +metaFn() Promise~TableMetadata~
  }

  class TableCraftAdapter_T_ {
    +query(params QueryParams) Promise~QueryResult_T_~
    +queryByIds(ids Array~string_or_number~, options QueryByIdsOptions) Promise~Array~T~~
    +meta() Promise~TableMetadata~
    +export(format string, params QueryParams) Promise~string~
  }

  class DataTable_T_ {
    +getSelectedItemsForExport() Array~T~
  }

  DataAdapter_T_ <|.. TableCraftAdapter_T_
  TableCraftAdapterOptions_TFilters_ --> TableCraftAdapter_T_ : configures
  RestAdapterOptions_T_ --> DataAdapter_T_ : used_to_build
  QueryByIdsOptions --> DataAdapter_T_ : parameter
  QueryByIdsOptions --> TableCraftAdapter_T_ : parameter
  DataTable_T_ --> DataAdapter_T_ : uses_queryByIds_for_export
Loading

File-Level Changes

Change Details Files
Add queryByIds with sorting support to TableCraftAdapter and wire it to backend query building.
  • Extend TableCraftAdapterOptions with optional idField to specify primary key column name
  • Implement queryByIds that builds a QueryParams object with an IN filter on the primary key, applies optional sortBy/sortOrder, and reuses buildQueryUrl/applyCustomFilters/request pipeline
  • Default idField to "id" when not provided and return early for empty ID lists
packages/table/src/auto/tablecraft-adapter.ts
Update DataAdapter and RestAdapter contracts to support sorted queryByIds calls.
  • Broaden DataAdapter.queryByIds signature to accept an optional sort options object
  • Broaden RestAdapterOptions.queryByIdsFn signature to accept the same sort options
packages/table/src/types.ts
packages/table/src/auto/rest-adapter.ts
Change DataTable cross-page selection/export to delegate ordering to queryByIds instead of merging current and fetched pages.
  • Detect selected IDs and use adapter.queryByIds with current sortBy/sortOrder to fetch all selected rows in table order
  • Fallback to returning only itemsOnPage if queryByIds is unavailable or throws
  • Include sortBy and sortOrder in export-related memo dependencies
packages/table/src/data-table.tsx
Apply minor formatting and indentation cleanups.
  • Normalize indentation in mapped type definitions and props
  • Fix stray spacing inconsistencies in tablecraft-adapter filter handling and other sections
packages/table/src/auto/tablecraft-adapter.ts
packages/table/src/types.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In TableCraftAdapter.queryByIds, consider omitting unused query params (empty search and blank dateRange) rather than sending empty strings, to avoid subtle differences in backend behavior compared to the normal query path.
  • The queryByIds implementation currently sets pageSize to ids.length; if very large selections are expected, it may be safer to either chunk requests or enforce a reasonable upper bound to avoid oversized single queries.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `TableCraftAdapter.queryByIds`, consider omitting unused query params (empty `search` and blank `dateRange`) rather than sending empty strings, to avoid subtle differences in backend behavior compared to the normal `query` path.
- The `queryByIds` implementation currently sets `pageSize` to `ids.length`; if very large selections are expected, it may be safer to either chunk requests or enforce a reasonable upper bound to avoid oversized single queries.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/table/src/auto/tablecraft-adapter.ts`:
- Around line 282-312: queryByIds currently sets pageSize = ids.length and
builds a GET URL with all IDs which can exceed backend/URL limits; change
queryByIds (and use getMetadataWithFallback) to read
metadata.capabilities.pagination.maxPageSize (use a sensible fallback if
missing), split the ids array into chunks of at most that maxPageSize, perform a
request per chunk (reusing applyCustomFilters/buildQueryUrl and the existing
request call with filters: { [idKey]: { operator: "in", value: chunk } }),
concatenate all returned result.data arrays, and if sortOptions?.sortBy is
provided perform a client-side sort on the combined results using
sortOptions.sortBy and sortOptions.sortOrder before returning. Ensure the idKey,
getMetadataWithFallback, applyCustomFilters, buildQueryUrl, and request usage
remain consistent.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 922c028 and 58e9d84.

📒 Files selected for processing (4)
  • packages/table/src/auto/rest-adapter.ts
  • packages/table/src/auto/tablecraft-adapter.ts
  • packages/table/src/data-table.tsx
  • packages/table/src/types.ts

Comment thread packages/table/src/auto/tablecraft-adapter.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/table/src/export.tsx (1)

84-100: ⚠️ Potential issue | 🟡 Minor

Merge partial columnMapping with defaults to avoid blank headers.
columnMapping is now Partial<Record<...>>, but the current logic treats it as complete; missing keys will produce undefined headers. Suggest generating a default mapping and layering user overrides on top.

✅ Suggested fix
-    const exportColumnMapping: Record<string, string> =
-      (exportConfig?.columnMapping as Record<string, string>) ||
-      (() => {
-        const mapping: Record<string, string> = {};
-        orderedColumns.forEach((col) => {
-          const headerText = col.columnDef.header as string;
-          if (headerText && typeof headerText === "string") {
-            mapping[col.id] = headerText;
-          } else {
-            mapping[col.id] = col.id
-              .split(/(?=[A-Z])|_/)
-              .map((w) => w.charAt(0).toUpperCase() + w.slice(1).toLowerCase())
-              .join(" ");
-          }
-        });
-        return mapping;
-      })();
+    const baseMapping = (() => {
+      const mapping: Record<string, string> = {};
+      orderedColumns.forEach((col) => {
+        const headerText = col.columnDef.header as string;
+        if (headerText && typeof headerText === "string") {
+          mapping[col.id] = headerText;
+        } else {
+          mapping[col.id] = col.id
+            .split(/(?=[A-Z])|_/)
+            .map((w) => w.charAt(0).toUpperCase() + w.slice(1).toLowerCase())
+            .join(" ");
+        }
+      });
+      return mapping;
+    })();
+    const exportColumnMapping: Record<string, string> = {
+      ...baseMapping,
+      ...(exportConfig?.columnMapping ?? {}),
+    };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/table/src/export.tsx` around lines 84 - 100, The exportColumnMapping
logic treats exportConfig.columnMapping as complete and can leave headers
undefined; change it to first build the default mapping from orderedColumns (the
existing mapping logic in the IIFE that uses orderedColumns,
col.columnDef.header and fallback formatting) and then merge the user-provided
mapping (exportConfig.columnMapping typed as Partial<Record<string,string>>) on
top so user values override defaults (use Object.assign or spread to combine
defaultMapping with exportConfig.columnMapping when constructing
exportColumnMapping). Ensure exportColumnMapping remains a Record<string,string>
by filling any missing keys with the defaults from orderedColumns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/table/src/export.tsx`:
- Around line 84-100: The exportColumnMapping logic treats
exportConfig.columnMapping as complete and can leave headers undefined; change
it to first build the default mapping from orderedColumns (the existing mapping
logic in the IIFE that uses orderedColumns, col.columnDef.header and fallback
formatting) and then merge the user-provided mapping (exportConfig.columnMapping
typed as Partial<Record<string,string>>) on top so user values override defaults
(use Object.assign or spread to combine defaultMapping with
exportConfig.columnMapping when constructing exportColumnMapping). Ensure
exportColumnMapping remains a Record<string,string> by filling any missing keys
with the defaults from orderedColumns.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58e9d84 and 3a3783e.

📒 Files selected for processing (8)
  • apps/vite-web-example/src/pages/orders-page.tsx
  • docs/SUMMARY.md
  • docs/export-config.md
  • packages/table/src/data-table.tsx
  • packages/table/src/export.tsx
  • packages/table/src/index.ts
  • packages/table/src/toolbar.tsx
  • packages/table/src/types.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/export-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/table/src/data-table.tsx

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 24, 2026

Deployment failed with the following error:

Resource is limited - try again in 10 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/jacksonkasi1s-projects?upgradeToPro=build-rate-limit

…ig inference

chore(release): publish @tablecraft/table@0.2.12
…ng Record<string, unknown>

chore(release): publish @tablecraft/table@0.2.14
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/vite-web-example/package.json`:
- Line 16: The dependency version for `@tablecraft/table` in the app is stale (app
lists ^0.2.11 while the workspace package is 0.2.14); regenerate the Bun
lockfile and align the dependency: run bun install (or bun generate-lockfile) at
repo root to refresh bun.lock so it resolves to 0.2.14, and if desired update
the app's package.json entry for `@tablecraft/table` to ^0.2.14 so sources and
lockfile match consistently.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between deaeee1 and 87ebcb3.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • apps/vite-web-example/package.json
  • packages/table/package.json
  • packages/table/src/types.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/table/package.json

Comment thread apps/vite-web-example/package.json Outdated
chore(release): publish @tablecraft/table@0.2.15
@vercel
Copy link
Copy Markdown

vercel bot commented Feb 24, 2026

Deployment failed with the following error:

Resource is limited - try again in 9 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/jacksonkasi1s-projects?upgradeToPro=build-rate-limit

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/table/src/export.tsx (1)

62-78: ⚠️ Potential issue | 🟠 Major

columnMapping now completely replaces auto-generated header names — likely a regression.

When exportConfig.columnMapping is provided (even a single-entry object), the || short-circuit skips the IIFE that generates human-readable headers from col.columnDef.header. Columns not in columnMapping will export with their raw accessor IDs (e.g. statusLabel, vatAmount) instead of the formatted names that TanStack Table column definitions carry.

The example in orders-page.tsx maps only 3 of ~10 exported columns; all others would silently export with raw IDs.

The fix is to seed from auto-detected headers and then overlay user overrides:

🐛 Proposed fix
-    const exportColumnMapping: Record<string, string> =
-      (exportConfig?.columnMapping as Record<string, string>) ||
-      (() => {
-        const mapping: Record<string, string> = {};
-        orderedColumns.forEach((col) => {
-          const headerText = col.columnDef.header as string;
-          if (headerText && typeof headerText === "string") {
-            mapping[col.id] = headerText;
-          } else {
-            mapping[col.id] = col.id
-              .split(/(?=[A-Z])|_/)
-              .map((w) => w.charAt(0).toUpperCase() + w.slice(1).toLowerCase())
-              .join(" ");
-          }
-        });
-        return mapping;
-      })();
+    const autoMapping: Record<string, string> = {};
+    orderedColumns.forEach((col) => {
+      const headerText = col.columnDef.header as string;
+      if (headerText && typeof headerText === "string") {
+        autoMapping[col.id] = headerText;
+      } else {
+        autoMapping[col.id] = col.id
+          .split(/(?=[A-Z])|_/)
+          .map((w) => w.charAt(0).toUpperCase() + w.slice(1).toLowerCase())
+          .join(" ");
+      }
+    });
+    const exportColumnMapping: Record<string, string> = {
+      ...autoMapping,
+      ...(exportConfig?.columnMapping as Record<string, string> ?? {}),
+    };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/table/src/export.tsx` around lines 62 - 78, The current logic sets
exportColumnMapping to exportConfig.columnMapping or else an auto-generated
mapping, which causes a regression where a partial user-provided mapping
replaces all auto-detected headers; update exportColumnMapping so you first
build the default mapping from orderedColumns (using col.columnDef.header and
col.id fallback logic) and then shallow-merge/overlay exportConfig.columnMapping
on top (so keys in exportConfig override defaults but unspecified columns keep
the generated friendly names); refer to exportColumnMapping,
exportConfig.columnMapping, orderedColumns, col.columnDef.header and col.id to
locate and implement the merge.
🧹 Nitpick comments (4)
packages/table/src/data-table.tsx (1)

648-648: Consider a top-level import alias instead of inline import() in JSX.

import("./types").ExportConfig<ExportableData> is valid TypeScript but the inline dynamic-import-style type path is unusual in JSX attribute positions and can trip up some editors and linters. Since ExportConfig is already imported at Line 24 (via DataTableProps), you can pull it in explicitly and alias it:

♻️ Proposed refactor

At the top of the file, add ExportConfig to the existing type import:

-import type { DataTableProps, ExportableData, TableContext } from "./types";
+import type { DataTableProps, ExportableData, ExportConfig, TableContext } from "./types";

Then replace the inline cast:

-          exportConfig={exportConfig as unknown as import("./types").ExportConfig<ExportableData>}
+          exportConfig={exportConfig as unknown as ExportConfig<ExportableData>}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/table/src/data-table.tsx` at line 648, Replace the inline dynamic
type cast used in the JSX prop (exportConfig={exportConfig as unknown as
import("./types").ExportConfig<ExportableData>}) with a top-level named type
import: add ExportConfig to the existing type import where DataTableProps is
imported (line with DataTableProps) and then cast exportConfig to that imported
ExportConfig<ExportableData> type in the JSX; update the JSX attribute to use
the named ExportConfig type instead of the inline import() expression so editors
and linters handle it cleanly.
apps/vite-web-example/src/pages/orders-page.tsx (1)

17-23: Number(row.total) is redundant — total is already typed as number.

OrdersRow.total: number, so the explicit Number() conversion is unnecessary.

♻️ Proposed fix
-    total: `₹${Number(row.total).toFixed(2)}`,
+    total: `₹${row.total.toFixed(2)}`,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/vite-web-example/src/pages/orders-page.tsx` around lines 17 - 23, The
transformFunction is unnecessarily wrapping row.total with Number(...) even
though OrdersRow.total is already a number; update the transformFunction to
format total using row.total.toFixed(2) (inside the template string `₹${...}`)
and remove the Number(...) conversion so the code reads `total:
\`₹${row.total.toFixed(2)}\``; keep the existing createdAt handling and ensure
you reference transformFunction and OrdersRow.total when making the change.
packages/table/package.json (1)

21-22: typecheck script is inconsistent with the updated build/dev scripts.

build and dev now use plain tsc, but typecheck at Line 24 still uses bun run --bun tsc --noEmit. Align for consistency:

♻️ Proposed fix
-    "typecheck": "bun run --bun tsc --noEmit"
+    "typecheck": "tsc --noEmit"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/table/package.json` around lines 21 - 22, The package.json typecheck
script is inconsistent with build/dev; change the "typecheck" script to use the
same plain tsc invocation as "build" and "dev" but with --noEmit (e.g., replace
the current "bun run --bun tsc --noEmit" value with "tsc --noEmit") so all
scripts consistently call tsc; update the "typecheck" entry in package.json
accordingly.
packages/table/src/export.tsx (1)

80-84: Use a local variable to avoid the non-null assertion.

exportConfig.columnWidths! is safe because we're inside the truthy branch of exportConfig?.columnWidths, but a local variable is clearer:

♻️ Proposed refactor
-    const exportColumnWidths = exportConfig?.columnWidths
-      ? exportHeaders.map(
-        (_, i) => exportConfig.columnWidths![i] || { wch: 15 }
-      )
-      : exportHeaders.map(() => ({ wch: 15 }));
+    const userWidths = exportConfig?.columnWidths;
+    const exportColumnWidths = userWidths
+      ? exportHeaders.map((_, i) => userWidths[i] || { wch: 15 })
+      : exportHeaders.map(() => ({ wch: 15 }));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/table/src/export.tsx` around lines 80 - 84, The code uses a non-null
assertion exportConfig.columnWidths! inside the exportColumnWidths computation;
extract exportConfig.columnWidths into a local const (e.g., const columnWidths =
exportConfig?.columnWidths) within the same scope and then use columnWidths[i]
when mapping exportHeaders to avoid the bang and make the intent clearer; update
the conditional to check columnWidths truthiness (or reuse the existing
exportConfig?.columnWidths check) and fall back to { wch: 15 } when
columnWidths[i] is undefined, keeping the variable name exportColumnWidths and
referencing exportHeaders as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/vite-web-example/request.json`:
- Around line 1-44: This JSON payload (the accidental AI API request containing
top-level keys "contents", "generationConfig", and "safetySettings") should be
removed from the commit and repository; delete the file, remove it from git
history/index (e.g., git rm or git rm --cached and commit) so it isn’t picked up
by CI, and add an appropriate ignore rule to .gitignore to prevent future
accidental commits of similar test payloads.

---

Outside diff comments:
In `@packages/table/src/export.tsx`:
- Around line 62-78: The current logic sets exportColumnMapping to
exportConfig.columnMapping or else an auto-generated mapping, which causes a
regression where a partial user-provided mapping replaces all auto-detected
headers; update exportColumnMapping so you first build the default mapping from
orderedColumns (using col.columnDef.header and col.id fallback logic) and then
shallow-merge/overlay exportConfig.columnMapping on top (so keys in exportConfig
override defaults but unspecified columns keep the generated friendly names);
refer to exportColumnMapping, exportConfig.columnMapping, orderedColumns,
col.columnDef.header and col.id to locate and implement the merge.

---

Nitpick comments:
In `@apps/vite-web-example/src/pages/orders-page.tsx`:
- Around line 17-23: The transformFunction is unnecessarily wrapping row.total
with Number(...) even though OrdersRow.total is already a number; update the
transformFunction to format total using row.total.toFixed(2) (inside the
template string `₹${...}`) and remove the Number(...) conversion so the code
reads `total: \`₹${row.total.toFixed(2)}\``; keep the existing createdAt
handling and ensure you reference transformFunction and OrdersRow.total when
making the change.

In `@packages/table/package.json`:
- Around line 21-22: The package.json typecheck script is inconsistent with
build/dev; change the "typecheck" script to use the same plain tsc invocation as
"build" and "dev" but with --noEmit (e.g., replace the current "bun run --bun
tsc --noEmit" value with "tsc --noEmit") so all scripts consistently call tsc;
update the "typecheck" entry in package.json accordingly.

In `@packages/table/src/data-table.tsx`:
- Line 648: Replace the inline dynamic type cast used in the JSX prop
(exportConfig={exportConfig as unknown as
import("./types").ExportConfig<ExportableData>}) with a top-level named type
import: add ExportConfig to the existing type import where DataTableProps is
imported (line with DataTableProps) and then cast exportConfig to that imported
ExportConfig<ExportableData> type in the JSX; update the JSX attribute to use
the named ExportConfig type instead of the inline import() expression so editors
and linters handle it cleanly.

In `@packages/table/src/export.tsx`:
- Around line 80-84: The code uses a non-null assertion
exportConfig.columnWidths! inside the exportColumnWidths computation; extract
exportConfig.columnWidths into a local const (e.g., const columnWidths =
exportConfig?.columnWidths) within the same scope and then use columnWidths[i]
when mapping exportHeaders to avoid the bang and make the intent clearer; update
the conditional to check columnWidths truthiness (or reuse the existing
exportConfig?.columnWidths check) and fall back to { wch: 15 } when
columnWidths[i] is undefined, keeping the variable name exportColumnWidths and
referencing exportHeaders as before.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87ebcb3 and 1769037.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • apps/vite-web-example/package.json
  • apps/vite-web-example/request.json
  • apps/vite-web-example/src/pages/orders-page.tsx
  • docs/export-config.md
  • packages/table/package.json
  • packages/table/src/data-table.tsx
  • packages/table/src/export.tsx
  • packages/table/src/toolbar.tsx
  • packages/table/src/types.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/export-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/vite-web-example/package.json

Comment thread apps/vite-web-example/request.json Outdated
@jacksonkasi1 jacksonkasi1 changed the title feat(table): add queryByIds to TableCraftAdapter with sorting for opt… feat: Export Configuration Overhaul & Build Stability Fixes Feb 24, 2026
- fix(tablecraft-adapter): chunk queryByIds requests by maxPageSize to prevent URL/backend limits
- fix(export): merge custom column mapping on top of auto-generated headers
- fix(export): safely handle exportConfig.columnWidths without non-null assertion
- chore: remove accidental request.json payload and add to .gitignore
- refactor(orders-page): remove unnecessary Number() cast for row.total
- refactor(data-table): replace inline type cast with imported ExportConfig
@vercel
Copy link
Copy Markdown

vercel bot commented Feb 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tablecraft Ready Ready Preview, Comment Feb 24, 2026 6:59pm
tablecraft-demo Ready Ready Preview, Comment Feb 24, 2026 6:59pm

@jacksonkasi1 jacksonkasi1 merged commit 7be361d into main Feb 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant