Skip to content

fix: exp post build script to strip Table_Internal#6326

Open
KevinVandy wants to merge 2 commits into
betafrom
strip-internal-types
Open

fix: exp post build script to strip Table_Internal#6326
KevinVandy wants to merge 2 commits into
betafrom
strip-internal-types

Conversation

@KevinVandy

@KevinVandy KevinVandy commented Jun 15, 2026

Copy link
Copy Markdown
Member

🎯 Changes

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested this code locally with pnpm test:pr.

Summary by CodeRabbit

  • Chores
    • Updated the build to enhance TypeScript declaration handling by rewriting generated declaration files and validating that internal types don’t leak.
    • Updated internal table/header/column typing so generated public-facing TypeScript declarations remain consistent and clean.

@KevinVandy KevinVandy requested a review from a team as a code owner June 15, 2026 13:26
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1b60e0b3-fb22-4168-82b5-156a7d87e70d

📥 Commits

Reviewing files that changed from the base of the PR and between b8eefd2 and 4e7d50a.

📒 Files selected for processing (1)
  • scripts/rewrite-table-core-dts.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/rewrite-table-core-dts.mjs

📝 Walkthrough

Walkthrough

Column_Internal gains a table: Table_Internal<...> property and HeaderContext.table is typed as Table_Internal. A new post-build script (rewrite-table-core-dts.mjs) scans emitted .d.ts/.d.cts files, removes those internal interface declarations and type aliases, strips their named specifiers, and replaces remaining references with public Table/Column equivalents. The table-core build script is updated to invoke this script after tsdown.

Changes

Internal type adoption and .d.ts rewrite

Layer / File(s) Summary
Source types adopt Table_Internal
packages/table-core/src/types/Column.ts, packages/table-core/src/core/headers/coreHeadersFeature.types.ts
Column_Internal omits and re-declares table as Table_Internal<...>, and HeaderContext.table is typed as Table_Internal<...>, causing those names to appear in emitted declarations.
DTS rewrite script implementation
scripts/rewrite-table-core-dts.mjs
New script walks dist for .d.ts/.d.cts files, removes Table_Internal/Column_Internal exported interfaces and the Table_InternalBroadenedKeys type alias, strips their named import/export specifiers, replaces remaining internal references with public equivalents, and validates no names leaked.
Build script wiring
packages/table-core/package.json
Build script is updated to run the rewrite script after tsdown.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hop hop, the internals hide away,
Scrubbed from each .d.ts on build day,
Table_Internal quietly goes,
Replaced by Table wherever it shows,
A script to keep the public types clean and bright,
No leaking internals — the rabbit checked it right! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix: exp post build script to strip Table_Internal' is partially related to the changeset but uses abbreviation 'exp' that obscures the actual intent, and 'post build script' is vague given that the script is added to the build process itself. Consider clarifying the title to 'fix: add post-build script to strip internal types from declarations' or similar to better describe what the script does and why it's needed.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch strip-internal-types

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.

@nx-cloud

nx-cloud Bot commented Jun 15, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 4e7d50a

Command Status Duration Result
nx affected --targets=test:eslint,test:sherif,t... ✅ Succeeded 3m 11s View ↗
nx run-many --targets=build --exclude=examples/** ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2026-06-15 13:58:18 UTC

@pkg-pr-new

pkg-pr-new Bot commented Jun 15, 2026

Copy link
Copy Markdown
More templates

@tanstack/angular-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/angular-table@6326

@tanstack/angular-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/angular-table-devtools@6326

@tanstack/lit-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/lit-table@6326

@tanstack/match-sorter-utils

npm i https://pkg.pr.new/TanStack/table/@tanstack/match-sorter-utils@6326

@tanstack/preact-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/preact-table@6326

@tanstack/preact-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/preact-table-devtools@6326

@tanstack/react-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/react-table@6326

@tanstack/react-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/react-table-devtools@6326

@tanstack/solid-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/solid-table@6326

@tanstack/solid-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/solid-table-devtools@6326

@tanstack/svelte-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/svelte-table@6326

@tanstack/table-core

npm i https://pkg.pr.new/TanStack/table/@tanstack/table-core@6326

@tanstack/table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/table-devtools@6326

@tanstack/vue-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/vue-table@6326

@tanstack/vue-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/vue-table-devtools@6326

commit: 4e7d50a

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/rewrite-table-core-dts.mjs`:
- Around line 92-109: The regex pattern in the replacer function does not match
TypeScript's `import type { ... }` syntax, only `import { ... }` and `export {
... }`. Modify the regex pattern to make the `type` keyword optional between the
import/export keyword and the opening brace, so that both `import { ... }` and
`import type { ... }` statements are properly matched and processed by the
filtering logic.
- Around line 73-87: The removeTypeAlias function currently searches for the
pattern `type ${typeName}` but does not account for the `export` keyword that
may precede it in the TypeScript declaration file. When an exported type alias
is found, the function only removes from the word `type` onward, leaving behind
the dangling `export` keyword. Fix this by checking if the character sequence
before the start position contains `export ` (including the space), and if so,
adjust the start position to include and remove the export keyword as well. This
ensures that both exported and non-exported type aliases are completely removed
without leaving orphaned tokens.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ba3a2dcc-5fa1-4116-817b-c15b906fae24

📥 Commits

Reviewing files that changed from the base of the PR and between 26fc105 and b8eefd2.

📒 Files selected for processing (4)
  • packages/table-core/package.json
  • packages/table-core/src/core/headers/coreHeadersFeature.types.ts
  • packages/table-core/src/types/Column.ts
  • scripts/rewrite-table-core-dts.mjs

Comment thread scripts/rewrite-table-core-dts.mjs
Comment thread scripts/rewrite-table-core-dts.mjs Outdated
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