-
Notifications
You must be signed in to change notification settings - Fork 131
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
feat: remove dev node_modules deps #242
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
if (options.minify) { | ||
removeNodeModule(path.join(outputPath, "node_modules"), [ | ||
"@esbuild", | ||
"prisma/libquery_engine-darwin-arm64.dylib.node", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we include all prisma engines?
meaning all other OS packages not compatible with Lambda
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know where I can get a list of all non-lambda engines?
I might need to update this and use micromatch
to allow for negation...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some generic prisma node_modules cleanup code https://github.com/jetbridge/sst-prisma/blob/master/stacks/resources/prismaLayer.ts#L90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, what the hell is wrong with prisma?
// "react", // TODO: remove react/react-dom when nextjs updates its precompile versions | ||
// "react-dom", | ||
"@webassemblyjs", | ||
"uglify-js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate here, it's also listed above!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, I'll have to retest this w/ Next 14, which seems to have fixed the dev deps getting traced to the standalone output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Been busy last week and caught a cold this week... I'll eventually test this to confirm... @mathisobadia was still seeing these dev modules but I didn't...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@revmischa This is inside your server-function lambda ? There is a lot of things that you should be able to remove, you can use experimental.outputFileTracingExcludes
from next config to remove all of those.
You should probably create an issue in next repo as well, those dev deps should have been removed.
@khuezy The docs mention that next 14+ doesn't have the issue anymore but I that doesn't seem to be the case for me. So this PR is still valuable |
Any progress on this? |
Not much, for now, you can try using
It's hard to know which to remove so it might be better for the user to manage that. For example, if you add "watchpack", it'll break server actions even though it looks like a dev dep. |
I'm trying to use this but I get a "RangeError: Maximum call stack size exceeded" error: vercel/next.js#42641 (comment) |
Something change in the later version of next (not sure if bug or by design.) Using the "**" causes an infinite loop. You have remove the wildcard. |
Hm, I tried
And get the same error |
Maybe im doing something weird here, but on nextjs 14.1 and pnpm ... I've tried to manually remove the node-modules from the server function you have listed above and my lambda fails pretty hard.
|
Did you remove the entire folder? You should only remove the dev subfolders, not the entire node_modules |
|
||
if (options.minify) { | ||
removeNodeModule(path.join(outputPath, "node_modules"), [ | ||
"@esbuild", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very likely yes, but you'll need to test for yourself. You can probably remove :
swc core
sharp
esbuild
caniuse lite
uglify-js
terser
nextjs v14.2.4 / open-next 3.0.2 -- still includes dev dependencies in This next config works and excludes unwanted dependencies (note: the setup for monorepo):
|
fixes: #166
This reduces my zipped lambda from 31MB => 10MB
I investigated the use of next.config's
outputFileTracingExcludes
but that is hard to merge w/ the user's config file since the config isjs
and may contain functions - which we can't easily stringify. Doing a complex regex to replace is very tricky and brittle if the user's config file is non standard too...So the solution is to do the node_modules cleanup AFTER next has built.