-
Notifications
You must be signed in to change notification settings - Fork 468
Add Jsx.addKeyProp function #5664
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
Conversation
|
||
let addKeyProp (o : 't) k = | ||
Obj.magic (Js.Obj.assign (Obj.magic o) [%obj { key = k }]) | ||
[@@inline] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether [@@inline]
is correct or not. I'm wondering [@@bs.inline]
is current, but it fails to be compiled. Unused attribute ...
error is thrown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Js output seems incorrect either.
// generated
Jsx.addKeyProp({ x: "x" }, { key: "k1"})
// expected
Object.assign({ x: "x" }, { key: "k1"})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same:
let is_inline : attr -> bool =
fun ({ txt }, _) -> txt = "bs.inline" || txt = "inline"
I think it's more like inline is not so well specified. Not even sure it works across files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would assume a bundler would inline it anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would assume a bundler would inline it anyway?
Worth checking. The current output is working fine without any issues in the example project anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK to merge?
This PR adds
Jsx.addKeyProp
in order to add the value of key into the props for React API. This function is needed to pass the type checking to the React component's props type which doesn't havekey
anymore. rescript-lang/syntax#635