-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Provide ability to bundle dev dependencies for Node adapter #3176
Comments
So there used to be an @Rich-Harris was this an unintended side-effect of that? I can't see it mentioned in the PR or discussion. |
I ran into a similar issue to @Conduitry since upgrading and taking inspiration from this thread I added my package dependencies manually to the A path forward would be a welcome discussion. EDIT: This was actually an issue with my build pipeline. We were copying the output directory of the Node adapter into a Docker image thinking it would provide a fully working application. I think what was going on is because certain packages were being externalized I needed to also copy the node_modules folder into the image. |
Hello, I agree with @Conduitry that it could be interesting to have an option to bundle dev dependencies when using node adapter. The workaround is working well but an option would be simpler to use. I think it should not create a single entry file (like before) but bundles only dev dependencies as suggested by Conduitry. |
Waiting for this issue to be fixed sveltejs/kit#3176
As of the latest versions of SvelteKit - with a separate |
Doing this makes the system harder to understand in my opinion because it makes dev different than prod (though perhaps that ship has sailed) and SvelteKit different from Vite. It also adds another way in which If the adapters were Vite plugins (or could provide Vite plugins) then we could at least make this functionality available with a single option like |
Yeah, this is really annoying. Currently my docker image is 1.5G and could be 150-200M if everything was bundled |
FWIW, we presently use a // File: scripts/postbuild.js
// In package.json: scripts.postbuild = "node scripts/postbuild.js"
require('esbuild').buildSync({
entryPoints: ['./build/index.js'],
outfile: './build/index.bundle.js',
format: 'esm',
target: 'esnext',
platform: 'node',
// When outputting ESM, any require calls will be replaced with the __require
// shim, since require will not be available. However, since external paths
// will not be bundled, they will generate a runtime error. This matters only
// for built-in node modules.
// https://github.com/evanw/esbuild/pull/2067#issuecomment-1152399288
banner: {
js: `
import { createRequire } from 'module';
import { fileURLToPath as urlESMPluginFileURLToPath } from "url";
import { dirname as pathESMPluginDirname} from "path";
const __filename = urlESMPluginFileURLToPath(import.meta.url);
const __dirname = pathESMPluginDirname(urlESMPluginFileURLToPath(import.meta.url));
const require = createRequire(import.meta.url);
`,
},
bundle: true,
}); |
With the now-latest version of SvelteKit, My new workaround is something that I think did not used to be possible in Vite 2 but which is now possible in Vite 3. Rather than telling Vite which dependencies to bundle, I tell it to bundle all dependencies, but exclude the ones that are prod dependencies of the app: import { sveltekit } from '@sveltejs/kit/vite';
import { readFileSync } from 'fs';
const pkg = JSON.parse(readFileSync('package.json' ,'utf8'));
export default ({ command }) => ({
plugins: [sveltekit()],
ssr: command === 'build' && {
external: Object.keys(pkg.dependencies),
noExternal: true,
},
}); |
Issue #5944 was marked as duplicate, yet I found out recent version of sveltekit and adapter-node makes the build dependent on the |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
I think the process of build need to add adapter node dependencies in production. It' very confusing if every time this dependencies was changed, we would need to start the app in production to discover that some packages is not there. |
Opened a PR that adds @Conduitry's solution to the default Vite config #6301 |
@fmaclen can you share a reproduction on the latest SvelteKit? I just tried @Conduitry's suggestion and it works fine for me |
@benmccann just tried @Conduitry's solution on my project again (with the latest version and the one I was using when I posted my comment) and both work as expected so the error was probably caused by some other issue specific to my repo 👍 |
If I install devalue, when run builded occurs:
If I not install, the App don't work because it don't find devalue. Would someone help me? |
@moisesbites you should pin the devalue version to 2.0.1 (last version is 3.0.0 but the upgrade as not yet been done in kit) here are the version that I used with a successful build yesterday :
|
@vboulaye Thank you! It would be nice for the adapter to keep the libraries up to date with the latest versions, as for now the build installation has been delegated to the developer at the end. That would be less confusing, at least for now. |
* bundle non-prod dependencies with esbuild - closes #3176 * changeset * update README
This should be good to go now in |
Describe the problem
Currently, the Node adapter will not bundle any external server side dependencies. This means that you need to make all of the dependencies you might need at runtime be prod dependencies. This differs from its (recent) previous behavior, and from Sapper's behavior, and from (I would argue) expected behavior.
Until now, it's been standard operating procedure to bundle as many of the server-side dependencies as possible, to reduce the deployed app size, parse time, and startup time. If there's some good reason to stop doing this, it should certainly at least be documented.
This issue may also well apply to other adapters (other than the static one), but I haven't tried to validate that.
Describe the proposed solution
I'm not entirely sure. It would be nice to be able to provide sensible default Vite configuration so that we have Vite do it, rather than do this in a separate esbuild step afterwards (either automatically or manually).
I don't know whether some different behavior from Vite would make this easier on our end, or whether this is something that Vite ought to change.
Below, I describe what I'm currently doing in userland to deal with this, but I'm not positive that this would have an equivalent in-framework analogue. (Do we have convenient access to the user's project's
package.json
?)Alternatives considered
Currently, I have a
vite.ssr.noExternal
setting ofprocess.env.NODE_ENV === "development" ? undefined : Object.keys(pkg.devDependencies)
(wherepkg
is the parsed JSON frompackage.json
) to force Vite to bundle all of my dev dependencies.The check for development vs prod mode is there because I was having an issue using
Object.keys(pkg.devDependencies)
in dev mode that I haven't fully tracked down or tried to reproduce in a minimal repo, but which seem to stem from having a CJS-only dependency that Vite doesn't try to convert to ESM during dev mode.Importance
would make my life easier
Additional Information
No response
The text was updated successfully, but these errors were encountered: