search

chrishoermann / zod-prisma-types

Generator creates zod types for your prisma models with advanced validation
Other
578 stars 43 forks source link

[BUG] `decimal.js` false-positive detection in monorepo #246

Open sylver opened 2 months ago

sylver commented 2 months ago

Describe the bug

When used in the context of a monorepo (typically with pnpm), since all packages dependencies are hoisted to <root>/node_modules, if any of them has decimal.js in its dependencies (even a nested one), the method getDecimalJSInstalled using require.resolve finds it in the root node_modules and returns true as a false positive (since the module is not actually a dependency of the current package).

A typical tree would be :

<root>/
 |- node_modules/  // `pnpm` hoisting all modules here, including `zod-prisma-types` and `decimal.js`
 |- packages/
 |   |- package-a/
 |   |   |- schema.prisma
 |   |   |- node_modules/ // `zod-prisma-types` installed here
 |   |   
 |   |- package-b/
 |   |   |- node_modules/ // `decimal.js` installed only here, not even as a direct dependency of `package-b`
... ...  ...

Package versions (please complete the following information):

Additional context

https://github.com/chrishoermann/zod-prisma-types/blob/a34b9f3cb04572185c598a692f62930337c3ca04/packages/generator/src/utils/getDecimalJSInstalled.ts#L1-L8

The method getDecimalJSInstalled should use a more contained approach to detect if decimal.js module is a dependency of the current package.

Problem is, given the versatile nature of monorepo dependency hoisting and the several options given to the user to set it up, it can become very tricky to know if a given decimal.js package is actually a direct dependency of the current package or not.

I would probably look directly into the current package.json dependencies since that would be the only reliable source of truth in such context.

I can do a PR but first I could use your thoughts on this @chrishoermann

chrishoermann commented 1 month ago

@sylver sorry for the late response and thanks for diving into this. As you mentioned, it seems to be a more precise approach to only look for the dependency in the current package.json. I actually never thougt about the problems the require.resolve() method could create in a monorepo setup like you mentioned. So I'd be happy if you could do a PR on this.