Skip to content

fix: build paths with Skia.PathBuilder for react-native-skia v3 - #144

Open
mikevocalz wants to merge 1 commit into
margelo:mainfrom
mikevocalz:fix/skia-v3-pathbuilder
Open

mikevocalz wants to merge 1 commit into
margelo:mainfrom
mikevocalz:fix/skia-v3-pathbuilder

Conversation

@mikevocalz

Copy link
Copy Markdown

Bug

react-native-skia v3 (published as react-native-skia, the successor to @shopify/react-native-skia) made SkPath immutable. Paths are now built with Skia.PathBuilder and finished with build(). react-native-graph still calls moveTo, cubicTo and lineTo on an SkPath in createGraphPath and in the straightLine placeholder in AnimatedLineGraph, so every graph fails on first render on iOS, Android and web:

TypeError: path.moveTo is not a function

I couldn't find an existing issue or PR for this.

Fix

A small makePathBuilder() helper in CreateGraphPath.ts checks for Skia.PathBuilder:

  • If it exists (@shopify/react-native-skia 2.6.0+ and react-native-skia v3), the path is built with Skia.PathBuilder.Make(), and the gradient path starts from Skia.PathBuilder.MakeFromPath(path).
  • If it doesn't (@shopify/react-native-skia 2.0 to 2.5), the helper wraps a mutable Skia.Path.Make() / path.copy(), the same calls the library makes today.

createGraphPathBase and the straightLine memo in AnimatedLineGraph now 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-skia 2.6.2.
  • tsc with @shopify/react-native-skia mapped to the react-native-skia 3.0.2 type definitions: passes for CreateGraphPath.ts, AnimatedLineGraph.tsx, StaticLineGraph.tsx and GetYForX.ts. On main the same check reports 8 errors (Property 'moveTo' does not exist on type 'SkPath', and the same for cubicTo and lineTo).
  • bun lint: no errors. The one warning (no-shadow on x in AnimatedLineGraph.tsx) is already on main.
  • bun run test: 10/10 pass.
    • New PathBuilder.test.ts mocks the v3 API: Skia.PathBuilder exists and Skia.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 on main.
    • New case in CreateGraphPath.test.ts covers the fallback (no Skia.PathBuilder): the gradient path is a copy() of the line path, and the line path itself gets no lineTo calls.

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

  • Change the import from @shopify/react-native-skia or the peer dependencies. Apps on v3 still need to alias @shopify/react-native-skia to react-native-skia, for example "@shopify/react-native-skia": "npm:react-native-skia@3.0.2".
  • Bump the Skia version used by the repo or the example app.
  • Touch path interpolation, GetYForX, or anything else that only reads paths. Those APIs exist on both majors.
  • Commit lib/ build output.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant