thoth-org/Thoth.Json

Remove Inject ITypeResolver

alfonsogarciacaro opened this issue ยท 3 comments

For Fable 4 I'm considering to remove the Inject feature because, as far as I know, besides Thoth.Json nobody uses it (we do use Inject in Fable.Library but it relies on a different, internal resolution mechanism). Originally we used [<Inject>] ITypeResolver in Thoth.Json to make it compatible with the Fable REPL but since libraries are not compatible with Fable 3 REPL anyways, we can just remove it and jut use inline to resolve generic info as this is the usual thing to code with Fable code.

@MangelMaxime If you're ok with that I can send a PR. Just let me know if I should target master or vNext branch ๐Ÿ‘

Hello @alfonsogarciacaro,

wasn't the goal of Inject to also minimize the impact on the bundle size too? If we are [<Inject>] and inline are equivalent replacement, I am ok. You know better than me on this subject.

Please target master branch, vNext lost it's traction I need to rewrite it completely because I am unsure of the breaking changes I introduced compare to master branch. Worked on it for a too long period of time and lost track of it...

For information Thoth.Fetch is also using [<Inject>]

https://github.com/thoth-org/Thoth.Fetch/blob/master/src/Fetch.fs

wasn't the goal of Inject to also minimize the impact on the bundle size too?

I forgot about that ๐Ÿ˜… but you're right, inlining big functions may cause duplication of code and increase the bundle size. To avoid this I usually inline a minimal function that just gets the type info with typeof<'T> and passes it to another function:

static member generateBoxedDecoderCached(t: System.Type, ?caseStrategy : CaseStrategy, ?extra: ExtraCoders): BoxedDecoder =
    let caseStrategy = defaultArg caseStrategy PascalCase

    let key =
        t.FullName
        |> (+) (Operators.string caseStrategy)
        |> (+) (extra |> Option.map (fun e -> e.Hash) |> Option.defaultValue "")

    Util.CachedDecoders.GetOrAdd(key, fun _ -> autoDecoder (makeExtra extra caseStrategy) false t)

static member inline generateDecoderCached<'T>(?caseStrategy : CaseStrategy, ?extra: ExtraCoders): Decoder<'T> =
    Auto.generateBoxedDecoderCached(typeof<'T>, ?caseStrategy=caseStrategy, ?extra=extra) |> unboxDecoder