-
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
Bring back [<POJO>]? #2421
Comments
A record can already be used as input props for React components, except that due to limitations of react-refresh (the hot module reloading plugin) the record type must be defined in a separate module module PropTypes
type Foo = { A: string; B: int } It work fine if you define the record type in the same module but during development when you edit your component, it will reset the state instead of just rerendering
I think it was removed because it behaved differently than normal records in structural equality and reflection.
@alfonsogarciacaro would it be possible to handle |
@ncave is working on #2279 to extend the "erase declaration" feature (currently only available for some unions with the We're just in doubt if the feature:
In the case of 1. we should probably add a |
@alfonsogarciacaro Technically #2279 is to erase to named tuples (arrays), not POJOs. But your point about attached members is a valid one, I'm still unsure how to proceed about that. For both tuple arrays and POJOs, can we attach members to the object itself, not to the prototype? Or is that going to bloat it too much? We don't have that many attached members anymore, so it may be ok, what do you think? |
Ah, ok! I thought records were being erased to POJOs 😅 This adds another variable to the equation, some people may want to erase records as POJOs for better compatibility and other as arrays for maximum bundle size efficiency 🤔 Hmm, maybe this complicates things but probably the safest way for now is to introduce the feature in a per-case basis with the In any case, as discussed in the PR, I'd like to remove |
@alfonsogarciacaro It's already removed, there are no AST modifications in the #2279, the erasing has been moved to Fable2Babel. It's still unfinished and WIP though. |
Ups, I should have checked the PR better 😅 I've been having a look but it seems my hope of just extending the behavior of |
Sorry for the delay! I'll try to summarize the current situation and the alternatives to add more possibilities to erase class declarations in the generated JS for unions/records. Currently
ProposalAllow removing the declaration of more records/unions by either a compiler flag or specialized attributes. This can have the following pros:
But there will be also the following cons:
How to erase unionsIf we want to keep access to the case name, we should likely always erase to a JS array where the first item is the case name. Or if we make the first item be the tag we can use the reflection info to access the case name when needed. In any case, the main drawback of always erasing to an array is it would conflict with the current behavior of How to erase recordsTwo main options:
Should we decide on one of the two ways? Or give the user the ability to choose either? |
Description
In numerous occasions, I find myself having to use anonymous records for a POJO and this discards a whole bunch of type-safety and conciseness (destructuring & reusability) when I'm able to re-use a defined type. I noticed in older examples there's a
[<POJO>]
attribute which is exactly what I'm in need of, and I've also seen a couple of other people mention this in the Slack too.This would obviously open up destructuring, but also allow a record as React component props (I think?). Additionally, it makes interop a lot easier as it doesn't mean dropping down to potentially unsafe anonymous records.
Repro code
would become something like
Was this removed for a technical reason or was it just to remove the amount of options?
Would you be open to this returning?
The text was updated successfully, but these errors were encountered: