Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 2cf7651 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe declaration rewrite script now appends ChangesDeclaration export preservation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change adds module markers to generated declarations that need them. No actionable merge blocker was established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
🎯 Changes
Fixes #6619.
Since 9.2.5 the
table-core.d.tsfiles no longer end withexport { … }(rolldown-plugin-dts 0.28.2+ inlines theexportmodifiers instead). A declaration file without any export statement exports every top-level declaration, so private aliases such asRangeValueinaggregationFns.d.tsbecome referable, but only through a deep path that the packageexportsmap blocks. A consumer emitting declarations then fails:scripts/rewrite-table-core-dts.mjsnow appendsexport {};to declaration files that have noexport { … },export *orexport =statement, as suggested in the issue. This is a workaround until sxzz/rolldown-plugin-dts#312 is fixed upstream (sxzz/rolldown-plugin-dts#310 is still open); once it is, the step becomes a no-op.Verified against the built
distlinked into a consumer through the packageexportsmap, with the repro from the issue (module/moduleResolution: NodeNext,declaration: true):RangeValue; 117 of the emitted.d.tsfiles had no export statement(number | Date) | undefinedas with 9.2.4; no declaration file is left without an export statement.,static-functions,reactivity,flex-render,store-reactivity-bindings,experimental-worker-plugin) withskipLibCheck: falsetype-checks both before and after, so nothing that the files import from each other was relying on the implicit exports✅ Checklist
pnpm testandpnpm test:e2e, or these tests do not apply to this pull request.Ran for
@tanstack/table-core:build,test:types,test:build(publint) andtest:lib(1330 tests pass), plustsc --noEmitfor the React, Vue and Solid adapters against the rebuiltdist. I did not run the fullpnpm test/pnpm test:e2esuites; the change only touches the post-build declaration rewrite.🚀 Release Impact
Summary by CodeRabbit