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

RFC: branded types for aliases and external types (for strings or more) #23

Open
mradamczyk opened this issue Jun 13, 2018 · 5 comments

Comments

@mradamczyk
Copy link

Currently, even if a named type in Conjure is distinguishable from a string, a typescript declaration will just have string in place of the type. That can easily lead to problems where type A is used instead of type B (which wouldn't happen eg. with conjure generated java).

I propose to generate stronger typed interfaces for named strings (aliases, rids, external objects with base-type string). The behavior might require to "opt-in" so that it won't cause a breaking change.


Example - Alias

ExampleAlias:
  alias: string

could produce:

export type ExampleAlias = string & {
    __conjure_branded: "ExampleAlias";
};
export const ExampleAlias = {
    of: (exampleAlias: string): ExampleAlias => exampleAlias as ExampleAlias,
};

Example - Import

imports:
  ResourceIdentifier:
    base-type: string
    external:
      java: com.palantir.ri.ResourceIdentifier

could produce:

export type ResourceIdentifier = string & {
    __conjure_branded: "ResourceIdentifier";
};
export const ResourceIdentifier = {
    of: (resourceIdentifier: string): ResourceIdentifier => resourceIdentifier as ResourceIdentifier,
};

With such declarations, we can get much stronger typings:

function foo1(bar: ExampleAlias) {
    return bar;
}
foo1("abc"); // ERROR - TS doesn't allow to use an unnamed string where ExampleAlias is expected
foo1("abc" as ExampleAlias); // OK
foo1(ExampleAlias.of("abc")); // OK
foo1(ResourceIdentifier.of("def")); // ERROR - TS doesn't allow to use another branded string

function foo2(bar: string) {
    return bar;
}
foo2("abc"); // OK
foo2("abc" as ExampleAlias); // OK
foo2(ExampleAlias.of("abc")); // OK - as we don't expect ExampleAlias excplicitly
foo2(ResourceIdentifier.of("def")); // OK - as we don't expect ResourceIdentifier excplicitly

Moreover, I can imagine such extension for any primitive type, not only string.

I'd be happy to contribute if you think it makes sense.

@mnazbro
Copy link

mnazbro commented Mar 15, 2019

@iamdanfox Has this proposal been considered?

@iamdanfox
Copy link
Contributor

👍 think it looks sensible. Sadly we can't really do opt-in feature flags in this repo because this would allow API producers to ship compilation breaks in a minor release of their API.

The internal publish plugin uses the '301 versioning scheme' to mitigate this, so I think we'd just need to get this right first time and release a 4xx.

@mcintyret
Copy link

Agree that this would be very cool, although slight preference to not provide the of() method - since this resolves to a noop at runtime it feels unnecessary. Users could just do the cast themselves.

@Que3216
Copy link

Que3216 commented May 26, 2021

+1 on this issue. We've seen serious bugs that this would have prevented.

@westinrm
Copy link

westinrm commented May 4, 2022

#189 addressed half of this since it generates flavored types for aliases, but it doesn't cover external types. Would extending #189 to support external types be reasonable?

@jdm2212 jdm2212 closed this as completed Oct 17, 2022
@jdm2212 jdm2212 reopened this Feb 29, 2024
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

7 participants