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
Fix transforming of PathBindable and also the anyValPathBindable macro #11712
Conversation
@Mergifyio backport 2.8.x |
✅ Backports have been created
|
|
||
override def unbind(key: String, value: ${t.tpe}): String = { | ||
binder.unbind(key, value.${param.name.toTermName}) | ||
} |
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.
The javascriptUnbind
does not get overridden.
|
||
} | ||
private val binder = _root_.scala.Predef.implicitly[_root_.play.api.mvc.PathBindable[${param.typeSignature}]] | ||
binder.transform((p: ${param.typeSignature}) => new ${t.tpe}(p), (p: ${t.tpe}) => p.${param.name.toTermName}) |
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.
Using transform is preferred, safer and saner. Just like we do below (line 29) for anyValQueryStringBindable
.
} | ||
"JavaScript Unbind Pferd as String which is a Boolean and uses special js unbind" in { | ||
implicitly[PathBindable[Pferd]].javascriptUnbind must equalTo("function(k,v){return !!v}") | ||
} |
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.
This test would fail without both fixes applied.
While working on the macro stuff for Scala 3 I noticed two bugs:
AnyVal
regarding Pathbindable also does not correctly transform an AnyVal. Even when the above is fixed, the macro does create a new instance which does not override the js method... calling transform is the preferred way here.The added tests test both scenarios.