kwhitley / itty-router

A little router.
MIT License
1.75k stars 78 forks source link

Optional format params feature is broken in recent versions since 2.3.10 #66

Closed dbradleyfl closed 2 years ago

dbradleyfl commented 2 years ago

Example code on replit: https://replit.com/@dbradleyfl/itty-optional-format-parsing#index.js

const { Router } = require('itty-router')

const router = Router()

// if you remove the ? below you get different results and this is a regression
router.get('/pages/:id.:format?', (req, res) => {
  return `Params: ${JSON.stringify(req.params)}`
})

router.handle({
  method: 'GET',
  url: 'https://example.com/pages/1',
}).then(console.log)

router.handle({
  method: 'GET',
  url: 'https://example.com/pages/1.html',
}).then(console.log)

// this will log the following
// Params: {"id":"1"}
// Params: {"id":"1.html"}

// if you remove the ? from the route to indicate the format is no longer optional you get the following
// undefined
// Params: {"id":"1","format":"html"}
kwhitley commented 2 years ago

So this whole issue arose around a very frustrating combination of regex requirements...

  1. the ability to have a period in the filename itself (e.g. my.file)
  2. the ability to allow for extension parsing (e.g. my.file.jpg)

Turns out, it's really difficult to do both, especially in an efficient/code-golfed regex manner (kinda key to itty being... you know... "itty"). Since both were relatively undocumented paths, I opted short term to at least not break with periods in the filenames, under the assumption that if you're going down the format path, that you could just declare it non-optionally. It's not ideal, and this will be fixed as soon as we/I solve the regex.

From the test suite:

image image

The regex is a royal pain, because it's really regex that writes regex, that is LATER parsed in the request.handle. If anyone wants to take a crack at solving it (especially without adding a bunch of characters), I'd name someone else's firstborn after the winner! Here's the entire regex for itty:

image

kwhitley commented 2 years ago

This is being fixed in an upcoming PR... lost some bytes in the process, but I'll see if I can regain them later!