Repository navigation
Fix tracer support for options (was crashing) - #77
Conversation
cd7f403 to
a6b3084
Compare
|
Using a fsproj for the build is fine to me, but could you please add a |
|
Please open an issue proposing the switch fsx -> fsproj instead of taking liberties, let's get this PR to building without the switch. |
a6b3084 to
d0ec296
Compare
The previous implementation could actually crash: if the input is Some(DUCase), Fable.Core.Reflection.isUnion returns true because it does not "see" the option, and only see the wrapped DUCase. Reflection.FSharpType.GetUnionCases() would then attempt to get the fields of the input, and since option types are not considered DU under Fable, it would throw.
d0ec296 to
7330652
Compare
No problem, I removed the commit. EDIT: looks like pinning the build.fsx dependencies versions was enough to fix the build. |
00212d8 to
208dde2
Compare
|
Thank you! |
|
Thanks, published as 4.1.0 |
Also see fable-compiler/Fable#4082
Fix options not being considered as DU in the tracer.
The previous implementation could actually crash:
if the input is Some(DUCase), Fable.Core.Reflection.isUnion returns true
because it does not "see" the option, and only see the wrapped DUCase.
Reflection.FSharpType.GetUnionCases() would then attempt to get the fields
of the input, and since option types are not considered DU under Fable,
it would throw.
Now there is a special case for options so they work the same as other DU for the tracer.
Note: I also took the liberty to replace the build.fsx file with an equivalent build.fsproj project, because I couldn't manage to get it to build and run without errors locally.
With hindsight, maybe it was just a question of pinning the script-referenced nugets to a working version instead of always restoring the latest one, but I didn't try to go back after I got it working with an fsproj. It seems more robust to me that way.
I kept the build changes in their own commit so they can easily be removed if desired...