fix: Prisma recordings on extended Prisma clients #140
Conversation
zermelo-wisen
force-pushed
the
fix/prisma-recordings-on-extended-clients
branch
4 times, most recently
from
2b8a072
to
e684a54
Compare
src/hooks/prisma.ts
Outdated
|
|
||
| // Normally, we "Cannot assign to 'PrismaClient' because it is a read-only property." | ||
| // Fortunately with this hack it can be done. | ||
| const modHacked = mod as unknown as { PrismaClient: unknown }; |
There was a problem hiding this comment.
Maybe we could use Object.defineProperty() instead?
src/hooks/prisma.ts
Outdated
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument | ||
| const result = Reflect.construct(target, argArray, newTarget); | ||
| if (!hookAttached) { | ||
| hookAttached = true; |
There was a problem hiding this comment.
I didn't notice it before, but now I can see hookAttached is a global. I know it's an edge case, but it will break if there are multiple prisma clients (as can be configured with multiple prisma: entries in the config file). How about using a WeakSet instead to keep track of patched clients?
| const params = ["data", "include", "where"].map((k) => { | ||
| return { type: "Identifier", name: k } as ESTree.Identifier; | ||
| }); | ||
| const params = [{ type: "Identifier", name: "args" } as ESTree.Identifier]; |
There was a problem hiding this comment.
Not sure why this change is necessary. Nb, you can use generate.identifier().
There was a problem hiding this comment.
There can be lots of options like data, select, include, where, orderBy, take, skip, distinct, cursor, relationLoadStrategy, etc. Having a parameter for each one seemed not to look good when exploring the AppMap in the viewer, since most of them are irrelevant in a specific call and end up showing as "undefined."
Making different subsets of parameters for each Prisma model query isn't practical either, because, for example, findMany can have around 10 options and many of them will again be "undefined" in a specific call.
src/hooks/prisma.ts
Outdated
| if (requestParams.action && requestParams.model) { | ||
| prismaCall = recording.functionCall( | ||
| getFunctionInfo(requestParams.model, requestParams.action, moduleId), | ||
| requestParams.model, | ||
| [requestParams.args?.data, requestParams.args?.include, requestParams.args?.where], | ||
| [requestParams.args?.data, requestParams.args?.include, requestParams?.args?.where], |
There was a problem hiding this comment.
Why only ? in one case? Surely either all can be undefined or none.
| "value": "undefined", | ||
| "class": "String", | ||
| "name": "args", | ||
| "value": "'{"data":{"name":"Alice","email":"[email protected]"}}'", |
There was a problem hiding this comment.
Why is this recorded as a string? Should probably be an object.
src/hooks/prisma.ts
Outdated
| [requestParams.args?.data, requestParams.args?.include, requestParams?.args?.where], | ||
| ); | ||
| // In extended prisma clients, we receive repeated calls | ||
| // with the same action, module and args. It can be seen |
There was a problem hiding this comment.
I suspect the problem might be that we're patching the same client multiple times. You can double check by looking at this, this.prototype and the proxy; if I'm right, the calls will have the same this and prototype but different proxies. In this case, perhaps you can track the created proxies using a WeakSet to ensure you're not repatching.
zermelo-wisen
force-pushed
the
fix/prisma-recordings-on-extended-clients
branch
3 times, most recently
from
1c2db9c
to
e1eb991
Compare
zermelo-wisen
force-pushed
the
fix/prisma-recordings-on-extended-clients
branch
from
e1eb991
to
c16940c
Compare
|
🎉 This PR is included in version 2.21.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #127
We do two types of Prisma recordings:
This fixes two problems reported in #127, that seem to occur with extended Prisma clients.