Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange overlap report after adding another route #96

Closed
OndrejSpanel opened this issue Dec 16, 2023 · 7 comments · May be fixed by #113
Closed

Strange overlap report after adding another route #96

OndrejSpanel opened this issue Dec 16, 2023 · 7 comments · May be fixed by #113

Comments

@OndrejSpanel
Copy link
Contributor

Following code prints error when run:

object MinimalApplication extends cask.MainRoutes {

  @cask.get("/root/x")
  def hello() = {
    "Hello World!"
  }

  @cask.get("/:path")
  def doThing(path: String) = {
    path.reverse
  }

  initialize()
}

The error is:

Routes overlap with wildcards: get /root/x, get /:path

When I replace the "/root/x" route with "/", the error disappears (this is how default MinimalApplication looks like).

@lihaoyi
Copy link
Member

lihaoyi commented Dec 16, 2023

This appears to be a limitation in the Mill routing logic; it doesn't realize the /:path and /root/x cannot clash. Should be fixable

@OndrejSpanel
Copy link
Contributor Author

The framework looks very nice to me as an idea (typical Li Haoyi Easy, Boring and Fast), but my first experience is there are some rough egdes which make work unpleasant up to the level of nearly impossible. I hope you will not be annoyed by me raising issues I may have - I will try to stay factual and brief.

@lihaoyi
Copy link
Member

lihaoyi commented Dec 16, 2023

Please do continue to report issues. We need these reports in order to improve the framework, so they are very much appreciated

@OndrejSpanel
Copy link
Contributor Author

OndrejSpanel commented Dec 16, 2023

This one is currently a blocker for me. I am unable to implement OAuth for my app, as I am unable to handle /auth?code=xxxx loopback, as /auth overlaps with @get("/", subpath = true). I use this "serve all" at the moment to serve everything, as this is the only way to handle overlapping paths I have found to work, but this "serve all" is not called once there are any query parameters.

Btw, routing directly is Scala based on remainingPathSegments does not look bad at all:

      request.remainingPathSegments match {
        case Seq("auth") =>
          authRoute(request)
        case Seq("favicon.ico") =>
           resourceRoute("html/favicon.ico").withContentType("image/x-icon")
        case "classes" +: prefix +: path if whiteListClasses.contains(prefix) =>
          resourceRoute((prefix +: path).mkString("/")).withAutoContentType(path.last)
        case Seq(path) if path.endsWith(".js") =>
          jsRoute(path).withContentType("text/javascript")
      }

@OndrejSpanel
Copy link
Contributor Author

OndrejSpanel commented Dec 17, 2023

No longer a blocker, this is my "catch them all" endpoint at the moment:

  @get("/", subpath = true)
  def root(code: Option[String] = None, state: Option[String] = None, request: Request): Response[Response.Data]

It is kind of strange to declare all possible query parameters this way, as I am using them directly from the request eventually, but it works and allows me to proceed.

@jodersky
Copy link
Member

#134 might have also fixed this. Is this still an issue?

@OndrejSpanel
Copy link
Contributor Author

My app uses a single "catch them all" wildcard at the moment and I am working on different things now. The intensions seems reasonable and well thought and it should fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants