Allow Configurable Serializer Function (Solves #153)

[jordanstephens]

Here's a first pass on a configurable serializer for issue #153.

Usage is as described in that issue.

JsRoutes.setup do |config|
  config.serializer = "myApp.myCustomSerializerFn"
end

Also note that I removed a bunch of "if jQuery present" specs as they should no longer be relevant.

If everything looks good to you guys, I'll update the readme.

Let me know if you guys have any questions.

Thanks!

[bogdan]

I think we should use nil here. This value feels too magical if someone will try to debug it.

[jordanstephens]

@bogdan If we set the default to nil here, we will have to do a check somewhere else to set the default initializer. It has to get set somewhere, we can't just have a nil initializer floating around. Where would you prefer it get set?

Something like this on line 108?

js.gsub!("SERIALIZER", @options[:serializer] || "this.default_serializer")

I think setting in the defaults is better than this option. This feels like a magic value. And really, this.default_serializer is the default—not nil. It doesn't make sense to have a nil initializer.

[bogdan]

The way we can make it clear is to make default_serializer a public method and let default be JsRoutes.default_serialize. In this way people can clearly see where is the default and even test in console it in console comparing to other serializers.

[jordanstephens]

@bogdan I'm not totally sure I follow. How does this work with the configurable namespace? We can't set the default to a public method on JsRoutes because we don't know that JsRoutes exists.

It might be better to just make default_serializer public when we run createGlobalJsRoutesObject, and then just add a README note that it is public on the namespace that the user defines, rather than try to get the default value to include the name of the configurable namespace.

[bogdan]

Hm, you are right. namespace config value makes it not static. Let me think till tomorrow.

[le0pard]

js.gsub!("SERIALIZER", @options[:serializer] || "null")

and in coffeescript:

serializer = SERIALIZER
unless serializer?
  #use default

[bogdan]

I still vote for nil as default and implementation as @le0pard suggested.

[jordanstephens]

@le0pard @bogdan alright, I set the default to nil, and moved the override logic into the javascript.

I think you guys are making the wrong call setting the default value to be a nonsense nil value rather than the actual default, and then hiding the override logic in the javascript. I don't see the benefit to the increased complexity in doing it this way. However, as it's your project, I'll follow your lead here.

I hope we can get this merged in soon.

Thanks

[le0pard]

Funny, @jordanstephens . You think line "this.default_serializer" should mean something for developers, but one condition can slowdown all codebase :)

I think to not set initializer each time in function variable with serializer, we can cache it in Utils in this way (also better check, what this defined serializer is really function, and not something else):

custom_serializer: SERIALIZER

serialize: (object) ->
  if @custom_serializer? and @get_object_type(@custom_serializer) is "function"
    @custom_serializer(object)
  else
    @default_serializer(object)