-
Notifications
You must be signed in to change notification settings - Fork 1
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
Consider using itty-router #9
Comments
I'm a big +1 on using Other routers that exist, that we should consider if itty-router is not sufficient, before we maintain our own |
IMO we should have our own router since we are so committed to cloudflare workers. The nft.storage router was made really early when we started and required very little maintenance but allowed us to tweak it to our needs when we needed it. We dont need to go and change all the worker to this new router but anything new IMO should use it. It's already faster, more flexible, has better types (easier to change if we need to) and we can easily that all the good ideas from the alternatives to improve it. |
What do you benchmark bro? |
We should use the same router for all our things. I think we have more things that use itty-router at this point. I think we should use it more. But If we are gonna go in on our own router we should publish it properly as a thing with a guide and docs. |
req/s on both, its kinda hard to see the improvement on a small set of routes but it seems about 5% faster on a small sample and way more stable on the latency distribution probably because it doesnt use |
We have a custom router implementation here. I know we use it (or a similar version) in nft.storage API. However, in most of other places (w3s API, nftstorage.link gateway, nftstorage.link API, minibus, ...) we use
itty-router
.From what I know, we are happy with it and I think we should go with it instead of maintaining our own implementation.
With this, we could just export common middlewares like
withCorsHeaders
making usage of common building blocks on top ofitty-router
much easier.Thoughts @hugomrdias ?
The text was updated successfully, but these errors were encountered: