search

kwhitley / itty-router

A little router.
MIT License
1.81k stars 79 forks source link

Optional route path not supported anymore or missing documentation in migration guide #179

Closed Cali93 closed 1 year ago

Cali93 commented 1 year ago

Describe the Issue

In previous versions (3.x and lower) it was possible to have optional route paths using the following pattern/(api?)/v1/my-route. So making a GET request on both /api/v1/my-route and /v1/my-route would work and would be handled as expected. However v4 introduces a breaking change as the optional path isn't supported anymore.

Example Router Code

Please provide the itty-router code related to the issue. If possible, create a minimal, reproducible example.

// This was used to work on 2.X and 3.X
router.get('(/api)?/v1/:something', getSomething)

Request Details

Steps to Reproduce

Steps to reproduce the behavior:

  1. Run a wrangler that has the following route router.get('(/api)?/v1/:something', getSomething)
  2. Send request to /api/v1/my-route or /v1/my-route
  3. See error 
    FetchError [ERR_NO_RESPONSE]: No fetch handler responded and no upstream to proxy to specified.
Make sure you're calling `event.respondWith()` with a `Response` or `Promise<Response>` in your handler.

Expected Behavior

Making a request to both /api/v1/my-route and /v1/my-route should work as it is on 2.x and 3.x for the following route /(api?)/v1/my-route

Actual Behavior

FetchError [ERR_NO_RESPONSE]: No fetch handler responded and no upstream to proxy to specified.
Make sure you're calling `event.respondWith()` with a `Response` or `Promise<Response>` in your handler.

Environment (please complete the following information):

Additional Context

Add any other context about the problem here.

kwhitley commented 1 year ago

Taking a look now, but gotta warn you - no promises on that one. You're using undocumented (and not officially supported) syntax that happened to work before, but certainly not by plan :) Basically some regex works by happy accident, but others get mangled in the rewrite process - just depends on what characters/patterns you're using!

If I can make it work for you though, I will, because I think that's a pretty cool switching syntax...

kwhitley commented 1 year ago

So, 4.0.14 should have test coverage in place to verify your functionality, and it should have been restored :)

image

Cali93 commented 1 year ago

Amazing, thanks a lot! 🚢