Repository navigation
fix(🌎): type MakeWebGPUCanvasContext as nullable and keep getNativeSurface() working after unmount - #548
Open
YevheniiKotyrlo wants to merge 3 commits into
Conversation
…rface() working after unmount The global RNWebGPU type said MakeWebGPUCanvasContext always returns a context, while the web polyfill returns null without WebGPU support (wcandillon#489); the polyfill hid that behind a @ts-expect-error over its whole object and a cast. The type now says null, and the polyfill is checked as the members of the native module it provides. On the web, getNativeSurface() and MakeWebGPUCanvasContext read the canvas element without a null check, so both threw a TypeError once the canvas had unmounted. getNativeSurface() now answers what native answers for a context it no longer knows, a 0 x 0 surface whose DOM methods do nothing, and MakeWebGPUCanvasContext returns null.
This branch has not been deployed
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
1 participant
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.
Problem
The global
RNWebGPU.MakeWebGPUCanvasContextis typed to always return a context, but the web polyfill returnsnullwithout WebGPU support (#489). The polyfill hides that behind a@ts-expect-errorover its whole object and a cast, andWebPolyfill.spec.tsdeclares its ownPolyfilltype to say so.On the web,
getNativeSurface()andMakeWebGPUCanvasContextread the canvas element withgetElementById(...) as HTMLCanvasElement, so once the canvas has unmounted they throwTypeError: Cannot read properties of null (reading 'clientHeight')and(reading 'getAttribute'). Native answers a context it no longer knows withCanvas(nullptr, 0, 0): a0nsurface, every size 0, and DOM methods that do nothing.Solution
getCanvasElementreturnsHTMLCanvasElement | null. For a missing element,getNativeSurfaceanswers that same retired surface andMakeWebGPUCanvasContextreturnsnull. The global type returnsRNCanvasContext | null. The polyfill is annotated as thePickof the four members it provides, so each is checked against the native module's type, and assigned with one cast for the members the web lacks.WebPolyfill.spec.tsdrops its ownPolyfilltype, andCanvasContext.spec.ts's four device evals throw if native ever returns no context.Tests
WebPolyfill.spec.tsgains two cases withgetElementByIdansweringnull: a 0 × 0 surface whose DOM methods do not throw, and no context. Onmainboth fail with thoseTypeErrors (2 failed, 2 passed); with the change, 4 passed.Verification
yarn lint·yarn tsc·yarn turbo run build --filter='./packages/*'· the Expo template's web build ·yarn test:plugin·yarn check-dawn·yarn build:docs. CI's Android e2e step (debug build, Metro withCI=true) on an Android 15 emulator (API 35): 221 of 223,main's 219 and the two newWebPolyfill.spec.tscases; the two failures, the YUV cases ofExternalTextureColorSpaceandNativeBufferWrapper, fail onmainthere too. On devices the whole suite passes, 223 of 223: on a Galaxy S23 (Android 14), and on an iPhone 12 Pro Max (iOS 27.0.1) built and run as CI's iOS job does on a simulator (Debug build, Metro withCI=true).mainpasses 221 of 221 on both. Not run here: the semantic-release dry run, which needs the repository's token.