Get field name from attribute when pattern matching #6456
Conversation
| @@ -26,6 +26,19 @@ open Printf | |||
|
|
|||
| let dbg = false | |||
|
|
|||
| let find_name (attr : Parsetree.attribute) = | |||
There was a problem hiding this comment.
This is similar to find_name in jscomp/core/record_attributes_check.ml. I prefer if this can be extracted to a common place where both places can use the same function, not sure where though.
There was a problem hiding this comment.
Looks identical. Can't you use the one from record_attributes_check.ml?
There was a problem hiding this comment.
Looks identical. Can't you use the one from
record_attributes_check.ml?
I tried, but record_attributes_check.ml is in the core package, and trying to import it from ml causes a circular import.
There was a problem hiding this comment.
I tried, but
record_attributes_check.mlis in thecorepackage, and trying to import it frommlcauses a circular import.
Sorry I'm an idiot, I can reverse the dependency 🤦
|
Looks good to me! Maybe it's tested elsewhere but it'd be good to add a prop that's not renamed as well in the same test, just to be sure it's not changed. @cristianoc any comments? |
|
Looks great! |
|
@shulhi congratulations on a great first contribution! |
Changelog updated. |
Fix for #6438
PS: First time contributor here. It would be nice to get some guidance on a proper way to contribute this fix properly. (I have read the contributing guidelines).