Release TanStack Charts 0.0.1 - #5
Conversation
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (25)
📝 WalkthroughWalkthroughTanStack Charts is prepared for the public ChangesRelease preparation
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MARKETING.md (1)
238-244: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the CTA state with the current public release.
The document calls
0.0.1a public release, but the hero still presents “today” CTAs for exploring examples/docs and reserves “Build your first chart” for “after release.” This conflicts with the current conversion at Lines 629-632. Use the post-release CTA state consistently.🤖 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 `@MARKETING.md` around lines 238 - 244, Update the CTA state in MARKETING.md so the current public-release messaging uses “Build your first chart” as the primary CTA and “Explore examples” as the secondary CTA, matching the post-release state and the conversion referenced around the existing CTA guidance.
🧹 Nitpick comments (4)
scripts/publish-release.mjs (2)
118-140: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
requireProvenanceis alwaystrueat both call sites.Lines 58 and 78 both pass
true; the flag is unused surface area. Dropping it makes the provenance requirement unconditional and non-bypassable.♻️ Drop the flag
-function validateRegistryPackage(artifact, registry, requireProvenance) { +function validateRegistryPackage(artifact, registry) { @@ - if (requireProvenance) { - assert.ok( - hasAttestations(registry), - `${artifact.name}@${artifact.manifest.version} lacks provenance attestations`, - ) - } + assert.ok( + hasAttestations(registry), + `${artifact.name}@${artifact.manifest.version} lacks provenance attestations`, + ) }🤖 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/publish-release.mjs` around lines 118 - 140, Update validateRegistryPackage to remove the requireProvenance parameter and make the hasAttestations assertion unconditional. Adjust both call sites to stop passing the redundant true argument, while preserving the existing registry validation checks and provenance error message.
84-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTimeout path returns a stale result with a misleading assertion.
After 60 attempts the loop falls through and only asserts
lastResultis non-null, so the "did not appear" message fires for a case that is really "integrity/attestations never matched". The caller'svalidateRegistryPackagedoes catch it, so this is diagnostics only — but a direct assertion here would point at the real cause.♻️ Clearer timeout failure
- assert.ok(lastResult, `${artifact.name}@${releaseVersion} did not appear`) - return lastResult + assert.ok( + lastResult, + `${artifact.name}@${releaseVersion} did not appear on the registry`, + ) + assert.equal( + lastResult.dist?.integrity, + artifact.integrity, + `${artifact.name}@${releaseVersion} did not propagate matching integrity in time`, + ) + assert.ok( + hasAttestations(lastResult), + `${artifact.name}@${releaseVersion} did not propagate attestations in time`, + ) + return lastResult🤖 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/publish-release.mjs` around lines 84 - 98, Update waitForRegistryPackage so the timeout path asserts that the final result matches both the expected artifact.integrity and hasAttestations(lastResult), rather than only checking that lastResult exists. Keep returning the validated result on success, and use a timeout message that identifies the integrity/attestation mismatch.scripts/release-package-config.mjs (1)
82-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssertion message doesn't match the checked value.
workspace:*is the "any workspace version" specifier; the "exact-version" property comes from pnpm replacing it with the concrete version at pack time (whichvalidatePackedManifestdoes verify). Consider rewording to avoid implyingworkspace:*itself pins a version.📝 Suggested wording
- `${manifest.name} must use the exact-version workspace protocol`, + `${manifest.name} must depend on `@tanstack/charts` via workspace:*`,🤖 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/release-package-config.mjs` around lines 82 - 87, Reword the assertion message in the manifest dependency check to describe that `@tanstack/charts` uses the workspace:* protocol, without calling it an exact-version specifier. Keep the asserted value and validatePackedManifest behavior unchanged.scripts/release-artifacts.mjs (1)
132-139: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAvoid depending on external
tar -xOfcompression detection.This script is already invoked via
node scripts/..., so keep the manifest extraction Node-backed;-xOfcan fail differently across localtarbuilds than CI’subuntu-24.04. Use-xzOfornode:zlib/node:tarto remove the platform-dependent external dependency.🤖 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/release-artifacts.mjs` around lines 132 - 139, Update readPackedManifest to avoid relying on external tar’s automatic compression detection: explicitly use gzip extraction with the tar invocation (such as the -xzOf flags), or replace the external command with Node-backed zlib/tar handling. Preserve extracting package/package.json from the provided tarball and parsing the resulting manifest as JSON.
🤖 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 @.github/workflows/release.yml:
- Line 47: Remove the version-only npm bootstrap step from the release workflow
before pnpm release:publish. Use the Node distribution-provided npm, or replace
it with a digest-pinned artifact bootstrap if one is required; do not install
npm solely by the mutable version string 11.12.1.
- Line 42: Update the release job’s actions/setup-node configuration to
explicitly disable automatic package-manager caching by setting its cache option
to an empty value. Keep the existing pinned action version and Node setup
unchanged.
In `@docs/comparison.md`:
- Around line 114-117: Replace the mutable main-branch URLs in the comparison
evidence links with immutable references to commit a91106c or the eventual
v0.0.1 tag in docs/comparison.md lines 114-117. Regenerate the mirrored links in
packages/charts-core/docs/comparison.md lines 114-117 with the same immutable
references.
In `@packages/charts-core/docs/examples/polar-and-radar.md`:
- Around line 599-600: Restore the removed guidance in the radialDot example
documentation: explain that marks infer identity from source IDs or unique
positions, and a key is only needed when neither is available. Keep the
documentation consistent with radialDot’s data.id and unique-event/index
identity behavior.
In `@scripts/compare-chart-libraries.mjs`:
- Around line 1165-1183: Update the TanStack branch in the baseline validation
around source.revision to require an exact match with the current workspace
revision() result, rather than only accepting any 40-character SHA; retain the
existing workspace-kind check and failure behavior, while leaving external
package validation unchanged.
In `@scripts/evaluate-chart-authoring.mjs`:
- Around line 22-30: Update the chartsPackageVersion validation in
evaluate-chart-authoring.mjs to require a valid npm semver, including supported
prerelease versions, rather than only a non-empty string. Reject values such as
“latest” and whitespace before generating workspace metadata, while preserving
the existing error behavior for invalid versions.
---
Outside diff comments:
In `@MARKETING.md`:
- Around line 238-244: Update the CTA state in MARKETING.md so the current
public-release messaging uses “Build your first chart” as the primary CTA and
“Explore examples” as the secondary CTA, matching the post-release state and the
conversion referenced around the existing CTA guidance.
---
Nitpick comments:
In `@scripts/publish-release.mjs`:
- Around line 118-140: Update validateRegistryPackage to remove the
requireProvenance parameter and make the hasAttestations assertion
unconditional. Adjust both call sites to stop passing the redundant true
argument, while preserving the existing registry validation checks and
provenance error message.
- Around line 84-98: Update waitForRegistryPackage so the timeout path asserts
that the final result matches both the expected artifact.integrity and
hasAttestations(lastResult), rather than only checking that lastResult exists.
Keep returning the validated result on success, and use a timeout message that
identifies the integrity/attestation mismatch.
In `@scripts/release-artifacts.mjs`:
- Around line 132-139: Update readPackedManifest to avoid relying on external
tar’s automatic compression detection: explicitly use gzip extraction with the
tar invocation (such as the -xzOf flags), or replace the external command with
Node-backed zlib/tar handling. Preserve extracting package/package.json from the
provided tarball and parsing the resulting manifest as JSON.
In `@scripts/release-package-config.mjs`:
- Around line 82-87: Reword the assertion message in the manifest dependency
check to describe that `@tanstack/charts` uses the workspace:* protocol, without
calling it an exact-version specifier. Keep the asserted value and
validatePackedManifest behavior unchanged.
🪄 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: 91b3bbc5-ccca-411c-8ec6-034a38b36113
📒 Files selected for processing (93)
.github/workflows/release.yml.gitignoreAPI-FRICTION.mdCHANGELOG.mdMARKETING.mdREADME.mdbenchmarks/comparison/bundle-baseline.jsonbenchmarks/comparison/libraries/tanstack/base.tsdocs/comparison.mddocs/concepts/chart-definitions.mddocs/concepts/data-and-channels.mddocs/concepts/grammar-of-graphics.mddocs/concepts/layout-axes-and-coordinates.mddocs/concepts/marks-and-layering.mddocs/concepts/scales-and-d3.mddocs/examples/annotations-and-overlays.mddocs/examples/bars-and-rankings.mddocs/examples/facets-and-multiple-views.mddocs/examples/interactive-charts.mddocs/examples/lines-and-areas.mddocs/examples/maps-and-spatial.mddocs/examples/polar-and-radar.mddocs/framework/octane/quick-start.mddocs/framework/react/adapter.mddocs/framework/react/quick-start.mddocs/guides/ai-authoring.mddocs/guides/bundle-size-and-performance.mddocs/guides/faceting-and-composition.mddocs/guides/interactions-and-selections.mddocs/guides/ssr-and-hydration.mddocs/guides/typescript.mddocs/installation.mddocs/overview.mddocs/quick-start.mddocs/reference/index.mddocs/reference/rendering-and-export.mddocs/reference/types.mdllms.txtpackage.jsonpackages/alpine-charts/package.jsonpackages/angular-charts/package.jsonpackages/charts-core/README.mdpackages/charts-core/docs/comparison.mdpackages/charts-core/docs/concepts/chart-definitions.mdpackages/charts-core/docs/concepts/data-and-channels.mdpackages/charts-core/docs/concepts/grammar-of-graphics.mdpackages/charts-core/docs/concepts/layout-axes-and-coordinates.mdpackages/charts-core/docs/concepts/marks-and-layering.mdpackages/charts-core/docs/concepts/scales-and-d3.mdpackages/charts-core/docs/examples/annotations-and-overlays.mdpackages/charts-core/docs/examples/bars-and-rankings.mdpackages/charts-core/docs/examples/facets-and-multiple-views.mdpackages/charts-core/docs/examples/interactive-charts.mdpackages/charts-core/docs/examples/lines-and-areas.mdpackages/charts-core/docs/examples/maps-and-spatial.mdpackages/charts-core/docs/examples/polar-and-radar.mdpackages/charts-core/docs/framework/octane/quick-start.mdpackages/charts-core/docs/framework/react/adapter.mdpackages/charts-core/docs/framework/react/quick-start.mdpackages/charts-core/docs/guides/ai-authoring.mdpackages/charts-core/docs/guides/bundle-size-and-performance.mdpackages/charts-core/docs/guides/faceting-and-composition.mdpackages/charts-core/docs/guides/interactions-and-selections.mdpackages/charts-core/docs/guides/ssr-and-hydration.mdpackages/charts-core/docs/guides/typescript.mdpackages/charts-core/docs/installation.mdpackages/charts-core/docs/overview.mdpackages/charts-core/docs/quick-start.mdpackages/charts-core/docs/reference/index.mdpackages/charts-core/docs/reference/rendering-and-export.mdpackages/charts-core/docs/reference/types.mdpackages/charts-core/llms.txtpackages/charts-core/package.jsonpackages/lit-charts/package.jsonpackages/octane-charts/package.jsonpackages/preact-charts/package.jsonpackages/react-charts/README.mdpackages/react-charts/package.jsonpackages/solid-charts/package.jsonpackages/svelte-charts/package.jsonpackages/vue-charts/package.jsonscripts/build-release-artifacts.mjsscripts/check-framework-adapters.mjsscripts/check-packed-consumers.mjsscripts/compare-chart-libraries.mjsscripts/docs-contract.mjsscripts/docs-contract.test.mjsscripts/evaluate-chart-authoring.mjsscripts/publish-release.mjsscripts/release-artifacts.mjsscripts/release-artifacts.test.mjsscripts/release-package-config.mjsscripts/sync-package-docs.mjs
💤 Files with no reviewable changes (2)
- packages/charts-core/docs/reference/rendering-and-export.md
- docs/reference/rendering-and-export.md
Publishes the coordinated 0.0.1 release candidate for core and all nine framework adapters.
Includes:
Local validation:
The npm trusted publisher is configured after this workflow reaches main and before the v0.0.1 tag is pushed.
Summary by CodeRabbit
New Features
0.0.1pre-alpha release across core and framework integrations.Documentation
Chores