Skip to content
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

Cannot encode field to JSON #860

Closed
capJavert opened this issue May 28, 2024 · 5 comments
Closed

Cannot encode field to JSON #860

capJavert opened this issue May 28, 2024 · 5 comments

Comments

@capJavert
Copy link

capJavert commented May 28, 2024

I have proto schema like:

syntax = "proto3";
package project;

message Message {
    string id = 1;
    string type = 2;
    int32 created_at = 3;
    optional string header_image = 4;
}

When I try to create a instance in Typescript with:

const messageFromDb = await con.getRepository('message').findOneBy({ id })

const message = new Message({
  id: messageFromDb.id,
  type: messageFromDb.type,
  createdAt: messageFromDb.createdAt,
  headerImage: messageFromDb.headerImage // this can be null since it is optional
});

I get this error:

cannot encode field project.Message.headerImage to JSON

  at Object.writeMessage (node_modules/@bufbuild/protobuf/dist/cjs/private/json-format.js:146:23)
  at c.toJson (node_modules/@bufbuild/protobuf/dist/cjs/message.js:87:21)
  at c.toJSON (node_modules/@bufbuild/protobuf/dist/cjs/message.js:113:21)
      at stringify (<anonymous>)

I am using latest versions of following libraries to generate schema and use in Typescript project:

@bufbuild/protobuf @bufbuild/protoc-gen-es

I noticed above is happening due to headerImage sometimes being passed as null since it can be optional in the DB. If I fallback to undefined eg. headerImage: messageFromDb.headerImage || undefined everything works. While this is easy for one type I have more complex type where doing above undefined fallback would be inefficient and complicated.

My question is why can't the library encode null to JSON or why does it encode it in the first place when it is marked as optional in the proto schema?

@timostamm
Copy link
Member

Hey Ante, the optional Protobuf field compiles to:

  /**
   * @generated from field: optional string header_image = 4;
   */
  headerImage?: string;

The questionmark makes the property optional, meaning that it can be omitted, or set to undefined. However, setting the property to null is a type error:

TS2322: Type 'null' is not assignable to type 'string | undefined'.

In TypeScript, optional properties cannot be null. Here is a Playground link that shows the behavior.

Can you share which ORM you are using? I'd expect headerImage: messageFromDb.headerImage to be a type error, if messageFromDb.headerImage is string | null.

@capJavert
Copy link
Author

capJavert commented May 29, 2024

@timostamm using typeorm and it returns null because that is the actual value in the DB. It is probably not showing type error in this project since TS is not set to strict. That said, I am still wondering why it can't encode null since it should be a valid JSON value?

@timostamm
Copy link
Member

null is a valid JSON value, but not a valid value for the message property. Hence the type error.

The issue typeorm/typeorm#8703 pinpoints the problem. Unfortunately, there isn't a good resolution. I hope that it's clear that we cannot make all optional properties also accept null, since that would also affect users who want to consume a Protobuf message.

What we can do is treat null the same as undefined in the constructor, though. We have already done this in the upcoming v2, and I'm backporting the change to v1 in #862.

BTW, || will evaluate to the right side if the left side is truthy. The nullish coalescing operator is more reliable, see 0 || undefined vs. 0 ?? undefined.

@capJavert
Copy link
Author

@timostamm yes absolutely it makes sense! Thanks for back-porting this!

@timostamm
Copy link
Member

Closed by #862, released in v1.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants