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

Solid Integration #469

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Solid Integration #469

merged 2 commits into from
Oct 31, 2024

Conversation

rokotyan
Copy link
Contributor

A clone of #422, but rebased from main instead of merging. Let's see if the build passes.

@rokotyan rokotyan force-pushed the feature/solid branch 7 times, most recently from dca7a0e to f097a8f Compare October 25, 2024 17:19
@rokotyan
Copy link
Contributor Author

@hngngn @lee00678

I was able to make the solid build work, but the website build is failing because it can't find the examples for some reason. If you can take a look into this, it would be great.
image

@rokotyan
Copy link
Contributor Author

There is one more problem with the build I've found. All Unovis libraries preserve the source directory structure in the dist, making it possible to import components individually, like import { VisAnnotations } from '@unovis/react/components/area':
image

The Solid build doesn't have that, and it seems to be bundling all of @unovis/ts plus its dependencies. This will have a negative effect on the final bundle size of the apps that use @unovis/solid.
image

I think you can take a look at the vite.config.ts from packages/vue and use it as an example of the build configuration.

@rokotyan rokotyan changed the title [Test] Solid Integration Solid Integration Oct 25, 2024
@lee00678
Copy link
Collaborator

@hngngn @lee00678

I was able to make the solid build work, but the website build is failing because it can't find the examples for some reason. If you can take a look into this, it would be great. image

Yea, this happened to me during my test a couple of days ago as well. (#475 (comment)) Seems to be this is caused by solid, so we may need some insights from @hngngn

@hngngn
Copy link
Contributor

hngngn commented Oct 25, 2024

Sorry for the late reply—I've been pretty busy lately. I'll look into the issue and report back ASAP.

@hngngn
Copy link
Contributor

hngngn commented Oct 28, 2024

I've already pushed a fix for both problems you're facing. Can you test it? @rokotyan @lee00678

- Upgrade Node.js version to 20.x
- Upgrade npm version to 10.x
- Update npm installation commands accordingly
@lee00678
Copy link
Collaborator

Thanks for the fix @hngngn. Looks like all checks passed. @rokotyan, should I go ahead and merge this PR, then close #422?

@rokotyan
Copy link
Contributor Author

@lee00678 Looks good, I think we can merge it

Thanks @hngngn!

@hngngn
Copy link
Contributor

hngngn commented Oct 28, 2024

@rokotyan I'm having trouble with the gallery right now, and the last time I checked, the example of Grouped Bar with Brush and Interactive Legend was broken. I'll report back if everything is working properly

@hngngn
Copy link
Contributor

hngngn commented Oct 29, 2024

I've pushed a fix. Could you test it?

@lee00678
Copy link
Collaborator

@rokotyan do you mind pushing the latest fix so I can do a last round of testing before merging?

@rokotyan
Copy link
Contributor Author

@lee00678 Done. Amended the main commit and did a force push.

@lee00678
Copy link
Collaborator

I think you can take a look at the vite.config.ts from packages/vue and use it as an example of the build configuration.

This looks good!

@lee00678
Copy link
Collaborator

@hngngn Thanks for this contribution! This will be released in 1.5.0.

@lee00678 lee00678 merged commit 589d4c2 into f5:main Oct 31, 2024
4 checks passed
@lee00678 lee00678 mentioned this pull request Oct 31, 2024
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.

3 participants