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

Add option directorySlash to redirect with slash #125

Closed

Conversation

bjornharrtell
Copy link

Aims to implement support for #81.

@@ -108,6 +109,10 @@ async function send (ctx, path, opts = {}) {
// and not require a trailing slash for directories,
// so that you can do both `/directory` and `/directory/`
if (stats.isDirectory()) {
if (directorySlash && !trailingSlash) {
ctx.redirect(ctx.request.path + '/')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍, can you write the test !!

Suggested change
ctx.redirect(ctx.request.path + '/')
ctx.redirect(`${ctx.request.path}/`)

@niftylettuce
Copy link
Contributor

I don't know about this - I think just using koa-no-trailing-slash package is fine.

@bjornharrtell bjornharrtell deleted the bjornharrtell-patch-1 branch May 16, 2020 19:03
@liqi0816
Copy link

liqi0816 commented May 24, 2020

Hi @niftylettuce Would you please reconsider this feature? koa-send is behaving significantly differently from (the default of) other major static servers which is confusing.

When accessing /dirname:

  • Nginx: 301 redirect to /dirname/ doc
  • Apache: 301 redirect to /dirname/ doc with reasoning
  • npm http-server: 302 redirect to /dirname/
  • npm node-static: 301 redirect to /dirname/

...and

  • koa-send: no redirection and breaks any further relative paths.

I feel it reasonable to expect a built-in redirection since that is other static servers do. Monkey patching works, but is annoying.

@liqi0816
Copy link

I don't know about this - I think just using koa-no-trailing-slash package is fine.

For anyone who is interested, koa-no-trailing-slash removes the trailing slash instead of adding one, and has been archived.

The library that adds a trailing slash is koa-add-trailing-slashes, except it does not check whether the path is a directory or not and adds a slash anyway, so still something will break.

@niftylettuce
Copy link
Contributor

@liqi0816 can you just add a PR to koa-add-trailing-slashes or write your own package for this?

@liqi0816
Copy link

I can, but may I know if there is a reason koa-send need to be different from other static servers? It is breaking people's assumptions.

@liqi0816
Copy link

Just verified express.static also does a 301 redirect. https://github.com/expressjs/serve-static/blob/master/index.js#L182

I understand koa is not meant to be compatible with express or whatever other servers, but at least people are expecting similar functionalities. It would be really great if koa-send aligns with them.

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

Successfully merging this pull request may close these issues.

4 participants