Camel case conversion bug with numbers in field names

[allenhartwig]

protobuf.js version: 6.8.6

The snake to camel case conversion should remove all underscores from the field name and match the behavior of lodash's _.camelCase method. Currently, if there is a protobuf field with the name of street_1, protobufjs's camel casing of this field results in street_1 while lodash results in street1. I see lodash being correct here as a camel cased string should have no _ left in it after conversion.

const _ = require('lodash');
const text = "street_1"

var camelCaseRe = /_([a-z])/g;
const camelCase = function camelCase(str) {
    return str.substring(0, 1)
         + str.substring(1)
               .replace(camelCaseRe, function($0, $1) { return $1.toUpperCase(); });
};
console.log(camelCase(text));
console.log(_.camelCase(text));

camelCase function from https://github.com/dcodeIO/protobuf.js/blob/c482a5b76fd57769eae4308793e3ff8725264664/src/util.js#L90-L101

While I feel the current behavior is a bug, fixing it could introduce a breaking change. It may be preferable to introduce a new argument to the load function so that a custom case transformation could be passed in to be used in lieu of util.camelCase.

I am happy to submit a PR, but would like to know what the desired resolution would be, to fix the behavior of util.camelCase or allow for custom case transformations via a load argument.

[frankwallis]

I am also interested in extending this behaviour to enable conversion from PascalCase -> camelCase for field names. Again would be happy to send PR if we can agree on the API.

[alexander-fenster]

Hi @allenhartwig and @frankwallis,

The main problem with the requests like that, as discussed in #1154, is that these changes are not just bug fixes, but are actually breaking changes so I feel we cannot do anything like that without bumping a major version; we don't know if any existing code breaks after doing that. I think, previously, the change related to such field names happened when moving from v5 to v6 which was a major version update. So even if it's clearly a bug, we might as well just leave it there for now.

Adding an option might be a good solution but it has a risk of having too many options defined for different possible field name conversions. I'd leave it open for now and see if we can go back to this if we decide to have a major version update with breaking changes.

Thank you!

[quirkles]

Just out of curiosity's sake, what is the issue with releasing a new major version? People can stay on the old version or upgrade no?

[ssilve1989]

What's the status of this? As mentioned in other PR's comments this is pretty disruptive when consuming protobufs across multiple languages and other languages transform it appropriately. Also disruptive when using an alternative TypeScript interface generator like ts-proto which does it correctly but then the field won't match when using protobufjs as the serialization layer, which is the case in frameworks like Nest.JS