-
Notifications
You must be signed in to change notification settings - Fork 140
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
feat: add multiple parametric nodes support #320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this affects our benchmarks?
No, all changes are in the compile time. |
87531a6
to
7e102a8
Compare
This PR is ready to review. It allows you to have two identical routes with different param types: general parameter and regexp parameter. Example:
Code in the
If you have any suggestions, you are more than welcome. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some docs for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we defer the sorting to certain point later?
For example, once the first call of lookup
.
It seems to do a lot of unnecessary sorting when there are many routes.
Technically we can, you are right. But you will have this sorting in a very small amount of cases. You a sorting parametric childs of one parent node. IMHO, in 99% of all use cases, you have one parametric child, and in 1%, you have two children. All other cases are less than the statistical error. On the other hand, this will complicate the already complicated code and introduce an additional error in measuring the hot path. I would agree with you 100% if it would make any difference, but you will not notice it. |
At least we can do |
I think that the sort function is smart enough to not call a sorting handler for an array with one item. But I can add this, not a problem at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some docs!
nvm, it is actually smart enough to skip the run. function sorter(a, b) {
console.log('did run')
return a - b
}
;['a'].sort(sorter)
;['a', 'b'].sort(sorter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Close #317