-
Notifications
You must be signed in to change notification settings - Fork 8
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: handle undefined prevroute during createStore/firstRoute #27
base: master
Are you sure you want to change the base?
Conversation
Excellent, thanks! rudy/packages/rudy/src/core/createRequest.js Lines 59 to 60 in 3492cac
It looks like I prefer the idea that if there is no previous route,
If you wanted to work on that there's some unit tests here you can use as an example: 77ad467 Otherwise I'll aim to do it soon. |
@@ -21,6 +21,7 @@ export default (name, config = {}) => (api) => { | |||
|
|||
return (req, next = noOp) => { | |||
const route = prev ? req.prevRoute : req.route | |||
if (!route) return next() |
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.
awesome find
@hedgepigdaniel lets catch up I think it's time i get some skin in this game with you |
@stevelr Any chance you can create a simple reproduction of this? I'm struggling to understand how and why route could be undefined in that case. |
Some background:
|
@stevelr bump on reproduction |
I'm the last one mentioned here so I hope this wasn't blocked on me. I'm not a rudy user and I don't know if this is even still relevant. I shouldn't be on the decision chain for this. |
Inside createStore, during execution of firstRoute(), req.prevRoute is undefined, so when the middleware chain gets to call("beforeLeave", { prev=true }), route becomes undefined, and it crashes with "no beforeLeave on undefined".
I've been using rudy for over a month without running into this, so there is probably another issue in my code or build process, but this change made it work for me.