[feat] create-svelte: Add prompt for choosing adapter #2479
Conversation
🦋 Changeset detectedLatest commit: d562724 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 |
packages/create-svelte/bin.js
Outdated
| const dest = path.join(cwd, file.name); | ||
| mkdirp(path.dirname(dest)); | ||
| fs.writeFileSync(dest, file.contents); | ||
| } | ||
| }); | ||
|
|
||
| if (options.adapter) { | ||
| pkg.devDependencies[options.adapter.npm] = 'next'; |
There was a problem hiding this comment.
Should the version be the actual value of the latest release instead of the tag?
People installing like this would get left behind once adapters are released as 1.0 which switches to latest tag.
As a workaround for having to pull every adapter version for now we could use "^1.0.0-next.0" which should match all releases of the adapters including "1.x.y". It won't match new majors after 1 eg "2.x.y" or new prereleases after 1.0.0 eg "1.1.2-next.1"
There was a problem hiding this comment.
Another, more general thing: right now the logic for modifying the files is different and works by overwriting files using the files in https://github.com/sveltejs/kit/tree/master/packages/create-svelte/shared . So the overwrites should be added there, and there's also likely some other logic in bin.js
There was a problem hiding this comment.
@dummdidumm I tried using the shared files for this but would end up making 10 different svelte.config.js and that would make future changes more difficult. Can use it for adding adapter to devDependencies but the current logic will still be needed if we want to include community adapters as discussed here #2459 (comment)
|
I approve the change but not its current implementation, at least not yet. It's very different to what is present and having two different styles to add prompts is confusing. Honestly I was never a big fan of the refactoring to use these folders, which now shows as it would result in many more folders needed to be created. I'm okay to merge this after we agree on how to refactor this into a consistent approach. |
benmccann
changed the title
|
I think the folder-based approach is great, but the bottleneck now is merging |
|
If we want to provide an option to choose from a list of community adapters fetched from an API, we will still need some runtime logic outside the folders similar to the current implementation. |
|
I don't think we would provide an option for community adapters from an API in the future, since it implies something that we endorse but we don't have control of it. (Though that's just my opinion) Nevertheless, I think this is safe to merge as is. Per dummdidumm comment, the refactoring we can do after this IMO is to build a way to merge svelte config files to use to folder based structure instead. But I'll see if the other maintainers have an opinion on this. |
|
We ended up going in a different direction — #2867 — so I'm going to close this. Thank you! |
Fixes #2459
Add a new prompt for adapter. The default option is
"I'll add it later or use a community-provided adapter".Updated:
Before submitting the PR, please make sure you do the following
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpx changesetand following the prompts. All changesets should bepatchuntil SvelteKit 1.0