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

Some router paths don't work if router is mounted after defining routes #278

Closed
weedz opened this issue Jun 23, 2021 · 6 comments
Closed
Labels
bug Something isn't working core Affects `@tinyhttp/app` - the tinyhttp core 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt

Comments

@weedz
Copy link

weedz commented Jun 23, 2021

Issuehunt badges

Describe the bug

Can not find the correct route when using sub-apps.

To Reproduce

Steps to reproduce the behavior:

The following code does not work as intended:

import { App } from '@tinyhttp/app';

const app = new App()

const router = new App();
router.get("/list", (req,res) => {
    console.log("router/list");
    res.send("router/list");
});
router.get("/find", (req,res) => {
    console.log("router/find");
    res.send("router/find");
});
app.use("/router", router);

app.listen(3000);

I can't request /router/find but /router/list works.

If I instead "use" router before we define any routes it works:

import { App } from '@tinyhttp/app';

const app = new App()

const router = new App();
app.use("/router", router); // <-- HERE, we "use" router before we add any routes
router.get("/list", (req,res) => {
    console.log("router/list");
    res.send("router/list");
});
router.get("/find", (req,res) => {
    console.log("router/find");
    res.send("router/find");
});

app.listen(3000);

Expected behavior

Should not matter where the use is called? When requesting /router/find you should see router/find instead of Not Found.

Versions

  • node: 14.16.1
  • @tinyhttp/app: 1.3.11

Additional context

Add any other context about the problem here.


IssueHunt Summary

sbencoding sbencoding has been rewarded.

Backers (Total: $5.00)

Submitted pull Requests


Tips

@weedz weedz added the bug Something isn't working label Jun 23, 2021
@talentlessguy talentlessguy added the core Affects `@tinyhttp/app` - the tinyhttp core label Jun 23, 2021
@talentlessguy talentlessguy changed the title Problem with sub-apps/routes Some router paths don't work if router is mounted after defining routes Jun 24, 2021
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jun 25, 2021

@talentlessguy has funded $5.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Jun 25, 2021
@weedz
Copy link
Author

weedz commented Jun 25, 2021

This might not be the correct way to solve this (and might possibly break other things?) but in the App.use function, if we map over all the middlewares instead of only the first one it seems to work.

pushMiddleware(this.middleware)({
  path: base as string,
  regex,
  type: 'mw',
  handler: mount(fns[0] as Handler),
  handlers: fns.slice(1).map(mount),
  fullPaths: fns
  .flat()
  .map((fn) =>
-    fn instanceof App && fn.middleware?.[0] ? lead(base as string) + lead(fn.middleware?.[0].path)  : ''
+    fn instanceof App && fn.middleware?.length ? fn.middleware.map(mv => lead(base as string) + lead(mv.path))  : ''
  )
})

@talentlessguy
Copy link
Member

@weedz could you please fork it and check on some unit tests for router?

@weedz
Copy link
Author

weedz commented Jun 25, 2021

@weedz could you please fork it and check on some unit tests for router?

Sure

@weedz
Copy link
Author

weedz commented Jun 25, 2021

Does not seem to work. Fails at

Testing App routing › should can set url prefix for the application

Returns "Not Found" for /abc/route2 and /abc/route3

@issuehunt-oss issuehunt-oss bot removed the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Jul 12, 2021
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jul 12, 2021

@talentlessguy has rewarded $4.50 to @sbencoding. See it on IssueHunt

  • 💰 Total deposit: $5.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $0.50

@issuehunt-oss issuehunt-oss bot added the 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt label Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core Affects `@tinyhttp/app` - the tinyhttp core 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt
Projects
None yet
Development

No branches or pull requests

2 participants