Rename portable chart barrel to universal - #20
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe package renames portable chart barrels to universal barrels, updates published export maps, adds generic tooltip token exports, and expands packed consumer validation. Documentation and release metadata use universal terminology. Comparison metadata references a newer workspace revision. ChangesUniversal entry points
Comparison metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
56b8a45 to
4a8a256
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/check-packed-consumers.mjs (1)
1114-1127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso validate the token re-exports through
@tanstack/charts/universal.The contract validates
ChartTooltipExtensionTokenandChartTooltipPortalExtensionTokenonly through@tanstack/charts/types. It does not exercise theexport type * from './universal-types'path inpackages/charts-core/src/universal.ts. Add aliased imports from@tanstack/charts/universaland assign the fixtures to those aliases. Keep the existing/typesimports to retain coverage for both public paths.🤖 Prompt for 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. In `@scripts/check-packed-consumers.mjs` around lines 1114 - 1127, Extend the universalSource fixture in check-packed-consumers to import ChartTooltipExtensionToken and ChartTooltipPortalExtensionToken from `@tanstack/charts/universal` under distinct aliases, while keeping the existing imports from `@tanstack/charts/types`. Assign the token fixtures to those aliases so both public re-export paths are validated.
🤖 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 @.changeset/universal-chart-barrels.md:
- Around line 5-7: Update the changeset description to explicitly name both
tooltip-extension and portal-token contracts, while preserving its existing
statements about the /universal and /types entry points and browser-oriented
root exports.
In `@packages/charts-core/docs/comparison.md`:
- Around line 15-22: Update the source documentation file corresponding to
docs/comparison.md rather than editing the generated packages/charts-core copy,
then run pnpm docs:sync to regenerate packages/charts-core/docs/comparison.md
and preserve the documented comparison entries.
---
Nitpick comments:
In `@scripts/check-packed-consumers.mjs`:
- Around line 1114-1127: Extend the universalSource fixture in
check-packed-consumers to import ChartTooltipExtensionToken and
ChartTooltipPortalExtensionToken from `@tanstack/charts/universal` under distinct
aliases, while keeping the existing imports from `@tanstack/charts/types`. Assign
the token fixtures to those aliases so both public re-export paths are
validated.
🪄 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 Plus
Run ID: 7cbfe4d2-6774-4f96-8e2b-25fca9e7b4d9
📒 Files selected for processing (19)
.changeset/portable-chart-barrels.md.changeset/universal-chart-barrels.mdAPI-FRICTION.mdCHANGELOG.mdbenchmarks/comparison/bundle-baseline.jsondocs/comparison.mddocs/installation.mddocs/reference/index.mddocs/reference/types.mdpackages/charts-core/docs/comparison.mdpackages/charts-core/docs/installation.mdpackages/charts-core/docs/reference/index.mdpackages/charts-core/docs/reference/types.mdpackages/charts-core/package.jsonpackages/charts-core/src/exports.test.tspackages/charts-core/src/index.tspackages/charts-core/src/universal-types.tspackages/charts-core/src/universal.tsscripts/check-packed-consumers.mjs
💤 Files with no reviewable changes (1)
- .changeset/portable-chart-barrels.md
| Add environment-safe `/universal` and `/types` entry points, including generic | ||
| tooltip-extension token contracts for non-DOM hosts, while preserving the | ||
| existing browser-oriented root exports. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Name both token contracts in the changeset.
The changeset mentions tooltip-extension token contracts but does not explicitly mention portal-token contracts. Name both tooltip and portal token contracts so the release note matches the public API.
🤖 Prompt for 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.
In @.changeset/universal-chart-barrels.md around lines 5 - 7, Update the
changeset description to explicitly name both tooltip-extension and portal-token
contracts, while preserving its existing statements about the /universal and
/types entry points and browser-oriented root exports.
| | [TanStack Charts](./overview.md) | `@tanstack/charts` | workspace `4a8a256` | | ||
| | [Chart.js](https://www.chartjs.org/docs/latest/) | `chart.js` | npm `4.5.1` | | ||
| | [Apache ECharts](https://echarts.apache.org/handbook/en/best-practices/canvas-vs-svg/) | `echarts` | npm `6.1.0` | | ||
| | [Recharts](https://recharts.github.io/en-US/) | `recharts` | npm `3.10.1` | | ||
| | [Observable Plot](https://observablehq.com/plot/features/plots) | `@observablehq/plot` | npm `0.6.17` | | ||
|
|
||
| The competitor versions are exact package pins, not latest versions inferred | ||
| at page render time. The measured TanStack workspace revision is `b1bba0e`. | ||
| at page render time. The measured TanStack workspace revision is `4a8a256`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Regenerate this documentation copy instead of editing it directly.
Update docs/comparison.md, then run pnpm docs:sync to produce packages/charts-core/docs/comparison.md. Direct edits to the generated copy can be overwritten and can cause documentation drift.
As per coding guidelines: do not directly edit packages/charts-core/docs because these copies are generated by pnpm docs:sync.
🤖 Prompt for 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.
In `@packages/charts-core/docs/comparison.md` around lines 15 - 22, Update the
source documentation file corresponding to docs/comparison.md rather than
editing the generated packages/charts-core copy, then run pnpm docs:sync to
regenerate packages/charts-core/docs/comparison.md and preserve the documented
comparison entries.
Source: Coding guidelines
What
@tanstack/charts/portablesubpath with@tanstack/charts/universal@tanstack/chartsroot entry unchangedWhy
universaldescribes the cross-runtime surface without implying a specialized portability mode. The generic token exports let non-DOM hosts implement definition extensions without hidden core imports. The published0.0.2package contains neither/portablenor/types, so no compatibility alias is needed.Impact
This is non-breaking for published consumers. Existing root imports and runtime behavior are unchanged.
Checks
pnpm run validate— 17/17 tasks passed before the final type-only token exportpnpm package:checkpnpm typecheckSummary by CodeRabbit
New Features
@tanstack/charts/universalentry point for cross-runtime chart definitions and scene compilation.Documentation
Tests