search

fsprojects / FSharp.Data.JsonSchema

MIT License
48 stars 6 forks source link

Generic Collections Stack Overflow #7

Closed fluxlife closed 3 years ago

fluxlife commented 4 years ago

I may be using the API the wrong way as I just tried to emulate what you have going on in tests but..

I have a generic type defined as:

type PaginatedResult<'T> =
    {
        Page: int
        PerPage: int
        Total: int
        Results: 'T seq
    }

and when I try to generate the schema it crashes and stack overflows. I have no other stack trace to show :(

The troubling property appears to be the 'T seq

Ver: 0.1 Dotnet Core 3.1.300 Mac OS

fluxlife commented 4 years ago

Btw, you may be in the early stages of this lib, but thank you for your contribution!

fluxlife commented 4 years ago

Another note: 'T array doesn't overflow and the schema is processed however the definition of 'T (represented as Tof in the schema) is simply defined as an empty array. The schema for T does not get generated.

panesofglass commented 4 years ago

Thanks for reporting the issue. I haven't tried this to any real depth. The library mostly relies on the behavior of NJsonSchema and FSharp.SystemTextJson, and the latter was a fairly recent change. I may not have picked up everything I need to address.

fluxlife commented 4 years ago

It's all good. Appreciate the lib!

panesofglass commented 4 years ago

A few more thoughts: I'm surprised by 'T seq. I suspect that is something in NJsonSchema as their System.Text.Json support was still very early, and I had hoped to help them add some tests. This is probably a bug there.

The schema generated for 'T[] should probably be any or {} or quite possibly empty, as you noted. There would be no way to generate the hole unless you specify a concrete type for 'T. Again, I'm not sure what NJsonSchema does for those.

These are good tests for me to add to that project, as I suspect that's where these issues would originate. I would also be curious to know what NJsonSchema does when using Newtonsoft.Json rather than System.Text.Json. Thanks again for raising these. I'll see what I can do in terms of investigation. I am once again working on limited availability for OSS. :(

panesofglass commented 3 years ago

Looks like this is an issue within NJsonSchema: https://github.com/RicoSuter/NJsonSchema/issues/23 and https://github.com/RicoSuter/NSwag/issues/877, though the latter seems to indicate it should work.

panesofglass commented 3 years ago

Looks like I could potentially add something to override open generics with an any, but I'll have to dig in to figure out how that could work.

panesofglass commented 3 years ago

Updated dependencies, and it seems like this has been resolved upstream.