-
-
Notifications
You must be signed in to change notification settings - Fork 38
fix: Add type/enum on string or number #530
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
base: main
Are you sure you want to change the base?
fix: Add type/enum on string or number #530
Conversation
Hi! please add tests ensuring your change works and fixes your specific problem :) |
Perfect. I’ll work on the weekend✅ |
case 'number': | ||
case 'Prisma.IntFieldUpdateOperationsInput | number': | ||
case 'Prisma.FloatFieldUpdateOperationsInput | number': | ||
if (!shouldReplaceStrings) { |
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.
As you've added types to number and reusing this flag, maybe we should rename this variable to somewhat like replacePrimitive
?
result = typeToChange; | ||
break; | ||
|
||
case `Prisma.${model}Update${field}Input | string[]`: |
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.
Maybe you should move these lines after L65
result = `(${typeToChange})[]`; | ||
break; | ||
|
||
case `Prisma.NullableIntFieldUpdateOperationsInput | number | null`: |
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 wonder whether these lines can be merged with the case
on L104
Thank you for the solution! I encountered the same problem recently and found this PR which fixed the problem. I left several comments and hoping to improve the code quality :) And I have a question about the error handling logic, which might not be directly related to this PR, but I did not find a good place to ask about it. I wonder why
|
Nice! At the moment I haven’t had time to finish the tests, but I find your observations valuable and I’ll review them tomorrow after work. For now, I’m using it from my GitHub and it’s working quite well for me:
I hope to get it working as soon as possible to merge in main. |
Fixed if one type a string, the other json stop working.
This is because I need to use an
enum
or string constants like'hello' | 'word'
Example:
Doesn't Work
I dont know why Work???