Fix macro for inherited types to avoid typescript error #76
Conversation
zzorba
force-pushed
the
dsalinas/fix_macro_for_inherited_types
branch
from
3ca1b10
to
1cdf6fd
Compare
|
Basically our exporting trick has one problem, microsoft/TypeScript#30355 So I marked them public and put a strongly worded note on the fields in question. We could also suppress the typescript warning, but it has to be done at each export site (annoying) |
| * This member should be private, but typescript requires | ||
| * it be public because it cannot enforce it. | ||
| */ | ||
| public readonly __variant: number |
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| * it be public because it cannot enforce it. | ||
| */ | ||
| public readonly __variant: number, | ||
| message?: string |
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| @@ -26,7 +38,7 @@ export class UniffiError extends Error { | |||
| return UniffiError.createMessage( | |||
| this.__uniffiTypeName, | |||
| this.__variantName, | |||
| this.message, | |||
| this.message | |||
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| @@ -54,7 +66,7 @@ export class UniffiThrownObject<T> extends Error { | |||
| constructor( | |||
| private readonly __uniffiTypeName: string, | |||
| public readonly inner: T, | |||
| message?: string, | |||
| message?: string | |||
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| @@ -66,7 +78,7 @@ export class UniffiThrownObject<T> extends Error { | |||
| return UniffiThrownObject.createMessage( | |||
| this.__uniffiTypeName, | |||
| this.inner, | |||
| this.message, | |||
| this.message | |||
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| @@ -108,7 +120,7 @@ export const UniffiInternalError = (() => { | |||
| class BufferOverflow extends Error { | |||
| constructor() { | |||
| super( | |||
| "Reading the requested value would read past the end of the buffer", | |||
| "Reading the requested value would read past the end of the buffer" | |||
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| @@ -145,14 +157,14 @@ export const UniffiInternalError = (() => { | |||
| class ContractVersionMismatch extends Error { | |||
| constructor(rustVersion: any, bindingsVersion: any) { | |||
| super( | |||
| `Incompatible versions of uniffi were used to build the JS ($${bindingsVersion}) from the Rust (${rustVersion})`, | |||
| `Incompatible versions of uniffi were used to build the JS ($${bindingsVersion}) from the Rust (${rustVersion})` | |||
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| ); | ||
| } | ||
| } | ||
| class ApiChecksumMismatch extends Error { | ||
| constructor(func: string) { | ||
| super( | ||
| `FFI function ${func} has a checksum mismatch; this may signify previously undetected incompatible Uniffi versions`, | ||
| `FFI function ${func} has a checksum mismatch; this may signify previously undetected incompatible Uniffi versions` |
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| @@ -37,7 +49,7 @@ export class UniffiError extends Error { | |||
| private static createMessage( | |||
| typeName: string, | |||
| variantName: string, | |||
| message: string | undefined, | |||
| message: string | undefined | |||
There was a problem hiding this comment.
Nit: prettier wants a trailing comma.
| /* | ||
| * This member should be private, but typescript requires | ||
| * it be public because it cannot enforce it. | ||
| */ | ||
| public readonly __uniffiTypeName: string, | ||
| /* | ||
| * This member should be private, but typescript requires | ||
| * it be public because it cannot enforce it. | ||
| */ | ||
| public readonly __variantName: string, | ||
| /* | ||
| * This member should be private, but typescript requires | ||
| * it be public because it cannot enforce it. | ||
| */ | ||
| public readonly __variant: number, |
There was a problem hiding this comment.
I wonder if this would be better as private properties in the generated subclasses rather than the super class.
My hunch is that the DX of having __ properties turn up in autocomplete becomes annoying after a while.
WDYT? Is that annoyance a non-issue, or is it worth the extra work?
There was a problem hiding this comment.
Yes, that would be better, and they should still be accessible on the union type we're creating. AFAICT it is just the inheritence + const composure that is the problem.
It would be very nice if the __ didn't show up in autocomplete, since you aren't supposed to use them -- makes the generated code a lot more self documenting.
|
Closing this in favour of #84. |
I believe this will address #75