strip trailing slashes for go links#5023
Conversation
|
Can you please add a test in go.test.js. |
|
jawoll captain |
6af1d1d to
a44f546
Compare
a44f546 to
c516aca
Compare
platform/lib/routers/go.js
Outdated
| }); | ||
| } else { | ||
| simple[key] = value.url; | ||
| simple[`${key}/`] = value.url; |
There was a problem hiding this comment.
what if the key already has a trailing slash?
There was a problem hiding this comment.
These are static strings, it would be /foo/ and /foo//. Handling it felt like the complexity outweighed the use since there are no uses of it currently. It would allow for /foo/ and /foo//.
That being said, I do something like
} else {
key = key.replace(/\/?$/, '/?');
simple[key] = value.url;
}which would force an optional trailing slash (e.g. /?) on all simple routes
There was a problem hiding this comment.
I think it'd be good to consistently support both schemes with and without trailing slash. Stripping trailing slashes when loading the input and when handling a request (like in your first version) might be the easiest way to do this.
There was a problem hiding this comment.
I think it'd be good to consistently support both schemes with and without trailing slash.
makes sense
pushed an update, wdyt
sebastianbenz
left a comment
There was a problem hiding this comment.
Looks good once build passes. Thanks!
fixes #3859