optimize route building when there's query params. (#1854)

* optimize route building when there's query params. Fixes #1831

* Adding a deprecation warning to update Crystal beyond 1.10

* The min Crystal version is now 1.9 due to the TypeNode#warning

.github/workflows/ci.yml

@@ -18,7 +18,7 @@ jobs:
       - uses: actions/checkout@v1
       - uses: crystal-lang/install-crystal@v1
         with:
-          crystal: 1.6.2
+          crystal: latest
       - name: Install shards
         run: shards install
       - name: Format
@@ -32,7 +32,7 @@ jobs:
         shard_file:
           - shard.yml
         crystal_version:
-          - 1.6.2
+          - 1.9.0
           - latest
         experimental:
           - false

shard.yml

@@ -1,7 +1,7 @@
 name: lucky
 version: 1.1.0
 
-crystal: ">=1.6.0"
+crystal: ">=1.9.0"
 
 authors:
   - Paul Smith <[email protected]>

src/lucky/routable.cr

@@ -288,29 +288,38 @@ module Lucky::Routable
     {% end %}
     anchor : String? = nil
     ) : Lucky::RouteHelper
-      path = path_from_parts(
-        {% for param in path_params %}
-          {{ param.gsub(/:/, "").id }},
-        {% end %}
-        {% for param in optional_path_params %}
-          {{ param.gsub(/^\?:/, "").id }},
+      path = String.build do |io|
+        path_from_parts(
+          io,
+          {% for param in path_params %}
+            {{ param.gsub(/:/, "").id }},
+          {% end %}
+          {% for param in optional_path_params %}
+            {{ param.gsub(/^\?:/, "").id }},
+          {% end %}
+        )
+        query_params = {} of String => String
+        {% for param in PARAM_DECLARATIONS %}
+          # add query param if given and not nil
+          query_params["{{ param.var }}"] = {{ param.var }}.to_s unless {{ param.var }}.nil?
         {% end %}
-      )
-      query_params = {} of String => String
-      {% for param in PARAM_DECLARATIONS %}
-        # add query param if given and not nil
-        query_params["{{ param.var }}"] = {{ param.var }}.to_s unless {{ param.var }}.nil?
-      {% end %}
-      unless query_params.empty?
-        path += "?#{HTTP::Params.encode(query_params)}"
-      end
+        unless query_params.empty?
+          io << '?'
+          {% if compare_versions(Crystal::VERSION, "1.10.0") < 0 %}
+            {% @type.warning("[Deprecated] Please update your Crystal version #{Crystal::VERSION}. Using Lucky with a version below 1.10.0 is deprecated.") %}
+            io << HTTP::Params.encode(query_params)
+          {% else %}
+            HTTP::Params.encode(io, query_params)
+          {% end %}
+        end
 
-      anchor.try do |value|
-        path += "#"
-        path += URI.encode_www_form(value)
+        anchor.try do |value|
+          io << '#'
+          URI.encode_www_form(value, io)
+        end
       end
 
-      Lucky::RouteHelper.new {{ method }}, path
+      Lucky::RouteHelper.new({{ method }}, path.presence || "/")
     end
 
     def self.with(
@@ -344,6 +353,31 @@ module Lucky::Routable
       \{% end %}
     end
 
+    private def self.path_from_parts(
+      io : IO,
+      {% for param in path_params %}
+        {{ param.gsub(/:/, "").id }},
+      {% end %}
+      {% for param in optional_path_params %}
+        {{ param.gsub(/^\?:/, "").id }},
+      {% end %}
+    ) : Nil
+      {% for part in path_parts %}
+        {% if part.starts_with?("?:") %}
+          if {{ part.gsub(/^\?:/, "").id }}
+            io << '/'
+            URI.encode_www_form({{ part.gsub(/^\?:/, "").id }}.to_param, io)
+          end
+        {% elsif part.starts_with?(':') %}
+          io << '/'
+          URI.encode_www_form({{ part.gsub(/:/, "").id }}.to_param, io)
+        {% else %}
+          io << '/'
+          URI.encode_www_form({{ part }}, io)
+        {% end %}
+      {% end %}
+    end
+
     private def self.path_from_parts(
         {% for param in path_params %}
           {{ param.gsub(/:/, "").id }},
@@ -352,21 +386,16 @@ module Lucky::Routable
           {{ param.gsub(/^\?:/, "").id }},
         {% end %}
     ) : String
-      path = String.build do |path|
-        {% for part in path_parts %}
-          {% if part.starts_with?("?:") %}
-            if {{ part.gsub(/^\?:/, "").id }}
-              path << '/'
-              URI.encode_www_form({{ part.gsub(/^\?:/, "").id }}.to_param, path)
-            end
-          {% elsif part.starts_with?(':') %}
-            path << '/'
-            URI.encode_www_form({{ part.gsub(/:/, "").id }}.to_param, path)
-          {% else %}
-            path << '/'
-            URI.encode_www_form({{ part }}, path)
+      path = String.build do |io|
+        path_from_parts(
+          io,
+          {% for param in path_params %}
+            {{ param.gsub(/:/, "").id }},
           {% end %}
-        {% end %}
+          {% for param in optional_path_params %}
+            {{ param.gsub(/^\?:/, "").id }},
+          {% end %}
+        )
       end
 
       path.presence || "/"