Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fix element screenshots for react-native-skia and similar #4493

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mauricedoepke
Copy link
Contributor

@mauricedoepke mauricedoepke commented May 21, 2024

Also see:
Shopify/react-native-skia#1880

In this pull request, I have implemented a method to manually render TextureView in android apps as they do not support rendering themselves.

This is still a WIP as my fix itself works, but the e2e test is still missing, as I have probems compiling the sample app with react-native-skia

For features/enhancements:

  • I have added/updated the relevant references in the documentation files.

For API changes:

  • I have made the necessary changes in the types index file.

@mauricedoepke mauricedoepke changed the title add skia to example app [WIP ]Fix element screenshot for react-native-skia May 21, 2024
@mauricedoepke mauricedoepke changed the title [WIP ]Fix element screenshot for react-native-skia [WIP] Fix element screenshots for react-native-skia and similar May 21, 2024
@mauricedoepke
Copy link
Contributor Author

In my own app, the fix in this pr is working. I am having trouble adding rn skia to the detox test app though to create an e2e test. As soon as I add skia, I get the following error:

error node_modules/@shopify/react-native-skia/lib/module/specs/SkiaPictureViewNativeComponent.js: /Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@shopify/react-native-skia/lib/module/specs/SkiaPictureViewNativeComponent.js: Could not find component config for native component.
Error: /Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@shopify/react-native-skia/lib/module/specs/SkiaPictureViewNativeComponent.js: Could not find component config for native component
    at throwIfConfigNotfound (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/codegen/lib/parsers/error-utils.js:285:11)
    at findComponentConfig (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/codegen/lib/parsers/parsers-commons.js:896:3)
    at buildComponentSchema (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/codegen/lib/parsers/flow/components/index.js:24:32)
    at buildSchemaFromConfigType (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/codegen/lib/parsers/parsers-commons.js:462:34)
    at buildSchema (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/codegen/lib/parsers/parsers-commons.js:526:10)
    at FlowParser.parseString (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/codegen/lib/parsers/flow/parser.js:134:12)
    at parseFile (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/babel-plugin-codegen/index.js:36:23)
    at generateViewConfig (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/babel-plugin-codegen/index.js:49:18)
    at PluginPass.exit (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/babel-plugin-codegen/index.js:169:32)
    at newFn (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/visitors.js:160:14)
    at NodePath._call (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/path/context.js:46:20)
    at NodePath.call (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/path/context.js:36:17)
    at NodePath.visit (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/path/context.js:90:8)
    at TraversalContext.visitQueue (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/context.js:89:16)
    at TraversalContext.visitSingle (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/context.js:65:19)
    at TraversalContext.visit (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/context.js:112:19)
    at traverseNode (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/traverse-node.js:22:17)
    at traverse (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/traverse/lib/index.js:52:34)
    at transformFile (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/core/lib/transformation/index.js:82:31)
    at transformFile.next (<anonymous>)
    at run (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/core/lib/transformation/index.js:24:12)
    at run.next (<anonymous>)
    at /Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/core/lib/transform-ast.js:23:33
    at Generator.next (<anonymous>)
    at evaluateSync (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/gensync/index.js:251:28)
    at sync (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/gensync/index.js:89:14)
    at stopHiding - secret - don't use this - v1 (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/core/lib/errors/rewrite-stack-trace.js:47:12)
    at transformFromAstSync (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@babel/core/lib/transform-ast.js:43:83)
    at Object.transform (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/@react-native/metro-babel-transformer/src/index.js:216:7)
    at transformJSWithBabel (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/metro-transform-worker/src/index.js:287:45)
    at Object.transform (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/metro-transform-worker/src/index.js:412:18)
    at transformFile (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/metro/src/DeltaBundler/Worker.flow.js:54:36)
    at Object.transform (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/metro/src/DeltaBundler/Worker.flow.js:30:10)
    at execFunction (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/jest-worker/build/workers/processChild.js:149:17)
    at execHelper (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/jest-worker/build/workers/processChild.js:137:5)
    at execMethod (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/jest-worker/build/workers/processChild.js:140:5)
    at process.messageListener (/Users/maurice-pascaldopke/Desktop/projects/Detox/detox/test/node_modules/jest-worker/build/workers/processChild.js:44:7)
    at process.emit (node:events:517:28)
    at emit (node:internal/child_process:944:14)

Unfortunately I have no real experience how native module resolution in react-native works and due to the lack of google results on this I am a bit stuck with this.
(The only result said to add a react-native field to the package.json of the lib, which skia already has)

Do you guys have any hints or ideas on how to solve this or even to debug this further?

I asked the same question to wcandillion here: Shopify/react-native-skia#1880 as well as I am not sure if this is a skia problem or a problem of how the Detox test app is configured.

Copy link

stale bot commented Jan 22, 2025

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.
For more information on bots in this repository, read this discussion.

@stale stale bot added the 🏚 stale label Jan 22, 2025
Copy link

stale bot commented Jan 31, 2025

The issue has been closed for inactivity.

@stale stale bot closed this Jan 31, 2025
@d4vidi d4vidi reopened this Feb 3, 2025
@d4vidi
Copy link
Collaborator

d4vidi commented Feb 3, 2025

@mauricedoepke thanks for this effort! sorry, seemed to have fallen between the cracks. Do you have any work left here, or are you just waiting for feedbacks / reviews?

@stale stale bot removed the 🏚 stale label Feb 3, 2025
@mauricedoepke
Copy link
Contributor Author

@d4vidi The fix itself is ready and working. I am using it for a long time in a locally patched version of detox.

I was unable to develop a test for this feature withing the detox repo though, as I could not manage to add rn-skia to the test app. I was unable to make it compile. See the error above. Unfortunately I don't have any bare react nativ experience without expo. So even with extensive research I was unable to get skia into the testapp as a dependency.

@d4vidi
Copy link
Collaborator

d4vidi commented Feb 4, 2025

Oh I see it now. @noomorph you have any clue about what could be the cause for this? @mauricedoepke this happens when you build the test app? What's the command you're running?

@d4vidi
Copy link
Collaborator

d4vidi commented Feb 4, 2025

@mauricedoepke could you please remerge from master?

@d4vidi
Copy link
Collaborator

d4vidi commented Feb 6, 2025

@mauricedoepke wdyt?

@mauricedoepke
Copy link
Contributor Author

@d4vidi Will do so this weekend, then I have some time to do so.

@d4vidi
Copy link
Collaborator

d4vidi commented Feb 17, 2025

@mauricedoepke would love to get the merged branched from you in order to be able to run CI checks, etc

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.

2 participants