-
Notifications
You must be signed in to change notification settings - Fork 348
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
unexpected changes in camelToSnake()
at v1.176.1
#1048
unexpected changes in camelToSnake()
at v1.176.1
#1048
Comments
Same problema here. This is a breaking change that is given us a lot of problems. |
Hey guys, thanks for the report! Hopefully it won't be too bad to craft a regex that achieves both goals--I'll take a look at this in a few days/next weekend, unless @BrandonArmand you can take a look/beat me to it. Thanks! |
Hey yeah i actually noticed this today as well. I will be happy to whip up some regex. |
Thanks. I think, the original cause was two-fold:
For 1. For 2. So, I came up with the following regular expression. export function camelToSnake(s: string): string {
return s.replace(/(?<!^)([a-z0-9][A-Z]|([A-Z][A-Z])[a-z0-9])/g, (m) => m.length === 2 ? m[0] + "_" + m[1] : m[0] + "_" + m[1] + m[2]).toUpperCase();
}
// getAPIValueV1Beta1 => GET_API_VALUE_V1_BETA1 I have passed the test case, but I feel it is not very elegant. If you have any good ideas, please let me know. |
I believe #1052 is a good approach, and at least fixes all current test cases 😅 , I went ahead and merged the fix, let us know if there are other exceptions. 🤞 |
🎉 This issue has been resolved in version 1.176.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@stephenh Thank you so much for the fix ! |
Probrem
I noticed that the behavior of the
camelToSnake()
changed with the fix in #1046.Specifically, the behavior has changed when the string to be converted contains numbers.
For example, if the package name includes a version like
v1
, it is affected.With the following code:
As a result, the generated constant name has changed, causing build errors.
To clarify, these codes are generated when using the
nestJs=true
option.Upon reviewing the PR, it seems this behavior change was not intended.
This unexpected change in a patch version has caused a lot of confusion.
That said, the changes in #1046 appear reasonable.
Reverting the commit does not seem to be a good solution.
I reviewed the documentation and code of the
case-anything
package, which is used internally bycamelToSnake()
, and it seems difficult to control the handling of numbers externally.Moreover, there is no set specification on how to handle numbers in snake_case, and there are multiple opinions on the matter. Therefore, I don't think the behavior in v1.176.1 is incorrect.
How do you plan to address this issue?
If the behavior in v1.176.1 is considered correct, I believe it should be clearly documented, and a minor version update should be made.
Sorry I can't offer any concrete suggestions for improvement.
I am always grateful for this package. Thank you very much.
Environment Used for Testing
The information about the environment I used for testing is as follows:
buf.gen.yaml
sample.proto
The text was updated successfully, but these errors were encountered: