lapis icon indicating copy to clipboard operation
lapis copied to clipboard

route name cannot be one of the paths

Open bungle opened this issue 2 years ago • 2 comments

This used to work before, but now there is this code: https://github.com/leafo/lapis/blob/v1.14.0/lapis/application.moon#L179-L194

    filled_routes = {}

    each_route @, true, (path, handler) ->
      route_name, path_string = if type(path) == "table"
        next(path), path[next path]
      else
        nil, path

      if route_name
        return if filled_routes[route_name]
        filled_routes[route_name] = true

      return if filled_routes[path_string]
      filled_routes[path_string] = true

      @router\add_route path, @wrap_handler handler

This means that if route_name == path_string OR that route_name has collision with other route's path_string the add_route may be skipped.

I have been trying to update this library from 1.8.3 to 1.14.0 in Kong, and found this to be problematic for us as we do: https://github.com/Kong/kong/blob/master/kong/api/api_helpers.lua#L429

app:match(route_path, route_path, app_helpers.respond_to(methods))

Not sure if this was intent, but just letting you know.

bungle avatar May 11 '23 14:05 bungle

This would probably fix it:

    -- this will hold both paths and route names to prevent them from being
    -- redeclared by paths lower in precedence
    filled_route_names = {}
    filled_route_paths = {}

    each_route @, true, (path, handler) ->
      route_name, path_string = if type(path) == "table"
        next(path), path[next path]
      else
        nil, path

      if route_name
        return if filled_route_names[route_name]
        filled_route_names[route_name] = true

      return if filled_route_paths[path_string]
      filled_route_paths[path_string] = true

      @router\add_route path, @wrap_handler handler

bungle avatar May 11 '23 14:05 bungle

This is related to https://github.com/leafo/lapis/issues/760

The route name was intended to be in the form of a lua module name, not a route path.

leafo avatar Feb 29 '24 22:02 leafo