Repository navigation
fix: build paths with Skia.PathBuilder for react-native-skia v3 - #144
Open
mikevocalz wants to merge 1 commit into
Open
mikevocalz wants to merge 1 commit into
mikevocalz wants to merge 1 commit into
Conversation
react-native-skia v3 made SkPath immutable, so calling moveTo, cubicTo or lineTo on a path throws "path.moveTo is not a function". Build paths through a small makePathBuilder helper that uses Skia.PathBuilder when it exists and falls back to the mutable SkPath API on @shopify/react-native-skia versions before 2.6.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
react-native-skia v3 (published as
react-native-skia, the successor to@shopify/react-native-skia) madeSkPathimmutable. Paths are now built withSkia.PathBuilderand finished withbuild(). react-native-graph still callsmoveTo,cubicToandlineToon anSkPathincreateGraphPathand in thestraightLineplaceholder inAnimatedLineGraph, so every graph fails on first render on iOS, Android and web:I couldn't find an existing issue or PR for this.
Fix
A small
makePathBuilder()helper inCreateGraphPath.tschecks forSkia.PathBuilder:@shopify/react-native-skia2.6.0+ and react-native-skia v3), the path is built withSkia.PathBuilder.Make(), and the gradient path starts fromSkia.PathBuilder.MakeFromPath(path).@shopify/react-native-skia2.0 to 2.5), the helper wraps a mutableSkia.Path.Make()/path.copy(), the same calls the library makes today.createGraphPathBaseand thestraightLinememo inAnimatedLineGraphnow go through that helper. The point math is unchanged. The gradient path is still a separate path from the line path, so the line is never mutated.Verification
bun typecheck: passes against the pinned@shopify/react-native-skia2.6.2.tscwith@shopify/react-native-skiamapped to the react-native-skia 3.0.2 type definitions: passes forCreateGraphPath.ts,AnimatedLineGraph.tsx,StaticLineGraph.tsxandGetYForX.ts. Onmainthe same check reports 8 errors (Property 'moveTo' does not exist on type 'SkPath', and the same forcubicToandlineTo).bun lint: no errors. The one warning (no-shadowonxinAnimatedLineGraph.tsx) is already onmain.bun run test: 10/10 pass.PathBuilder.test.tsmocks the v3 API:Skia.PathBuilderexists andSkia.Path.Make()throws. It covers the line path, the gradient path built from the line path, and the empty-data path. All three tests fail onmain.CreateGraphPath.test.tscovers the fallback (noSkia.PathBuilder): the gradient path is acopy()of the line path, and the line path itself gets nolineTocalls.Both majors are tested through Jest mocks only. The repo pins a single Skia version, so the tests don't run the real Skia runtime for v2 and v3 side by side. I've been using the same change as a local patch against react-native-skia 3.0.2.
What this PR does NOT do
@shopify/react-native-skiaor the peer dependencies. Apps on v3 still need to alias@shopify/react-native-skiatoreact-native-skia, for example"@shopify/react-native-skia": "npm:react-native-skia@3.0.2".GetYForX, or anything else that only reads paths. Those APIs exist on both majors.lib/build output.