-
Notifications
You must be signed in to change notification settings - Fork 143
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
Can not follow nft's reason why to include some dependencies #203
Comments
Ah, just remembered that we had a theory how it gets from Nexus Plugin Prisma to the postinstall hook, but we could not confirm that: https://github.com/graphql-nexus/nexus-plugin-prisma/blob/fce0ffb5270007124444a36365691bf085538095/src/index.ts#L62 This reads the |
Great write up, you've followed exactly the right steps here it seems @janpio. I think the reason for this is the logic in To fix this in nexus-plugin-prisma, you could similarly use eval to hide the |
Can we have a built-in option to run a script like @janpio did to output the reasons into some readable format so that we can figure out what the root of an introduced file is at a glance? for example: {
"reasons": {
"entry.js": {
"node_modules/x/index.js": {
"node_modules/x/util.js": { /* ... */ }
"node_modules/x/app.js": { /* ... */ }
// ...
},
"node_modules/y/index.js": { /* ... */ }
// ...
}
}
} |
@guybedford Thanks for the analysis, I will try to confirm that experimentally. Really want to understand this one. This also gave me an idea: Would it be possible for |
Thanks again @guybedford and @styfle, I could confirm that the line you pointed out indeed triggered this behavior. Wrapping it in prisma/prisma#6932 (comment) <3 |
Thanks @janpio, your investigation helped us a lot. In case someone wants to use the original script now, here's version translated for Map/Set that const { nodeFileTrace } = require("@vercel/nft");
(async () => {
const files = ["./.next/server/pages/api/test1.js"];
const { fileList, reasons } = await nodeFileTrace(files);
async function traceReasons(path) {
let reason = reasons.get(path);
while (reason) {
console.log(reason);
const [parent] = reason.parents;
if (reason.parents.size > 1) {
console.warn("More than one parent, choosing first one");
}
reason = reasons.get(parent);
}
}
console.log("----------");
traceReasons("node_modules/@sentry/cli/sentry-cli");
})(); |
This feature was added in #282 using |
A recent Prisma release lead to multiple people reporting problems with deploying their projects to Vercel, although we have end to end tests of projects that are deployed to Vercel - and those worked just fine with this version of course, or we would not have released it.
We could fortunately pinpoint the commit that introduced the problem (as we publish each commit as a
dev
version ofprisma
) and understood that arequire.resolve('@prisma/engines')
lead tonft
treating this whole package as required and included it in the archive.@prisma/engines
usually is only needed by Prisma CLIprisma
, and not by@prisma/client
which should be included in the serverless function. As the Engines package is really big, that was very bad. We could fix this problem by replacing this line witheval("require.resolve('@prisma/engines')")
fortunately asnft
does not "follow" this.But that did not explain why this only happened in some user projects, and not our end to end test projects 😕
So we looked at the user's project and finally noticed a pattern: They all use Nextjs with Nexus and Nexus Plugin Prisma. Our end to end tests do not. Based on that information we created a minimal reproduction repository that fails to deploy to Vercel because of the archive size: https://github.com/janpio/prisma-limit-repro
This is what I see when we try to deploy that repo:
After checking out the reproduction repository, and running
npm install
,npm run generate
andnpm run build
, finallynpm install -D @vercel/nft
, I can output the list of files thatnft
picks withnpx nft print ./.next/server/pages/api/graphql.js > nft-print.log
- and indeed the wholenode_modules/@prisma/engines
is included:I then used this small script look into why one of those binaries in
@prisma/engines
are included:This outputs something similar to:
Now we already know that
node_modules/prisma/build/index.js
is related to our bug - there we had the misguidedrequire.resolve('@prisma/engines')
. But I could not figure out how we go fromnode_modules/nexus-plugin-prisma/dist/builder.js
tonode_modules/@prisma/client/scripts/postinstall.js
and thennode_modules/prisma/build/index.js
in the first place.nexus-plugin-prisma
does not include/require the postinstall script of@prisma/client
in any way I could find, and the Postinstall script does not require the CLI build in a way I could follow.Can you please help me understand and see what I am failing to see here?
PS: I committed the repo state where I did the investigation (including
.next
andnode_modules
) at janpio/prisma-limit-repro#10 / https://github.com/janpio/prisma-limit-repro/tree/nft-investigation as well.The text was updated successfully, but these errors were encountered: