-
-
Notifications
You must be signed in to change notification settings - Fork 47
feat(cli): sv create --from-playground
#662
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 188c9e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
This is exciting 🤩 I have this one that could show a few things: https://svelte.dev/playground/770bbef086034b9f8e337bab57efe8d8
![]() +layout.svelte example<script lang="ts"> import favicon from '$lib/assets/favicon.svg'; |
This is perfect, and i was able to get it fully working from the tests.
Not sure, I'm a fan of this. The current "download app" button does nothing like this. Also, I'm not sure if this provides any additional value to the user, or just causes styling problems because of the added css. We can always add this later if users want this. |
Great progress 🚀🚀🚀 Nice
The cool thing about a layout like this is that:
Yes, I love this idea 😜 Anyhow, very nice job |
Updated pr description, this is working now. Let me know what you think. |
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.
Comments in random order:
- I find it strange to have the "Template content" coming from a playground. Maybe
+page.svelte
should have ONLY<App />
, nothing else ? - I still like the playground layout idea :p
- Maybe a default folder name could be proposed "svelte-playground-slug-name-of-playgound" ?
- Maybe the
readme.md
could mention the origin of the playground ? - If some extra deps are needed, prompt it in CLI and add it to readme.md ?
It's probably nitpicking, because it's working well already.
Great work 🎉🎉🎉
import * as js from '@sveltejs/cli-core/js'; | ||
|
||
export async function write_playground_files(url: string, cwd: string): Promise<void> { | ||
if (!validatePlaygroundUrl(url)) throw new Error(`Invalid playground URL: ${url}`); |
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 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.
Maybe even not exiting the cli
when this happen to have the chance to paste a valid url
?
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.
This exact one should only trigger when you call create
programmatically. But you are absolute right, this should be fixed.
Closes #602
You can now run
pnpx https://pkg.pr.new/sveltejs/cli/sv@662 create --from-playground=https://svelte.dev/playground/hello-world
. This is currently doing more or less the same the stuff theDownload App
button in the playground is doing. Apart from that, you can choose additional addons,ts
vsjs
and install deps. External dependencies are auto-detected and installed into thepackage.json
Todos:
svelte
-?version=
parameter from the playground urldominikg
: one thing to remember is that this is untrusted input basically, the playground could reference malicious packages, so installation with --ignore-scripts and making users aware that they have to check the content beforehand would be good.