-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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(create-vite): add qwik templates #13620
Conversation
Run & review this pull request in StackBlitz Codeflow. |
From what i see: the |
I don't think a QwikCity template fits here? Similarly we don't add the SvelteKit template here. At most it's a vanilla Vite + Qwik template similar to the other existing frameworks setup and design. And then we can add a CLI redirect to QwikCity. |
I get it! I changed the starter app and also I added the option to QwikCity like SvelteKit. @manucorporat WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is multiple things that are off compared to others templates (not an issue but need to be justified):
- lib mode
- ssr entry
- tsconfig/.gitignore diverge from other templates
@patak-dev @ArnaudBarre core qwik team here! worked last week to make sure qwik's CSR is in pair with the rest, 100% standard vite starter, |
@leifermendez think we should remove the .eslintrc.cjs |
Thanks @manucorporat, I think we should also remove Also, you could add information on the template We were discussing with @bluwy that with the previous setup that included low-level SSR handling, it was better to merge this into create-vite-extra. But with csr, at least to me, create-vite is a better place. Just to set the expectations, create-vite templates are barebone and mainly intended to try out Vite (or do bug repros). We always point users to prefer the create starters from the frameworks when possible. |
Done! @patak-dev removed the files as suggested and added the |
Added READMEs! |
Are the |
There is an extra |
Not really, i misunderstood u, thought u wanted the eslint added! removed |
width={32} | ||
height={32} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The templates prompts the user "Edit |
HMR is not implemented in Qwik CSR yet, CSR never been a focus. Fixed the image size and remove that prompt to the user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now! Thanks for the changes.
The changes look great and follows the convention! I have a small question but not 100% blocking this:
I wonder if this CSR setup is something you'd recommend building with Qwik today? If not, it feels a bit weird keeping a template for it. In most docs it looks like QwikCity (or an SSR setup in general) is the preferred starting point. |
There are uses cases! Qwik is equivalent to Vue, Solid, React, (same group). It dont need server, we are coming up with new uses cases for CSR, like mobile apps (Capacitor), Qwik automatic lazy loading is still interesting even without SSR involved! Regarding HMR i plan to work on this soon, for CSR and improvements for SSR too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. If there are plans for improving CSR, let's get this in 👍
pr-vite-qwik-convert.mp4
Description
A new template has been added for Qwik Framework
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).