Inlining Vec, HashMap, Option and Result

[escritorio-gustavo]

The four types in the title (as well as anything that uses impl_shadow! to defer to them) cause the #[ts(inlne)] attribute to generate invalid import statements, as shown in #168 and #232. This discussion is meant to decide how to tackle this problem.

[escritorio-gustavo]

The following is one of the ideas I proposed in #233:

For Vec and HashMap, we could make Self::inline just return Self::name or panic! saying they can't be inlined

This idea seems ridiculous, but think about it:

• Vec<T>::name returns Array<T::name()>
• Vec<T>::inline returns Array<T::inline()>

But notice that Vec<T>::inline doesn't actually inline Vec, instead it inlines T.
Why? Because there is no way to inline Array.

The problem is instead of stating this to be illegal (or just the same as calling name), we recursively
inline T, but our derive macro can't handle that!

The same is true for Option and Result. Self::name returns them already inlined! With any other enum, inlining gives the type union representation, while these two do that for name. There is no inlining an enum's type union form!

In fact, if Option were exported, name would return Option<T::name()> and inline would return what name currently returns

You can even test this out by creating your own Option and Result enums:

#[derive(TS)]
#[ts(export, untagged)]
enum MyOption<T> {
    Some(T),
    None
}

#[derive(TS)]
#[ts(export, rename_all = "lowercase")]
enum MyResult<T, E> {
    Ok(T),
    Err(E),
}

#[derive(TS)]
#[ts(export)]
struct MyStruct {
    not_inlined_option: MyOption<String>,

    #[ts(inline)]
    inlined_option: MyOption<String>,

    not_inlined_result: MyResult<EnumWithName, String>,

    #[ts(inline)]
    inlined_result: MyResult<EnumWithName, String>,
}

#[test]
fn check() {
    assert_eq!(
        MyStruct::decl(),
        "type MyStruct = { not_inlined_option: MyOption<string>, inlined_option: string | null, not_inlined_result: MyResult<EnumWithName, string>, inlined_result: { \"ok\": EnumWithName } | { \"err\": string }, };"
    )
}

[escritorio-gustavo]

And here is the exported TS, which contains all the correct imports

// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { EnumWithName } from "./EnumWithName"; // from #168
import type { MyOption } from "./MyOption";
import type { MyResult } from "./MyResult";

export type MyStruct = {
  not_inlined_option: MyOption<string>;
  inlined_option: string | null;
  not_inlined_result: MyResult<EnumWithName, string>;
  inlined_result: { "ok": EnumWithName } | { "err": string };
};

[NyxCode]

I think your analysis here is correct to some extent. The handeling for Option and Result is different such that they're always being inlined.

To make them behave like every other type, we could just define #[derive(TS)] enum ResultImpl { ... }, and then use impl_shadow!(as ResultImpl<T, E>: impl<T: TS, E: TS> TS for Result<T, V>); to make Result<T, E> behave just like our ResultImpl.

However, it seems like there's an other bug lurking here:

#[derive(TS)]
#[ts(export)]
struct Test {}

#[derive(TS)]
#[ts(export)]
enum MyOption<T> {
    Some(T),
    None
}

#[derive(TS)]
#[ts(export)]
struct Outer {
    a: MyOption<Test>
}

generates this TS, in which the import for Test is missing!

// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { MyOption } from "./MyOption";

export type Outer = { a: MyOption<Test>, };

I'm not 100% sure about this, but fixing this bug might allow us to keep our current special-handeling of Option and Result.

[escritorio-gustavo]

I think I outlined the problem with that approach in #241 - type aliases.

Makes sense, and then we'd be right back to #70

[NyxCode]

Only for imports though, so I think we are making progress :D

[escritorio-gustavo]

Okay, so we need to make sure to add the generics() to the import, but only when the type is not inlined, right?

[escritorio-gustavo]

I imagine this can be done by changing Dependencies::push to also add T::generics()?

[NyxCode]

Exactly! That's why I put that logic in Dependencies::push. If a type is inlined, we don't use Dependencies::push, since the parent type doesn't actually depend on the type that got inlined. In that case, we call Dependencies::append_from.

[escritorio-gustavo]

Alright, now that #241 has resolved the problem with generics not being imported, we need to get back to how we are supposed to deal with the inlining of transparent types (Option, Result, Vec, HashMap and their impl_shadows)

[NyxCode]

Absolutely!

After thinking about your analysis a bit, I now totally agree that the recursive inlining the impls currently do is a no-go since it breaks the dependency tracking outright.

Let's look at Result<T, E>:

It currently returns format!("{{ Ok : {} }} | {{ Err : {} }}", T::name(), E::name()) as TS::name(). That's not too different from "Result<{T::name()}, {E::name()}>", which is what a #[derive(TS)] enum Result<T, E> { .. } type would return. In both cases, the TS::name()s of T and E end up in the TS::name() of Result<T, E>.
If inlining here was just a no-op, I think that impl would be fine.

[escritorio-gustavo]

Yeah, there's no way to track the dependencies if we ever do T::inline() and E::inline().

My idea is to make both Result::<T, E>::name() and Result::<T, E>::inline() return format!("{{ Ok : {} }} | {{ Err : {} }}", T::name(), E::name())

[NyxCode]

Also, out of the four (Vec, HashMap, Option, Result), Result is different than the other 3.
The others are primitives in TS, and we actually need a custom TS impl for them.

Result, on the other hand, we could get with #[derive(TS)]

[escritorio-gustavo]

I think your idea of having Result be an impl_shadow of a derived ResultDef is the best way to go

[escritorio-gustavo]

@NyxCode, I have pushed a commit that should fix the problem by making all 4 of those impls more closely resemble impl_wrapper over their generics

[escritorio-gustavo]

This has been successfully resolved by merging #241