Skip to content

Using sequence means I need to include @sveltejs/kit in my Node Docker image #3709

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

Closed
Karlinator opened this issue Feb 3, 2022 · 7 comments

Comments

@Karlinator
Copy link
Contributor

Karlinator commented Feb 3, 2022

Describe the problem

I noticed when trying to slim down my Docker images that if I kept only the runtime dependencies after building (which I list in dependencies and install with npm ci --only=prod) my container would immediately crash because it Cannot find package '@sveltejs/kit' imported from /usr/srv/build/server/app.js.

It turns out that when you use sequence() for hooks, the generated app.js file includes import { sequence } from '@sveltejs/kit/hooks.

Describe the proposed solution

I'm not sure how, but it would be nice if we copied over the relevant code (the 26 lines from https://github.com/sveltejs/kit/blob/master/packages/kit/src/hooks.js) in the build or something so I don't have to include the entirety of SvelteKit in my runtime.

Not sure if this is Kit or the adapter.

Alternatives considered

I can just live with it. It's just ~2MB of the total image size.

Importance

nice to have

Additional Information

No response

@Conduitry
Copy link
Member

This sounds like it's essentially the same as #3176.

@Karlinator
Copy link
Contributor Author

Almost entirely (I didn't see it when searching). It would solve this, but I guess it is possible to solve this without also solving #3176 (this is essentially a narrower issue I encountered while working around that issue).

@Rich-Harris
Copy link
Member

https://kit.svelte.dev/docs#modules-sveltejs-kit-hooks is an anomaly in that it's the only SvelteKit-provided module that stays in your node_modules rather than essentially become part of your source code (which we do because the behaviour of functions like goto depends on the structure of your app). One way to fix this would be to turn it into $app/hooks or something instead

@Conduitry
Copy link
Member

I don't think that would be the correct fix, because I think the correct fix would be to address #3176 and not force people to do that in userland. Dev dependencies of your app shouldn't be required in production, and the build should take that into account when bundling stuff. Moving this into $app just so that we can avoid addressing that other issue seems like the wrong solution.

@Karlinator
Copy link
Contributor Author

Karlinator commented Feb 3, 2022

I checked quickly and what adapter-cloudflare does is buindle this little snippet into the worker script, along with every other runtime dependency (so what #3176 wants to do with adapter-node). I think that's a much better overall solution, removing the need for any finicky dependency management like I currently do in my Dockerfile to try to not keep any unneeded dependencies.

Moving sequence to $app/hooks wouldn't be the worst idea anyway—as you say it's the only part of SvelteKit runtime code that stays in node_modules.

@silentworks
Copy link

I ran into this issue today. I've for now moved my @sveltejs/kit to be a dependency to solve the issue for now.

@Rich-Harris
Copy link
Member

Going to close this in favour of #3176, as the solution to that would cover this issue. I don't think there's much mileage in applying a narrower fix here (though now that I think about it, @sveltejs/kit is now always marked as noExternal when building, so this is probably already fixed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants