-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Warn about root paths without a leading slash (fix #2550) #2591
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.
Thanks! We, however, require a test case for this (showing warning vs not showing). The warning should only appear in development mode and it should display more information like the path that may be incorrect. You can check other warnings for inspiration
@posva Added test cases, moved the change to |
src/create-route-map.js
Outdated
@@ -34,6 +34,16 @@ export function createRouteMap ( | |||
} | |||
} | |||
|
|||
// warn if routes do not include leading slashes | |||
const pathsMissingSlashes = pathList | |||
.filter(path => !!path && path.charAt(0) !== '*' && path.charAt(0) !== '/') |
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.
I think a single find
is simpler. No need for the map and I don't really understand the last filter
call
We can also remove the !!path
check as it's a string
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.
find
is much simpler but unfortunately I do not think it's supported by this version of nightwatch which causes e2e tests to fail.
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.
you are totally right! Thank you for catching that, it would fail on IE as well. Then let's keep the filter
and display the list of failing routes since the information is there already. If the routes could be displayed one per line, that would be perfect as it would be more readable (I don't remember how it was before)
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.
It used to be just comma separated, but one per line sounds better
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.
We are also missing the warning for addRoutes
. Could you add it as well?
@posva Are you referring to the |
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.
True! I forgot it does an in place thing with the same function
Thanks!
Hey @Sayegh7, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚 |
close #2550
This PR creates the following warning message when a root route exists without a leading slash:
This facilitates the debugging process as the current behavior does not generate any warnings or errors.