-
Notifications
You must be signed in to change notification settings - Fork 19
Fix tracer support for options (was crashing) #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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...