Repository navigation
Conversation
Declare repeating tiles in a chart's patterns list and reference them from fill or stroke as url(#id). SVG emits scoped userSpaceOnUse patterns, Canvas rasterizes the tile at device resolution and repeats it with the same rotation, and React Native emits react-native-svg Pattern. linePattern() and dotPattern() at @tanstack/charts/pattern build hatch and dot tiles with CSS-variable paint.
Pass the SVG paint hooks into pattern tiles so a tile that paints with a gradient gets the scoped id. Reject gradient and pattern ids that collide, since SVG emits both while Canvas picks one. Make ChartScene.patterns required like gradients while renderers still tolerate untyped scenes without it.
Keep ChartScene.patterns optional and leave gradient and pattern id collisions undefined, as documented in the reference. The check cost about 60 gzip bytes in every default bundle and newly rejected duplicate gradient ids. Restore bundle budgets and baselines to measured values.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 18 files. (16 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding renderer-neutral pattern resources across the chart renderers.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 18 files. (16 skipped: 16 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Autopilot is currently an internal CodeRabbit preview.
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 @coderabbitai help to get the list of available commands.
|
Thanks @greglum. I checked Two things still need attention before we can merge:
The benchmark creates one scene per trial, uses Full repository checks, the complete bundle matrix, and on-device React Native rendering are still pending. This does not approve the budget changes or mark the PR ready to merge. |
🦋 Changeset detectedLatest commit: ad42701 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 |
|
Thanks @greglum, full isolated validation now passes 2,233 DOM tests, all six additional framework targets, TypeScript, docs and the full packed-package gate, including bare React Native, Expo and seven adapters. Bundle checks pass against the PR’s proposed limits, which is not approval of those increases. The final packed artifact exposed one additional browser difference worth resolving before claiming fractional SVG/Canvas parity. With pixelRatio 1, a 96 × 96 scene, and a repeating 1.1 × 1.3 tile containing a white background and a black top half, WebKit paints visible Canvas stripes while the rasterized SVG is nearly solid gray. The 80 × 80 interior comparison has mean maximum-channel delta 78.825 and 22.5% of pixels above delta 96. A 6 × 6 integer tile matches exactly. The same fractional fixture passes in Chromium and Firefox. The SVG reference has explicit 96 × 96 dimensions, not percentage intrinsic sizing. I repeated the packed check and inspected the rendered comparison. Cycle, 65-level dependency and 4097-pixel side guards, plus CSS-variable repainting, pass in all three engines. This is a rendering-parity finding, not evidence of a new security problem. It needs a focused regression and a decision about consistent fractional sampling, without just widening the pixel tolerance. Update: the broader packed pattern/export fixture passes in all three engines. Its initial WebKit SVG-decoding failure came from the harness blocking local blob images. Allowing only local blob/data images, while continuing to block network requests, resolved it. The fractional-tile mismatch above remains reproducible. Existing budget, default-path performance and required-CI holds remain. Native-browser isolation: the same 1.1 × 1.3 mismatch reproduces using only CanvasPattern and SVG, with no Charts code. Smoothing on/off does not change WebKit’s result, and increasing tile resolution is not a consistent fix across engines. An independent area-coverage calculation also shows that rasterized SVG is not an exact reference for these subpixel stripes. Repeating multiple tiles over an integer-sized period improves coverage accuracy but still does not match native SVG, so I have not adopted that as a renderer fix or relaxed the test threshold. This narrows the finding to native sampling differences; it does not establish a Charts geometry bug. Verified head: |
This branch has not been deployed
2 participants
Resolves API-FRICTION F-259 and covers the hatch and dot requests in #128.
What: charts can declare
patternsnext togradientsand use them asfillorstrokethroughurl(#id).linePattern()anddotPattern()from@tanstack/charts/patternbuild hatch and dot tiles whose colors accept CSS variables. Gradient and pattern IDs share theurl(#id)namespace and must be unique; behavior on a collision is undefined. Animated patterns are not included.Why: authors had to render their own
<pattern>outside the chart, which broke export, Canvas, React Native andidPrefixscoping. Patterns also give charts a non-colour cue. We need one in a production app because our amber and red status fills look alike to readers with deuteranopia.How: a pattern is plain data: tile size, optional angle, and ordinary scene nodes (
nodes: SceneNode[]). The two factories are the vocabulary.We chose nodes over a kind descriptor such as
{ type: 'lines' }on purpose. With a descriptor, the line and dot drawing code would live in three renderers and grow every default bundle. With nodes, each renderer reuses the node code it already has, the vocabulary tree-shakes away unless imported, and custom tiles need no new renderer code.userSpaceOnUsepattern and scopes resource paints inside tiles too, so a tile can use a gradient.RangeError.Pattern(react-native-svg 15).Proposed bundle cost (gzip):
Matched production builds compare upstream
5a34b7e14d85ff8c7b7248fedb2fd9731cacbd67with pattern sourceec695a8b993f4c22d4210df39b0d75c56ceab3a2, using Node 24.18.0, pnpm 11.15.1, and esbuild 0.27.7.These increases and the corresponding ceilings/locked-baseline updates remain proposed, pending maintainer approval. Decimal kB uses 1000 bytes; source ceilings use KiB (1024 bytes). The existing upstream headroom is retained. Scene-only consumers add 7 to 8 bytes. The factories add 280 bytes (about 0.273 KiB) under a proposed 0.3 KiB incremental cap; the ordinary line consumer forbids their module.
Default path performance:
Both variants use matched production builds and identical scene fixtures in Chromium 151.0.7922.34. A trial creates one 960 by 540 scene with 1000 solid-color rectangles, each 6 by 6 at
(i % 100 * 8, Math.floor(i / 100) * 8), filled with#2563eb, with no patterns. It renders that scene 100 times throughsurface.render(scene, { ariaLabel: 'Bench' })at pixel ratio one.Each of two fresh browser processes reuses its two variant contexts for three warmups and seven paired trials. Execution order alternates per trial; the second process reverses context creation order.
No consistent default-path slowdown was reproduced in this bounded fixture. Solid-color paints return before the pattern lookup; the painter still creates its per-paint pattern cache and budget state. These timings did not justify a runtime optimization. Other workloads and devices need their own measurements.
Verified:
idPrefix(including a tile that uses a gradient), the Canvas paint path for fill, stroke and text, CSS-variable resolution, export inlining, and React Native markup.pnpm canvas:checkverification compared Canvas output for rotated hatch and dot tiles against rasterized exported SVG: mean difference about 1 in 255 per channel, with zero divergent pixels. Reversing the Canvas rotation makes it fail.pnpm run validatepasses all 19 targets atad427019503aa65c469bc047b998ecc015dbf0ea, including 2233 unit tests, strict packed consumers, all seven framework adapters, types, docs, bundles, comparison and catalog checks.Open question: do you want a duplicate resource ID check at scene compilation? It would cover gradient–gradient and gradient–pattern collisions. The measured cost is about 60 B gzip on every default bundle, and it would newly reject duplicate gradient IDs that work today, so we left it out.
Summary by CodeRabbit