-
Notifications
You must be signed in to change notification settings - Fork 464
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
External binding to component without @react.component
#5725
Comments
@react.component
@react.component
I think this is not an issue. This is the matter of what you bind to. So, the proper binding is to the component not function. // binding to the function
type props<'msg> = {msg: 'msg}
@module("@foo/bar")
external make: props<_> => React.element = "SomeComponent"
// binding to the component
type props<'msg> = {msg: 'msg}
@module("@foo/bar")
external make: React.componentLike<props<string>, React.element> = "SomeComponent" What do you think @cristianoc @cknitt ? |
Encouraged binding for external seems: type props<'msg> = {msg: 'msg}
@module("@foo/bar")
external make: React.component<props<_>> = "SomeComponent" |
I do not understand the reason for the difference in the output. After all,
If there is no way to "fix" the output, then this needs to be well documented. |
This change has the same effect: type compType<'t> = props<'t> => React.element
@module("@foo/bar")
external make: compType<_> = "SomeComponent" so this looks like a behaviour of external declarations. |
Here's an even simpler example of the same phenomenon the does no use anything: no JSX, no records: module MMM = {
type props = string
@module("@foo/bar")
external make: props => React.element = "SomeComponent"
}
let ddd = React.createElement(MMM.make, "message") |
I guess this is caused by different lambda IR. module React = {
type element
type compType<'props> = 'props => element
let createElement = (comp, props) => comp(props)
}
module M = {
type props = string
@module("@foo/bar")
external make: React.compType<props> = "SomeComponent"
}
module MMM = {
type props = string
@module("@foo/bar")
external make: props => React.element = "SomeComponent"
}
let d = React.createElement(M.make, "message")
let ddd = React.createElement(MMM.make, "message")
It seems that the different lambda generates different js expressions. |
This is the expected output by different external declarations. It seems not an issue. I'm going to close it. |
@fhammerschmidt and I just ran into this when writing bindings for MUI 5.0. I think the root cause for this issue is that the type definition for
But it's simply not true that every React component is of the form So actually /cc @mununki @cristianoc |
I agree that it would make users confused. I'm curious what issues are going to be raised when we change the type definitions of |
The JsxC/JsxU modules currently have type componentLike<'props, 'return> = 'props => 'return
type component<'props> = componentLike<'props, element>
/* this function exists to prepare for making `component` abstract */
external component: componentLike<'props, element> => component<'props> = "%identity" The comment suggests that there was already previously the idea of making the So if we had type componentLike<'props, 'return> = 'props => 'return
type component<'props>
external component: componentLike<'props, element> => component<'props> = "%identity" then the @react.component (~x, ~y=3+x, ~z=?) => body would need to be transformed to type props<'x, 'y, 'z> = {x: 'x, y?: 'y, z?: 'z}
React.component(({x, ?y, ?z}: props<_, _, _>) => {
let y = switch y {
| None => 3 + x
| Some(y) => y
}
body
}) Right? |
Hey folks. I want to provide some historical context here. Obviously React.component should be an abstract type on the JS side and so, for that reason alone, modeling it correctly in ReScript is worthwhile. But the original move toward making it abstract was because of the ReScript compiler. Having functions that return functions makes ReScript do interesting curry/application things that are often not what people intended when writing component factories. By annotating components as an alias instead of as a function it deoptimized that compiler behavior and made the code act as expected. So the alias was added because it was a much smaller breaking change and intention was to mod over to an abstract type eventually (which has tons of headaches). Also want to mention that the annotation itself is important not just for the ReScript transform but also the couple things it does for JS debug ability. If you remove it then you lose component names in the React Dev tools and lots of stuff becomes more difficult to understand. On the external side it's not as important, but you needed it to do handle non-function components. Edit because I was rambling. I think this is why you get the function wrapper on externals^^ it was a stopgap for issues with component factories that returned JS lib components that aren't functions. The function wrapper from @react.component makes it so all externals are functions and the type alias kinda protects against compiler optimizations. I haven't read JSX4 changes completely (yet) so I mostly don't know what I'm talking about :) but abstract type would solve all of that and be a super annoying breaking change. |
Hi @rickyvetter, thanks for the historical context! 👍 Could you elaborate a bit on the "tons of headaches"? (BTW, there is already a breaking change in JSX4 for ReScript 11 / @rescript/react 0.12.0 anyway because of #6238 / rescript-lang/rescript-react#94.) |
It was a big social change to ask people to use React.component(...) when there was a lot of angst over string. Also the modding of externals couldn't be fully automated and was pretty awkward because of make+makeProps needing to still exist. I actually think JSX4 makes that part of it quite a bit simpler. Social part of modding extra wrappers into code bases is still rough. |
Thank you for explaining @rickyvetter |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
https://forum.rescript-lang.org/t/call-for-help-2-test-jsx-v4/3781/22?u=moondaddi
As of JSX v4, the component can be written without
@react.component
. The issue came from the binding to the component with external.The output after transformation
@react.component
is different from the above.The generated js output gets different between
componentLike<'props, element>
andprops<_> => React.element
.The text was updated successfully, but these errors were encountered: