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

fix path finding in express #148

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Kenblair1226
Copy link

per #81, borrow from @evelyne24's fix to use req.path if req.route does not exist.

@kibertoad
Copy link
Collaborator

could you please add new test and also check why ci is failing?

@kibertoad
Copy link
Collaborator

Still missing test :)

@Kenblair1226
Copy link
Author

Still missing test :)

Need to study testing framework a little bit...

@kobik
Copy link
Collaborator

kobik commented Feb 1, 2021

@kibertoad anything else is missing?

@kibertoad
Copy link
Collaborator

@Kenblair1226 Would that work correctly when addressing parametrized routes? Wouldn't this result in passing ids etc as a part of route as well?

@Kenblair1226
Copy link
Author

Ya, the params will be treated as path and mostly be ignored since not matching any schema. Any suggestion?

@kibertoad
Copy link
Collaborator

@Kenblair1226 Something like this, I think: #74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants