typeshare icon indicating copy to clipboard operation
typeshare copied to clipboard

Are `usize` and `i64` not supported?

Open affanshahid opened this issue 3 years ago • 12 comments

Ran typeshare on my project and got the following errors:

thread '<unnamed>' panicked at 'failed to parse a rust type: ["usize"]', /Users/affan/.cargo/registry/src/github.com-1ecc6299db9ec823/typeshare-cli-1.0.0/src/main.rs:209:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread '<unnamed>' panicked at 'failed to parse a rust type: ["i64"]', /Users/affan/.cargo/registry/src/github.com-1ecc6299db9ec823/typeshare-cli-1.0.0/src/main.rs:209:17

i64 can be represented using a JS BigInt.

affanshahid avatar Nov 27 '22 15:11 affanshahid

as with ts-rs would be nice to have a #[typeshare(type = "number")] property attribute.

jeremychone avatar Nov 28 '22 20:11 jeremychone

Thanks for opening this issue. I'll look into whether this is possible.

snowsignal avatar Dec 01 '22 23:12 snowsignal

Getting a similar error with u64 and isize, those four types seems to be "disabled" here

https://github.com/1Password/typeshare/blob/fbf5aea145b7895f3a998db050bd972a9af79e05/core/src/rust_types.rs#L225-L227

But the types exist in the SpecialRustType and looking in the four supported languages, they all have mappings for those four types.

Locally I've changed those three lines in rust_types.rs to parse the types and I'm not getting any errors, but I'm guessing that there must be a reason that those lines/errors exist? It works for Go, Swift, TypeScript, and Kotlin.

Changes

In rust_types.rs:

-                    "u64" | "i64" | "usize" | "isize" => {
-                        return Err(RustTypeParseError::UnsupportedType(vec![id]))
-                    }
+                    "u64" => RustType::Special(SpecialRustType::U64),
+                    "usize" => RustType::Special(SpecialRustType::USize),
+                    "i64" => RustType::Special(SpecialRustType::I64),
+                    "isize" => RustType::Special(SpecialRustType::ISize),

and in typescript.rs:

            SpecialRustType::U64
            | SpecialRustType::I64
            | SpecialRustType::ISize
            | SpecialRustType::USize => {
-                panic!("64 bit types not allowed in Typeshare")
+                Ok("number".into())
            }

Edit: I'm guessing the reason it is disabled because this would not be a safe FFI type for TypeScript/JavaScript, since it may not support 64 bit types depending on the platform. Seems like we would just need to convert it to bigint https://www.typescriptlang.org/docs/handbook/typescript-in-5-minutes-func.html?#built-in-types, where the previous block of code would be:

            SpecialRustType::U64
            | SpecialRustType::I64
            | SpecialRustType::ISize
            | SpecialRustType::USize => {
-                panic!("64 bit types not allowed in Typeshare")
+                Ok("bigint".into())
            }

BenJeau avatar Dec 31 '22 15:12 BenJeau

I think part of the problem is that JSON is usually going to be the medium of exchange for typeshared data, and so when you're loading into javascript / typescript you'd need to not use JSON.parse, since it doesn't care what type you intended and will just load into a number. A lot of the complexity around typeshare enums is similarly related to conforming to the default behavior of various languages when dealing with their equivalent of enums.

Lucretiel avatar Jan 04 '23 22:01 Lucretiel

I understand that most people would use JSON (as it is one of the most popular transport medium), but it is not the only transport medium in which typeshare types can be used, e.g. FFIs

If users of typeshare decide to use it with 64bit types, maybe the library should just give a warning and maybe suggest an alternative library instead of the builtin JSON.parse/JSON.stringify, such as https://github.com/blitz-js/superjson, for the TypeScript/JavaScript side? It seems like this feature is blocked on the fact that JavaScript does not play well by default with 64bit values in JSON, whilst other languages are fine.

BenJeau avatar Jan 05 '23 02:01 BenJeau

Since typeshare is a developer tool, I would recommend picking a default (failing or bigint are both reasonable) and letting the developer override it by property.

In many circumstances, 2^53-1 or 2^64-1 can be considered equivalent (v.s. the complexity of introducing bigint in all JS code base). Obviously, this should not be the default, but not providing this escape hatch might add unnecessary code complexity downstream.

jeremychone avatar Jan 05 '23 17:01 jeremychone

I just ran into this -- I also need to serialize u64 types. Seems like an oversight to always panic when encountering these types as there are types in the destination languages to support large numbers. At least let the developer set the type themselves.

mattyg avatar Feb 18 '24 23:02 mattyg

@mattyg A workaround for this is to use #[typeshare(serialized_as = "number")].

AlexanderProd avatar Jul 22 '24 10:07 AlexanderProd

@mattyg A workaround for this is to use #[typeshare(serialized_as = "number")].

This workaround doesn't work. Tested stable and the beta branch.

Uzaaft avatar Aug 26 '24 06:08 Uzaaft

I think it would make sense for us to support these and emit a suppressable warning if they are used. @Lucretiel

LuminaSapphira avatar Sep 24 '24 20:09 LuminaSapphira

@LuminaSapphira I made a PR for supporting this about a year back. What do you think of that approach (i.e. allow only if the output type is explicitly specified?)

tomjw64 avatar Sep 28 '24 19:09 tomjw64

I resolved the existing merge conflicts in the above PR. I would still really appreciate a review + workflow approval from a maintainer!

tomjw64 avatar Sep 28 '24 20:09 tomjw64