protobuf-ts icon indicating copy to clipboard operation
protobuf-ts copied to clipboard

Is it possible to make `fromJson` and `create` methods case-insensitive?

Open someden opened this issue 3 years ago • 2 comments

Method fromJson can parse different variants, for example if we have proto like:

message SomeEntity {
    string ID = 1 [json_name="id"];
}

Then protobuf-ts will generate this:

interface SomeEntity {
    /**
     * @generated from protobuf field: string ID = 1 [json_name = "id"];
     */
    iD: string;
}

Then method will work with:

SomeEntity.fromJson({ id: 'new-id' });
SomeEntity.fromJson({ iD: 'new-id' });
SomeEntity.fromJson({ ID: 'new-id' });

But error will be with:

SomeEntity.fromJson({ Id: 'new-id' });

So, why don't make fromJson method case-insensitive? And also create method? It will be convenient, especially for migration from grps-web to protobuf-ts, because grps-web sometimes has different rules and different case for some fields, example: Proto:

message SomeEntity {
    string SomeField = 1 [json_name="someField"];
}

protobuf-ts will generate:

interface SomeEntity {
    someField: string;
}

But in grpc-web we will have somefield.

someden avatar Sep 15 '22 14:09 someden

This is an interesting idea. There is nothing saying JSON parsers cannot accept other variations besides json_name and proto field name, but I'm not sure it is worth the extra complexity. It's similar with our create(), with the additional problem that I don't think it's feasible to reflect case-insensitivity in the typing.

But did you know that most of those naming problems go away if you use snake_case for field names in protobuf? It is the recommended naming convention by all protobuf linters. protobuf-ts will generate a field named id instead of iD, and both protobuf-ts and grpc-web will generate someField consistently.

timostamm avatar Sep 15 '22 17:09 timostamm

Yep, I know it, but sometimes we can have some legacy that doesn't follow any conventions)

someden avatar Sep 19 '22 13:09 someden

You can patch ReflectionJsonReader for your project. This is the method you may want to replace:

https://github.com/timostamm/protobuf-ts/blob/3eea598471fb9c891c8b6da06bc1cbe18bc3457e/packages/runtime/src/reflection-json-reader.ts#L32

For example you can easily let it accept lowercase proto field names:

ReflectionJsonReader.prototype.prepare = function () {
        if (this.fMap === undefined) {
            this.fMap = {};
            const fieldsInput = this.info.fields ?? [];
            for (const field of fieldsInput) {
                this.fMap[field.name.toLowerCase()] = field; // <---
                this.fMap[field.name] = field;
                this.fMap[field.jsonName] = field;
                this.fMap[field.localName] = field;
            }
        }
}

Accepting arbitrary casing is not quite as straight-forward, and you would probably have to patch (or) wrap the read method.

I don't think it's a good idea to complicate the protobuf-ts runtime (increasing code size, maintenance effort, and possibly introducing bugs) in order to support non-standard JSON input, so I'm closing this.

timostamm avatar Sep 26 '22 08:09 timostamm