Fix the Javascript Reverse router. Problem with default values. #888

Merged
merged 1 commit into from Mar 25, 2013

Conversation

Projects
None yet
3 participants
Collaborator

guillaumebort commented Mar 21, 2013

The code generated in the Javascript reverse router in wrong for arguments with a default value.

Try:

GET  /       controllers.Application.index(limit: Int =? 10)

And try Js reverse router for this call:

Router.controllers.Application.index()

It gives

/?10

Because anyway the default values are fixed server side, the client doesn't need to repeat the value. The simplest fix is to omit completely non-specified arguments that have a default value server side.

Owner

jroper commented Mar 21, 2013

Looks good to me.

@jroper jroper added a commit to typesafehub/playframework that referenced this pull request Mar 22, 2013

@jroper jroper [#888] Implemented new HTML escaping 78ba56e

jroper merged commit 887fc04 into playframework:master Mar 25, 2013

Contributor

julienrf commented Mar 26, 2013

Why is it different for parameters with fixed value? Their value is fixed by the routing process on server side, right?
So maybe we should just get rid of all the JavaScriptLitteral stuff?

Contributor

julienrf commented Apr 3, 2013

Nothing about my remark?

Collaborator

guillaumebort commented Apr 3, 2013

Yes it should be the same.

Contributor

julienrf commented Jun 21, 2013

I just took a look and I think JavascriptLitteral is needed to make unambiguous the reverse routing of an action that is associated with several URLs:

GET    /foo  p.c.a(foo = "foo")
GET    /bar  p.c.a(foo)

When I reverse route p.c.a("bar") in JavaScript we must to check, from the client-side, the value of the parameter in order to generate the right URL (here, /bar).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment