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

Generated types allow assigning wrong message type if it is a superset of the target type #551

Closed
LinearSpoon opened this issue Aug 15, 2023 · 1 comment

Comments

@LinearSpoon
Copy link

There is no compile error in const a: ProtoA = new ProtoB() if the fields in ProtoA are a subset of the fields in ProtoB.

example.proto

syntax = "proto3";

message Foo {
    int32 foo = 1;
}

message NotFoo {
    int32 foo = 2;  // Different tag number
}

// Foo with an extra field
message FooAndBar {
    int32 foo = 1;
    int32 bar = 2;  // Extra field
}

example.ts

import { Foo, FooAndBar, NotFoo } from "./example_pb.js";

// Unsafe - No compile time error
const a0: Foo = new FooAndBar();
const a1: Foo = new NotFoo();
const a2: NotFoo = new Foo();
const a3: NotFoo = new FooAndBar();

// Good - compile time error
const a4: FooAndBar = new Foo();  // Property 'bar' is missing in type 'Foo' but required in type 'FooAndBar'.
const a5: FooAndBar = new NotFoo();  // Property 'bar' is missing in type 'NotFoo' but required in type 'FooAndBar'.

// This also affects function parameters
function example(foo: Foo) { }
example(new NotFoo());  // Unsafe - No error

An example fix would be adding a dummy private variable to to every generated proto. TypeScript doesn't output any code when compiling this, so there should be no runtime implications (Playground Link).

Relevant TypeScript docs:

When an instance of a class is checked for compatibility, if the target type contains a private member, then the source type must also contain a private member that originated from the same class.

export class Foo extends Message<Foo> {
  private unused: any;

  // Existing generated code...
}

// Good - Compile time error
// Type 'FooAndBar' is not assignable to type 'Foo'.
//   Types have separate declarations of a private property 'unused'.
const a0: Foo = new FooAndBar();
@timostamm
Copy link
Member

We've looked into this, also in the context of connectrpc/connect-es#694. Unfortunately, the mitigations we investigated came with major drawbacks.

We are fixing this with the upcoming v2. Here's how v2 behaves with the example above:

import { create } from "@bufbuild/protobuf";
import {
  FooSchema,
  FooAndBarSchema,
  NotFooSchema,
  type Foo,
  type NotFoo,
  type FooAndBar
} from "./gen/example_pb.js";

const a0: Foo = create(FooAndBarSchema); // TS2322: Type FooAndBar is not assignable to type Foo
const a1: Foo = create(NotFooSchema); // TS2322: Type NotFoo is not assignable to type Foo
const a2: NotFoo = create(FooSchema); // TS2322: Type Foo is not assignable to type NotFoo
const a3: NotFoo = create(FooAndBarSchema); // TS2322: Type FooAndBar is not assignable to type NotFoo
const a4: FooAndBar = create(FooSchema);  // TS2322: Type Foo is not assignable to type FooAndBar
const a5: FooAndBar = create(NotFooSchema);  // TS2322: Type NotFoo is not assignable to type FooAndBar
function example(foo: Foo) { }
example(create(NotFooSchema));  // TS2345: Argument of type NotFoo is not assignable to parameter of type Foo

The $typeName basically gives us nominal typing. The full error is:

TS2322: Type FooAndBar is not assignable to type Foo
  Type FooAndBar is not assignable to type Message<"Foo">
    Types of property $typeName are incompatible.
      Type "FooAndBar" is not assignable to type "Foo"

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