serverless adapters appear to be relying on @sveltejs/kit being present in runtime dependencies

[antony]

Describe the bug
A difficult one to track down this, but I've noticed that in the:

• netlify
• begin (from the PR)

adapers, but not the node adapter, after build and adapt the built app.js, there is a require statement for @sveltejs/kit which, afaik, shouldn't be a runtime dependency.

Logs

2021-01-14T09:39:43.520Z	undefined	ERROR	Uncaught Exception 	{"errorType":"Runtime.ImportModuleError","errorMessage":"Error: Cannot find module '@sveltejs/kit/dist/renderer'\nRequire stack:\n- /var/task/node_modules/@architect/shared/app.js\n- /var/task/index.js\n- /var/runtime/UserFunction.js\n- /var/runtime/index.js","stack":["Runtime.ImportModuleError: Error: Cannot find module '@sveltejs/kit/dist/renderer'","Require stack:","- /var/task/node_modules/@architect/shared/app.js","- /var/task/index.js","- /var/runtime/UserFunction.js","- /var/runtime/index.js","    at _loadUserApp (/var/runtime/UserFunction.js:100:13)","    at Object.module.exports.load (/var/runtime/UserFunction.js:140:17)","    at Object.<anonymous> (/var/runtime/index.js:43:30)","    at Module._compile (internal/modules/cjs/loader.js:1015:30)","    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1035:10)","    at Module.load (internal/modules/cjs/loader.js:879:32)","    at Function.Module._load (internal/modules/cjs/loader.js:724:14)","    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)","    at internal/main/run_main_module.js:17:47"]}

Build Output

"use strict";Object.defineProperty(exports,"__esModule",{value:!0}),require("@sveltejs/kit/dist/renderer");

To Reproduce
I have a simple repro project which I can provide access to if required, but you can also just create a new kit project, use the netlify adapter, and inspect the first line of app.js after build and adapt

Expected behavior
I expect this dependency to be bundled, I suppose.

Information about your SvelteKit Installation:

• Latest everything
• Netlify adapter, Begin adapter

Severity
It won't run once deployed: https://staging.kezia.ws/

[benmccann]

@antony I can't reproduce this with the Netlify adapter

I ran svelte-kit build && svelte-kit adapt in examples/hn.svelte.dev which uses @sveltejs/adapter-netlify. I don't see an app.js, but I can't find anything importing @sveltejs/kit/dist/renderer. I ran grep -r sveltejs/kit/dist/renderer build/ and it returns nothing

[antony]

I'll try again and create a repro if I can. It's possible that it has been fixed? Thanks for having a look!

[antony]

Hey @benmccann

I just ran it on a new svelte-kit app and here's the contents of build/app.cjs:

"use strict";Object.defineProperty(exports,"__esModule",{value:!0}),require("@sveltejs/kit/dist/renderer");var e=require("./chunks/app.cjs");exports.init=e.init,exports.render=e.render;
//# sourceMappingURL=app.cjs.map

[antony]

If you use the netlfy adapter, then it's functions/render/app.cjs you need to grep:

"use strict";Object.defineProperty(exports,"__esModule",{value:!0}),require("@sveltejs/kit/dist/renderer");var e=require("./chunks/app.cjs");exports.init=e.init,exports.render=e.render;
//# sourceMappingURL=app.cjs.map

[benmccann]

Ah, I was expecting it to be under build, so I missed it.

It looks like that's added here:

kit/packages/kit/src/api/build/index.js

Line 250 in dc4ea38

import * as renderer from '@sveltejs/kit/dist/renderer';

And made external here:

kit/packages/kit/src/api/build/index.js

Line 376 in dc4ea38

external: (id) => id[0] !== '.' && !path.isAbsolute(id)

I personally prefer having most of my dependencies be external on the server because then I get the original source code and line numbers without having to futz with sourcemaps which always seem like a bit of a pain to get correctly configured. E.g. to setup Sentry with source-mapped code you need to make sure to ship the updated source maps to Sentry with each deploy. I know a couple of the other folks prefer having their dependencies bundled in order to have a smaller deployable, but I've never thought it to be worth it with developer time being expensive and disk space being cheap. I'm not that familiar with Lambdas, but I believe they handle external dependencies just fine.

@samccone's request to get at the Rollup configuration is somewhat related to this as well. If we had a hook then different adapters might be able to do this differently. It seems like this is something that could be user configurable

[antony]

Ah nice - thanks for that!

To be honest, it's completely core to how Svelte Kit works so I can't see any value in making people install it. The source mapping is nice, but, and I also use Sentry, if even one part of your codebase needs sourcemaps, then you need to have sourcemaps, and it's unlikely there'd be a production app which doesn't bundle/minify/transpile and thus need sourcemaps.

With regards to making it user configurable, I'm not sure this would be a great idea, it adds complexity and I don't know who would configure it.

There is also the fact that each adapter needs a slightly different approach:

begin
If you add a package.json in the lambda directory, begin will automatically install any required dependencies

vercel
installs and bundles everything into a compiled package for the lambda

netlify/static
I have no idea, but the current status would be that the user would have to know about the dependency requirement and manually install it / deal with it.

[benmccann]

It looks like Netlify also installs the dependencies based off the package.json. I don't think this comes into play for the static adapter since there's no server-side component in that case.

It's pretty common for server-side libraries to be bundled or transpiled, but I rarely see them minified. That means I usually can get away without sourcemaps on the server-side. Of course, they're still needed for the client-side, so you can't really get away from it forever. But it's often enough to let you kick the can down the road or only have to worry about it for the client and not the server.

Anyway, I'd be fine bundling here as long as we don't minify. That still makes the code fairly readable without sourcemaps and probably easier to deploy for most users.

[antony]

Ok I might have to try each adapter with a dependency and see if that resolves the issue for each platform. How we ensure that users have this dependency listed might be a different issue.

[benmccann]

I started looking at this and think if we wanted to bundle then we'd need to include @rollup/plugin-node-resolve and @rollup/plugin-commonjs when bundling:

kit/packages/kit/src/api/build/index.js

Line 361 in dc4ea38

plugins: [

Also, I noticed that client dependencies also look to be externalized, which seemed surprising to me:

kit/packages/kit/src/api/build/index.js

Line 198 in dc4ea38

external: (id) => id[0] !== '.' && !path.isAbsolute(id)

Hopefully @Rich-Harris can weigh in with some thoughts here

[benmccann]

Hmm. So I tried moving a few devDependencies to dependencies and then deploying on Netlify. It didn't work for me:

6:47:19 AM: [snowpack] No ESM dependencies found!
6:47:19 AM: At least one dependency must have an ESM "module" entrypoint. You can find modern, web-ready packages at https://www.skypack.dev

We may have to bundle for compatibility with Snowpack

[Rich-Harris]

We definitely do want to bundle, otherwise your lambdas will have an indirect dependency on things like Snowpack. Putting it in adapters' devDependencies rather than dependencies should prevent Netlify etc installing stuff unnecessarily.

The alternative is to split the renderer and utilities out into a package separate from kit