-
Notifications
You must be signed in to change notification settings - Fork 347
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
Having to use stringToElement() is awkward #138
Comments
This is also somewhat related - The docs here make it seem like it should be possible to do this (though it wasn't intentional according to the docs on this):
It could help to point out why it's needed by linking ahead to where the docs talk about the render function, and perhaps point out the trick of making an alias to the function with a shorter name. |
FWIW in internationalized apps we'll already want to write something else over it, i.e. let i18n = FormatMessage.translate |> ReasonReact.stringToElement <div className="title">
(i18n("What to do"))
</div> |
Part of what makes Reason/OCaml great imho, is the lack of type coercion. The cognitive load is low, you don't have to learn tons of magic rules like in JS, you have to be explicit. Introducing an exception just for React risks taking Reason from the familiar syntax, into familiar syntax and behaviour territory. And the behaviour in JS is what makes it such a mad language sometimes.
This is because React embraces JS way of writing code. Input a 🍌 into a function, tons of validation rules run, and it will attempt to do what it thinks is best. While very handy, it gets confusing very quickly. Personally, 👎 to most of the proposal, however this bit:
looks very sensible. |
@kujon |
JSX is the exception. It's markup, not code. Nobody has ever complained about JSX being a mad language; it was never broken, and it didn't need Is there a technical reason we can't have standard JSX? With all (sincere!) gratitude and respect to the Reason team, what keeps you from addressing an issue that has nearly 50 thumbs-ups and is clearly a big turn-off for potential newcomers? Is there anything that can be done to help? |
Can I ask Reason experts to clarify what this has to do with type coercion? I don’t propose to coerce string to an object. I think what I’m proposing is more like making a node to be a union type of string and element. Which is my mental model from actual React. |
JSX is just a syntax sugar for React.createElement('div', null, false);
React.createElement('div', null, 42);
React.createElement('div', null, 'dsadasd'); All Reason is doing here is just telling you that a child must be an array of React elements and putting you in charge of converting your data structures to it. This is not that different to how you perform other conversions: /* won't work */
let foo = "4" ++ 4;
/* this will */
let foo = "4" ++ string_of_int(4); IMHO, we shouldn't try to make Reason behave like JS, because (as a JS guy myself), I feel like we've got a lot to learn from the OCaml community about how type-safe code works. Edit: @gaearon this wasn't meant to clarify your point, but the point @skosch made. As a Reason rookie, I'll attempt clarifying yours though: OCaml doesn't have a concept of |
So, not having looked into this in nearly enough depth – is this, in the end, simply a limitation of OCaml's type checker? I.e., that @gaearon's "union type of string and element" simply can't be done without explicit type annotations of some sort, because OCaml neither provides type introspection nor typeclasses-like polymorphism that will implicitly pick the right instance? And therefore standard JSX behaviour is just completely out of reach, at least with the current ppx infrastructure? |
@skosch that's correct. |
@gaearon I'm not an expert but I think I can explain.
It sounds like you're thinking reason-react/ReactMini/src/React.re Lines 57 to 59 in 9e344b8
Right now reason-react/src/ReasonReact.re Line 20 in 9e0d5c3
Although JSX is a ppx (compiler macro) reasonml/reason#1910 (comment) sounds like it doesn't receive typings therefore it could only coerce string literals. I agree it's awkward for newcomers. |
@gaearon This has been fixed and is now Would love to have you give Reason another go-around 😄 Thanks! Closing out for now. |
Even better imo |
See this thread: https://mobile.twitter.com/Stealtheritz/status/938224539330326528
Warning: subjective opinions ahead!
This doesn't impact me directly, but having to use
stringToElement()
to add text always felt like "appeasing the compiler" kind of work that I would expect from a language like Java, not Reason. :-)I have two issues with this API:
{type, props}
. A string is a string. You can't turn it into an element. A string is a React node though. You don't need to convert it to a node—a string is already a valid React node type. But if the compiler insists, I could see myself writingReasonReact.text()
for this.By the way, I know I can write
let s = ReasonReact.stringToElement()
at the top. This feels like an afterthought to me. Yes, I could, but this just disguises the fact that the API is not ergonomic.The text was updated successfully, but these errors were encountered: