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

Integrate Expo Snack into docs #305

Open
carbonrobot opened this issue Jun 24, 2024 · 16 comments
Open

Integrate Expo Snack into docs #305

carbonrobot opened this issue Jun 24, 2024 · 16 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request issue-accepted The issue is confirmed and accepted by the maintainers

Comments

@carbonrobot
Copy link
Contributor

Integrate Expo Snack into the docs site similar to how to do examples for Victory Web.

image

@carbonrobot carbonrobot added documentation Improvements or additions to documentation enhancement New feature or request issue-accepted The issue is confirmed and accepted by the maintainers labels Jun 24, 2024
@zibs
Copy link
Contributor

zibs commented Jun 24, 2024

@mrpmohiburrahman
Copy link

mrpmohiburrahman commented Jul 17, 2024

Hi, @carbonrobot, @zibs
I have successfully integrated expo snacks on the docs, in my fork
But I ran into a problem when running victory-native on the expo snack itself.
Currently, I am facing this issue:

App.js (0:1): Unable to resolve module 'module://@babel/runtime/helpers/interopRequireDefault.js'(Device)
Evaluating module://@babel/runtime/helpers/interopRequireDefault.js
Evaluating module://victory-native.js
Evaluating module://App.js.js
Loading module://App.js

Expo Snack: https://snack.expo.dev/@mrpmohibur/486cbe?platform=web

Please, note that I can successfully run expo managed project locally, with the same versions of libraries on the given expo snack above.

If anyone has any idea on how to solve this, I can implement expo snacks for all the example codes.

@zibs
Copy link
Contributor

zibs commented Jul 19, 2024

Hey @mrpmohiburrahman, thanks for your efforts here! Based on your snack, it looks like you're trying to integrate against the older versions of victory-native, whereas we want to use the latest versions and implementation of the library here! There is no more VictoryTheme and data goes in the VictoryChart now, etc. See more here: https://commerce.nearform.com/open-source/victory-native/docs/cartesian/line/

For the snack error itself, have you taken a look at the link in the issue above^ https://shopify.github.io/react-native-skia/docs/getting-started/web/#using-code-splitting? I think that might help!

@mrpmohiburrahman
Copy link

@zibs , sorry for the late reply.

Right now, I am getting a new error

Cannot read properties of undefined (reading 'XYWHRect')
TypeError: Cannot read properties of undefined (reading 'XYWHRect')
    at Object.XYWHRect (@shopify/react-native.skia:22:171768)
    at a (@shopify/react-native.skia:22:217269)
    at t.CartesianChart (victory.native:3:85586)

image

snack link: https://snack.expo.dev/@mrpmohibur/simple-line-chart

I already have implemented the expo snack on the doc, but stuck on this error.

@BssMsi
Copy link

BssMsi commented Jul 28, 2024

@mrpmohiburrahman
I am facing the same error on fresh install, were you able to resolve this?
"TypeError: Cannot read properties of undefined (reading 'XYWHRect')"

[email protected]
[email protected]
[email protected]
@shopify/[email protected]

@robwalkerco
Copy link
Member

The issue regarding XYWHRect only happens when viewing the 'web' display of the snack.

You can see in this snack https://snack.expo.dev/@robwalker/xywhrect-error-example that when switching to iOS or Android, the graph renders correctly.

This is because victory-native does not (yet?) support the Web as a target, meaning that we will always get errors when Snack tries to render for the web. This is replicable when trying to run the example app on a web target (after installing the Expo web dependencies and following the Skia Web setup docs).

We could add Snacks to the docs and set the default platform to Android or iOS, but the viewer would need to press the Launch Snack button and will likely end up in a queue to get access to a device to render the snack.

I suggest we focus on fully supporting the web as a platform before adding Expo Snacks to the docs.

@robwalkerco
Copy link
Member

I've opened a PR to fix issues with using Victory Native on the web, but it's not possible to use dependencies directly from GitHub within a Snack. For that reason, I'm currently blocked from continuing on any Expo Snack work until the PR is merged and deployed.

@mrpmohiburrahman
Copy link

I've opened a PR to fix issues with using Victory Native on the web, but it's not possible to use dependencies directly from GitHub within a Snack. For that reason, I'm currently blocked from continuing on any Expo Snack work until the PR is merged and deployed.

Will it be ok if I do the integration of expo snack when your PR get merged ?

@robwalkerco
Copy link
Member

Will it be ok if I do the integration of expo snack when your PR get merged ?

@mrpmohiburrahman We appreciate any contributions!

Once the PR is merged, I may (depending on other work commitments) start on an initial Snack integration on the Getting Started docs page immediately, just to check if it's possible! Feel free to open your own PR, though, as I may not be able to work on it immediately.

It would be great to include Snacks for all suitable docs pages eventually, so your help with that would also be amazing.

@robwalkerco robwalkerco self-assigned this Aug 14, 2024
@mrpmohiburrahman
Copy link

Feel free to open your own PR, though, as I may not be able to work on it immediately.
It would be great to include Snacks for all suitable docs pages eventually, so your help with that would also be amazing.

I can work on it full-time ( as currently, I don't have any office work ).

@mrpmohiburrahman
Copy link

mrpmohiburrahman commented Aug 15, 2024

Just noticed, the PR got merged.
I am working on implementing the expo snack.

@robwalkerco
Copy link
Member

Just noticed, the PR got merged.

@mrpmohiburrahman It's just been published to npm - https://www.npmjs.com/package/victory-native/v/41.1.0

@robwalkerco
Copy link
Member

@mrpmohiburrahman I've managed to get a working snack - https://snack.expo.dev/@robwalker/victory-native-snack-example

It's a bit fiddly, as it needs to be code-split so that Skia is not initialised until after WithSkiaWeb has loaded, so any snacks will need to have multiple files.

@robwalkerco
Copy link
Member

Another blocker :(

It's only possible to view the App.tsx file of an embedded snack. There is an existing open issue to improve this, but no progress.

For us, the App.tsx file will have to be Skia Web wrapping code. The embedded snack will not show or allow editing of the actual chart code, unless the Snack is opened on the full Expo Snack interface using the icon on the snack.

This is a screenshot of what our embedded snacks will look like
Screenshot 2024-08-15 at 11 01 57

@mrpmohiburrahman have you been able to get a working snack where the App.tsx file contains the code for a chart?

@mrpmohiburrahman
Copy link

@mrpmohiburrahman have you been able to get a working snack where the App.tsx file contains the code for a chart?

Unfortunately no,
We need to somehow place the actual code and the the App() in the same file.

The problem is the import() statement on getComponent().
Still working on it.

@robwalkerco
Copy link
Member

It's only possible to view the App.tsx file of an embedded snack.

I've opened a PR to add a feature to Expo Snack which will allow us to specify the file to show in the snack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request issue-accepted The issue is confirmed and accepted by the maintainers
Projects
None yet
Development

No branches or pull requests

5 participants