chore: add prepare script to run svelte-kit sync

[benmccann]

closes #388

see also sveltejs/kit#13304

I went with preprepare as I believe it runs on solely on local installs (not installs from registry. though perhaps installs using git). Had previously been thinking about prepare, but that also runs on publish and we don't need to do it then

marked as draft as this currently only works on npm: pnpm/pnpm#8987

[changeset-bot]

🦋 Changeset detected

Latest commit: a76df1a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
sv	Patch

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

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/sveltejs/cli/sv@409

npm i https://pkg.pr.new/sveltejs/cli/svelte-migrate@409

commit: a76df1a

[Conduitry]

preprepare scripts are also run when installing only prod dependencies (npm ci --omit=dev), as one would do in a multi-stage Dockerfile. This script will fail if the svelte-kit binary from the SK dev dependency isn't present, which will break the build.

My best suggestion currently is to add a fallback so the preprepare script always succeeds, even if the svelte-kit binary is missing. I think svelte-kit sync || echo would be a cross-platform way to do that.

[Conduitry]

A suggestion from @dominikg was to just keep using prepare rather than preprepare. If the concern was merely avoiding doing unnecessary work, I wouldn't worry about that, as the sync is super fast.

pnpm also provides an option to skip pre/post scripts when running the main script, which would break this if someone disabled it. To not run them was previously the default in pnpm 8, but it was changed to run them in pnpm 9 - but given that this possibility is out there, it makes sense to avoid the issue by just using prepare.

[benmccann]

My best suggestion currently is to add a fallback so the preprepare script always succeeds, even if the svelte-kit binary is missing. I think svelte-kit sync || echo would be a cross-platform way to do that.

I guess we only need to do that for the app templates and not the library template?

[Conduitry]

@benmccann If someone's deploying the embedded demonstration/documentation site for a library, I think they could conceivably have a situation where they install only prod dependencies. I think it's safest to just include the || echo in that template as well.

[benmccann]

For historical context, it looks like you suggested this solution a few years ago: sveltejs/kit#4585. It seems there was some debate then - does it need to be echo '' or is echo fine?

[benmccann]

Looks like the tests are failing because of Storybook 😝

[Conduitry]

I thought I had had this discussion before! Thanks for digging up the issue.

I'm not sure what to suggest around echo vs echo ''. I can confirm that echo in PowerShell waits for another argument, but I also see in that issue my comment about trying this in PowerShell and having the inner script run in cmd.exe. But I also also see that @mrkishi said it caused an issue for him.

The thing with echo '' is that it will actually print out the '' on cmd.exe, which is not the end of the world. This does suggest the possibility of echoing out a potentially more helpful message, rather than echoing out nothing. PowerShell does still want the string in quotes (otherwise it prints each word on its own line), which means that cmd.exe would actually then print those quotes. But that's probably also not the end of the world. And 'Some helpful message here' is less weird looking than bare ''.

[dominikg]

what about || cd .

[Conduitry]

Oh, cd . is another reasonable suggestion. That's going to look super weird and inexplicable in the package.json though - especially since we can't put comments in there. Something like "prepare": "svelte-kit sync || echo 'Sync failed - continuing'" would at least make it a little clearer what's going on.

[benmccann]

Something like "prepare": "svelte-kit sync || echo 'Sync failed - continuing'" would at least make it a little clearer what's going on.

I wonder if that message will be confusing to users who see it in their Docker logs and aren't sure what the implications of that are and what they should do in response. I tend to think that being silent might be a little less scary since there's nothing we actually need to make the users aware of.

[Conduitry]

This looks good to me!

[hyunbinseo]

Maybe esbuild should be added to pnpm.onlyBuiltDependencies?

pnpm dlx sv create
corepack use pnpm@10 # no warning message.

rm -rf node_modules
pnpm i # logs the following warning message:

# The following dependencies have build scripts that were ignored: esbuild
# To allow the execution of build scripts for these packages, add their names to "pnpm.onlyBuiltDependencies" in your "package.json", then run "pnpm rebuild"

┌  Welcome to the Svelte CLI! (v0.6.15)
│
◇  Where would you like your project to be created?
│  ./
│
◇  Which template would you like?
│  SvelteKit minimal
│
◇  Add type checking with Typescript?
│  Yes, using Typescript syntax
│
◆  Project created
│
◇  What would you like to add to your project? (use arrow keys / space bar)
│  none
│
◇  Which package manager do you want to install dependencies with?
│  None