-
Notifications
You must be signed in to change notification settings - Fork 311
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
Enable erase unions with tags #3282
base: main
Are you sure you want to change the base?
Conversation
396e154
to
de0eca8
Compare
Hi @alfonsogarciacaro , I had a quick look at this but not sure I have anything conclusive to add. A couple of observations: Line 812 - Previously the Erase;Emit type would not output the entity at all, but now it is creating a trait (it thinks it is now a real interface). I imagine this might be because the behaviour of Any has changed. I wonder if we could go through instances of Any in the FableToRust transform, and where Any was previously used to handle the erased scenario, we replace them with some kind of active pattern to any entity that has an emit attribute? I imagine part of the problem is Any's are not considered to be wrapped in a RC but managed types such as traits and records are (by shouldBeRefCountWrapped). We made a design decision to keep Emit dumb (and unwrapped), so you would add your own ref/deref/clone operators as needed in the template. This might explain the as_ref calls. Sorry my time is a bit sparse at the moment, but I will try and dig in a bit further in the next few days. |
@alfonsogarciacaro Hmm, previously @alexswan10k The reason we can't just use |
I initially was under the impression that Erasing a record type would default usage of it in JS/TS as POJOs. In my mind, it seems like it should be a natural conclusion. We can construct POJOs with Anon Records. If we erase a Record type from transpilation, then why would it not operate like a Anon Record would. I'm probably missing some fundamental concept with erasure though :P |
Because users expect declared records to be compatible with many of the functions available in F#/.NET like (explicit) interface implementation, type testing or reflection. And for this you need information about the type at runtime. |
Erase
attribute, compiling them as plain JS objectsErase
unions where multiple cases with multiple were not allowed. For this, I introduced thetag
argumentFeatures 1 and 2 basically make possible what we already tried in the past as an opt-in.
The last feature is the trickiest one because several patterns were relying on erased types being
Any
in Fable.AST. I managed to fix it for JS/TypeScript but for other languages I'm still outputtingAny
until the tests are fixed. Another thing to note is I'm wrapping new erased unions with a type cast so the type info is not lost.@dbrattli If you need the info from erased types, please add Python here and check which tests fail. @ncave Unfortunately I broke the Rust tests again 😞 could you please have a look?