reason-react

reason-react copied to clipboard

It doesn't follow the principle of least surprise because now the string literals will be interpreted differently by the PPX based on where they appear on the AST. I'm not sure it's a good tradeoff for the sake of saving a few keystrokes, or making ReasonReact component be more like JS ones, if those are the goals.

Indeed these are the tradeoffs we knew we'd face when we first thought about this proposal. As you mentioned, it's also the main concern highlighted in https://github.com/reasonml/reason/pull/1910.

Let me try to offer a perspective that I don't think has been shared yet:

• JSX (and Reason's JSX, by extension) is, effectively, a DSL (domain-specific language). Or it can be made to be a DSL through PPX interpretation.
• It can probably be explained that everything captured under a JSX element's children treats literals in a special way, and that they're inferred syntactically rather than at type-checking time

Now, probably the reasonable thing to do would be to gather some data of the ratio between calling React.string on literals vs. on bindings. And then assess whether that ratio is sufficiently disparate to justify implementing this proposal.

How else are you thinking about the tradeoffs here? I'd be curious to learn what tradeoffs I'm missing.

Nov 09 '23 02:11 anmonteiro

Now, probably the reasonable thing to do would be to gather some data of the ratio between calling React.string on literals vs. on bindings. And then assess whether that ratio is sufficiently disparate to justify implementing this proposal.

I can take this PR for a ride on Ahrefs codebase and see what % of usages of React.string or other converters can be removed, this also might help us surface some unknown problems. There are ~9k usages of React.string in the codebase.

But it'd have to be after the migration to React 18 / latest reason-react, which is currently ongoing.

Nov 09 '23 08:11 jchavarri