kwhitley / itty-router

A little router.
MIT License
1.7k stars 77 forks source link

Invalid Regular Expression after fetch #187

Closed anywhichway closed 3 months ago

anywhichway commented 9 months ago

Describe the Issue

Itty throws a regular expression error when trying to process a then callback after fetch if the then source contains an array like access that uses a string with a colon in it.

Note, Hono handles the code fine, but I would prefer to use Itty.

Example Router Code

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

router.fetch(new Request(src)).then(async (response) => {
        console.log([`test:controller`]);
    })

Request Details

Steps to Reproduce

Simply try to run the code with any URL

Expected Behavior

Expect the callback to be executed.

Actual Behavior

This is the error from my actual code. See Additional Context for Hypothesis and testing that resulted in the minimal example above.

Uncaught (in promise) SyntaxError: Invalid regular expression: /^async (response) => { (at itty-router.js:1:114) const string = replaceBetween(await response.text(), "", "", (text) => "" + text\.replaceAll(/</g, "&lt;") + ""), where = el.getAttribute(${prefix}\\((?<target>[^/]+?))) || el.getAttribute("target") || undefined, controller = el.attributes[${prefix}\\((?<controller>[^/]+?))]; if(state) { let content = document.createDocumentFragment(); const div = document.createElement("div"); div.innerHTML = string; while(div.firstChild) content.appendChild(div.firstChild); content = render(el,content,{state, root((?[^/]+?)), where((?[^/]+?))}); update({node((?[^/]+?)), content, state, root((?[^/]+?)), where, recurse: 1}); /if(controller) loadController({el,attribute((?[^/]+?)),state,root,lazui}) } else { render(el, string, {state, root((?[^/]+?)), where, recurse: 1}); if(controller) loadController({el,attribute((?[^/]+?)),state,root,lazui}) } }/*$/: Unmatched ')' at RegExp () at Object. (itty-router.js:1:114) at Object.src (src.js:10:36) at handleDirective (lazui.js?router=router&allowRemote=true:438:57) at lazui.js?router=router&allowRemote=true:457:21 at walk (lazui.js?router=router&allowRemote=true:277:107) at handleDirectives (lazui.js?router=router&allowRemote=true:454:9) at lazui.js?router=router&allowRemote=true:676:42 at Array.forEach () at updater (lazui.js?router=router&allowRemote=true:676:24)

Environment (please complete the following information):

Additional Context

The same code works fine with Hono.

Hypotheses: Something in the source of my code confuses the Itty RegExp.

Validate Hypotheses:

router.fetch(new Request(src)).then(async (response) => {
       console.log(response)
    })
}```

No error, but also nothing logged to console.

Try with //

router.fetch(new Request(src)).then(async (response) => { // test console.log(response) }) }```

No error

Try with embedded RegExp

router.fetch(new Request(src)).then(async (response) => {
        const string = replaceBetween(await response.text(), "`", "`", (text) => "`" + text.replaceAll(/</g, "&lt;") + "`");
        console.log(string);
    })

No error

Try with RegExp and string literal

  router.fetch(new Request(src)).then(async (response) => {
        const string = replaceBetween(await response.text(), "`", "`", (text) => "`" + text.replaceAll(/</g, "&lt;") + "`"),
            where = el.getAttribute(`${prefix}:target`) || el.getAttribute("target") || undefined;
        console.log(string,where)
    })

No error

Try with RegExp, string literal and conditional and [] access

 router.fetch(new Request(src)).then(async (response) => {
        const string = replaceBetween(await response.text(), "`", "`", (text) => "`" + text.replaceAll(/</g, "&lt;") + "`"),
            where = el.getAttribute(`${prefix}:target`) || el.getAttribute("target") || undefined,
            controller = el.attributes[`${prefix}:controller`];
        if(state) {

        } else {

        }
    })

Error similar to that being reported

Try with conditional alone:

 router.fetch(new Request(src)).then(async (response) => {
        if(state) {

        } else {

        }
    })

No error

Try conditional with embedded RegExp:

router.fetch(new Request(src)).then(async (response) => {
        const string = replaceBetween(await response.text(), "`", "`", (text) => "`" + text.replaceAll(/</g, "&lt;") + "`");
        if(state) {

        } else {

        }
    })

No error

Try with conditional and string interpolation:

router.fetch(new Request(src)).then(async (response) => {
        const where = el.getAttribute(`${prefix}:target`) || el.getAttribute("target") || undefined;
        if(state) {

        } else {

        }
    })

No error

Try with [] access

    router.fetch(new Request(src)).then(async (response) => {
        console.log([`test:controller`]);
    })

Error

Try with [] access without : in string

    router.fetch(new Request(src)).then(async (response) => {
        console.log([`testcontroller`]);
    })

No error

Conclusion: Array like access with a string value that includes a ':' confuses the itty-router RegExp

kwhitley commented 4 months ago

I'm very confused by what you have going on here... a couple points:

it('can handle a callback that contains an array-like access containing a colon', async () => {
  const handler = vi.fn()
  const challenge = () => { console.log([`test:controller`]) } // prints fine
  const router = Router().all('*', challenge, handler)

  await router
    .fetch(new Request('http://foo.bar'))
    .then(async (response) => {
      console.log([`test:controller`]) // prints fine
    })

  expect(handler).toHaveBeenCalled() // yup
})
kwhitley commented 3 months ago

Haven't heard back on this one, but again... the fact that this error is resulting from something being done in the .then() block of the .fetch() Promise chain means it's simply not running within the function scope of itty.

Furthermore, those regex snippets you see the error around aren't in itty either. You're seeing a processing issue downstream. Good luck, and I hope you found your issue (and if so, please do share the solution)

In the meantime, closing this issue.