haxetink / tink_web

Tinkerbell Web Framework
https://haxetink.github.io/tink_web
43 stars 13 forks source link

Router parsing optional args #4

Closed skial closed 8 years ago

skial commented 8 years ago

Optional args don't appear to be handled correctly by the router build macro, causing null/undefined is not a valid integer if you use the script below, compiled and run on either node or php, from localhost:8080/bob, while localhost:8080/bob/10 works.

I think the behaviour for the router build macro when encountering optional args should be to attempt to parse the value, as any null value should be handled by the method, instead of throwing an error.

I can attempt to submit a pr, once I've brushed up on my macros.

Using Haxe 3.3.0 and latest git versions of tink_web and tink_http, all other tink_* libs are from haxelib.

// Main.hx
package;

import tink.web.Router;
#if php
import tink.http.containers.PhpContainer;
#elseif js
import tink.http.containers.NodeContainer;
#end

class Main {

    static var routes = new Routes();
    static var router = new Router<Routes>();

    static function main() {
        var c = 
        #if js
        NodeContainer.new.bind(8080)
        #elseif php
        function() return PhpContainer.inst
        #end;
        c().run( function(req) {
            return router.route(routes, req).handleError(function(e) throw e);
        } );
    }

}

class Routes {

    public function new() {

    }

    @:get('/$a')
    @:get('/$a/$b')
    public function foo(a:String, b:Int = 100) {
        return 'hello $b $a world!';
    }

}
# build.hxml
-lib tink_web
-cp src
-main Main

--each

-lib hxnodejs
-js bin/test.js

--next

-php bin/php
back2dos commented 8 years ago

Hey @skial , sorry for the late response.

I took some time to think about this one and I'm inclined to say the current behavior makes more sense. With the change proposed, users may find themselves facing an API where they try all sorts of values for an optional parameter without being able to observe an effect, only to realize that they have been passing malformed values.

I would go even further and enforce the whole value to be a valid numeral (instead of relying on the current parse-anything-intelligible-and-ignore-the-rest type of behavior we have now).

If you want to have a lenient numeric data type, then nothing stops you from just declaring one:

abstract Intish(Int) from Int to Int {
  @:from static function ofStringly(s:tink.web.Stringly) {
    //do what you want here ... you can also interpret things like "first" or "last" or "highest" or whatever
  }
}

Would that cover your use case?

skial commented 8 years ago

That's exactly what I was thinking of doing, whenever I get back to project, I unfortunately haven't touched it in a couple of weeks.

So yes, it will cover my use case :+1:

back2dos commented 8 years ago

Alright, then I guess this is resolved ^^