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

Fix macro for inherited types to avoid typescript error #76

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions typescript/src/enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,20 @@
*/
export class UniffiEnum {
constructor(
private readonly __uniffiTypeName: string,
private readonly __variantName: string,
private readonly __variant: number,
/*
* 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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

) {}
}
36 changes: 24 additions & 12 deletions typescript/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,22 @@
// for each error.
export class UniffiError extends Error {
constructor(
private readonly __uniffiTypeName: string,
private readonly __variantName: string,
private readonly __variant: number,
message?: string,
/*
* 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,
Comment on lines +14 to +28
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

message?: string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

) {
// We append the error type and variant to the message because we cannot override `toString()`—
// in errors.test.ts, we see that the overridden `toString()` method is not called.
Expand All @@ -26,7 +38,7 @@ export class UniffiError extends Error {
return UniffiError.createMessage(
this.__uniffiTypeName,
this.__variantName,
this.message,
this.message
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

);
}

Expand All @@ -37,7 +49,7 @@ export class UniffiError extends Error {
private static createMessage(
typeName: string,
variantName: string,
message: string | undefined,
message: string | undefined
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

): string {
const prefix = `${typeName}.${variantName}`;
if (message) {
Expand All @@ -54,7 +66,7 @@ export class UniffiThrownObject<T> extends Error {
constructor(
private readonly __uniffiTypeName: string,
public readonly inner: T,
message?: string,
message?: string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

) {
// We append the error type and variant to the message because we cannot override `toString()`—
// in errors.test.ts, we see that the overridden `toString()` method is not called.
Expand All @@ -66,7 +78,7 @@ export class UniffiThrownObject<T> extends Error {
return UniffiThrownObject.createMessage(
this.__uniffiTypeName,
this.inner,
this.message,
this.message
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

);
}

Expand All @@ -81,7 +93,7 @@ export class UniffiThrownObject<T> extends Error {
private static createMessage<T>(
typeName: string,
obj: any,
message: string | undefined,
message: string | undefined
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

): string {
return [typeName, stringRepresentation(obj), message]
.filter((s) => !!s)
Expand All @@ -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"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

);
}
}
Expand Down Expand Up @@ -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})`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prettier wants a trailing comma.

);
}
}
Expand Down
Loading