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

feat: new extension vike-solid-query #111

Merged
merged 10 commits into from
Sep 4, 2024

Conversation

phonzammi
Copy link
Member

@phonzammi phonzammi commented Sep 1, 2024

@magne4000 @brillout

Please take a look and review this PR.

Note that I haven’t added any tests for vike-solid-query yet because we need to add some code to vike-solid to detect crawlers and bots, and then use renderToStringAsync() for that. Also a bit of refactor.

As Rom suggested, maybe we could merge this since we've refactored to a monorepo and continue with the remaining work in a separate PR.

closes #94

- Moved the `vike-solid` project from the root directory to `packages/vike-solid`.
- Updated scripts and dependencies to align with `vike-react` and ensure proper functionality.
- Verified build and test processes to confirm successful migration.
- Updated `README.md` in the root directory and `packages/vike-solid` directory to reflect the new project location.
@phonzammi phonzammi force-pushed the phonzammi/feat-vike-solid-query2 branch from 3ff0694 to 3091389 Compare September 3, 2024 16:39
@phonzammi phonzammi marked this pull request as ready for review September 3, 2024 17:01
Copy link
Member

@magne4000 magne4000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few typos to fix. It also needs to be updated from main branch. After that, we are good to merge!

packages/vike-solid-query/README.md Outdated Show resolved Hide resolved
packages/vike-solid-query/README.md Outdated Show resolved Hide resolved
@phonzammi
Copy link
Member Author

@magne4000, LGTM ?

@brillout, we’d love to hear your thoughts on this.

@magne4000
Copy link
Member

As Rom suggested, maybe we could merge this since we've refactored to a monorepo and continue with the remaining work in a separate PR.

Let's just merge and continue the work on another PR 👍

@phonzammi phonzammi merged commit 2ddbc50 into main Sep 4, 2024
5 checks passed
@phonzammi phonzammi deleted the phonzammi/feat-vike-solid-query2 branch September 4, 2024 15:17
@brillout
Copy link
Member

brillout commented Sep 4, 2024

I quickly glazed over the readme; looks good. I wonder if it's possible to have the interface be more like vike-react-query's withFallback(), but I guess it's good as it is.

FYI there are couple of typos, inconsistent code formatting, and markdown formatting errors in the readme.

Nothing much changed on vike-solid's side, so there isn't much for me to object.

LGTM.

@brillout
Copy link
Member

brillout commented Sep 4, 2024

Also, the main readme needs to be updated.

I've aded vike-solid-query to Vike's docs: vikejs/vike@9711fbb 🚀

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.

New extension vike-solid-query?
3 participants