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

feat: support page with trailing '.html' #111

Merged
merged 4 commits into from
Jul 1, 2020

Conversation

curbengh
Copy link
Contributor

@curbengh curbengh commented Dec 28, 2019

TL;DR compatibility with pretty_urls.trailing_html = false

Currently hexo-server supports redirects like,

// source is located at source/bar/index.md
foo.com/bar -> foo.com/bar/ => foo.com/bar/index.html
// -> is redirects
// => is transparent proxy

But not when source is located at source/bar.md, which requires url to be foo.com/bar.html. This can be an issue when pretty_urls.trailing_html is disabled.

With this PR,

foo.com/bar => foo.com/bar.html

Existing redirect/proxy still works.
Also tested with custom root: /root/ and server.compress.


Edit:

File: /foo/bar/index.html
Request: /foo/bar/
Respond: No redirect, serves file directly

File: /foo/bar/index.html
Request: /foo/bar
Respond: Redirect to /foo/bar/ and serve using example 1.

File: /foo/bar/index.html
Request: /foo/bar/index.html
Respond: If pretty_urls.pretty_urls.trailing_index === false, redirect to /foo/bar/ and serve using example 1; otherwise, no redirect and serves /foo/bar/index.html.

File: /foo/bar.html
Request: /foo/bar
Respond: No redirect, serves file directly

File: /foo/bar.html
Request: /foo/bar/
Respond: Redirect to /foo/bar and serve using example 4.

File: /foo/bar.html
Request: /foo/bar.html
Respond: If pretty_urls.pretty_urls.trailing_html === false, redirect to /foo/bar and serve using example 4; otherwise, no redirect and serves /foo/bar.html.

@curbengh curbengh requested a review from SukkaW December 28, 2019 06:05
@coveralls
Copy link

coveralls commented Dec 28, 2019

Coverage Status

Coverage increased (+1.3%) to 95.385% when pulling 96f02dd on curbengh:trailing-html into ee2805b on hexojs:master.

@curbengh
Copy link
Contributor Author

This is necessary even with the following config,

server:
  serveStatic:
    extensions:
      - html

because currently hexo-server always add a trailing slash;

Current behavior:

source is source/bar.md

foo.com/bar -> foo.com/bar/ => foo.com/bar/index.html

Expected behavior:

foo.com/bar => foo.com/bar.html

res.end('Redirecting');
return;
if (route.get(url + '/index.html')) {
// When the URL is `foo/index.html` but users access `foo`, redirect to `foo/`.
Copy link
Member

Choose a reason for hiding this comment

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

What if foo/index.html & foo.html both existed?

Should we prefer foo.html before redirect to foo/index.html?

Copy link
Contributor Author

@curbengh curbengh Dec 28, 2019

Choose a reason for hiding this comment

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

If request is foo, foo/index.html takes priority.
I assume source/foo/index.md is more common (if hexo-server is used in production) as hexo-server currently doesn't support foo => source/foo.md.

If the priority is switched, it could be a breaking change.


just to clarify, foo/ is always proxied to foo/index.html.

Copy link
Member

@SukkaW SukkaW Dec 28, 2019

Choose a reason for hiding this comment

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

I have set up a simple example on Netlify: https://pretty-url.netlify.com/

Here are 4 links:

Copy link
Member

@SukkaW SukkaW Dec 28, 2019

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://pretty-url.netlify.com/foo.html => /foo.html
https://pretty-url.netlify.com/foo => /foo.html
https://pretty-url.netlify.com/foo/index.html => /foo/index.html
https://pretty-url.netlify.com/foo/ => 301 /foo

/foo/ redirects to /foo which is then proxied to /foo.html?

Anyhow, I consider foo/index.html + foo.html to be invalid, not an edge case. Besides, hexo new page creates source/foo/index.md by default. So, if foo.html is prioritized, route.get() is more likely to be called three times, versus two in foo/index.html.

Copy link
Member

Choose a reason for hiding this comment

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

@curbengh

/foo/ redirects to /foo which is then proxied to /foo.html?

So I prefer the GitHub Pages' behavior:

/foo/ => /foo/index.html
/foo => /foo.html

For hexo-server, when /foo is requested, we should find foo.html first, then find /foo/index.html

Copy link
Member

@SukkaW SukkaW Dec 28, 2019

Choose a reason for hiding this comment

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

if (route.get(url + '.html')) {
  // serve the page
} else if (route.get(url + '/index.html')) {
  // redirect to "url/"...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For hexo-server, when /foo is requested, we should find foo.html first, then find /foo/index.html

Even if it's the default behavior of gh-pages/apache/nginx, shouldn't we follow hexo's default behavior?

hexo new page creates source/foo/index.md by default.

I meant to convey that source/foo/index.md should be more common, even though source/foo.md is supported as well.

Copy link
Member

@SukkaW SukkaW Jan 7, 2020

Choose a reason for hiding this comment

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

@curbengh No matter how common source/foo/index.md is, the point is if source/foo/index.md and source/foo.md both exists, then source/foo.md will no longer be accessible through /foo, which is not consistent with hexo's current pretty_url as well (Since the pretty_url is designed to be consistent with the platform the Hexo will be deployed to).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed the order:

Location foo/bar.html & foo/bar/index.html
Request foo/bar
Respond foo/bar.html via transparent proxy

Location foo/bar.html & foo/bar/index.html
Request foo/bar/
Respond foo/bar/index.html via transparent proxy

@curbengh
Copy link
Contributor Author

After hexojs/hexo#4359, hexo-server also needs to support permalink: first-article in front-matter; that config will results in public/first-article file without file extension, hexo-server currently responds with content-type: application/octet-stream header, instead of content-type: text/html.

@curbengh
Copy link
Contributor Author

curbengh commented Jun 25, 2020

Shall we prioritize index.html or foo.html based on actual file location and then pretty_urls option?
Some cases require redirect for SEO.

File: /foo/bar/index.html
Request: /foo/bar/
Respond: No redirect, serves file directly

File: /foo/bar/index.html
Request: /foo/bar
Respond: Redirect to /foo/bar/ and serve using example 1.

File: /foo/bar/index.html
Request: /foo/bar/index.html
Respond: If pretty_urls.pretty_urls.trailing_index === false, redirect to /foo/bar/ and serve using example 1; otherwise, no redirect and serves /foo/bar/index.html.

File: /foo/bar.html
Request: /foo/bar
Respond: No redirect, serves file directly

File: /foo/bar.html
Request: /foo/bar/
Respond: Redirect to /foo/bar and serve using example 4.

File: /foo/bar.html
Request: /foo/bar.html
Respond: If pretty_urls.pretty_urls.trailing_html === false, redirect to /foo/bar and serve using example 4; otherwise, no redirect and serves /foo/bar.html.


permalink: first-article (no file extension) probably not viable for now, I think mime operates based on file extension.

@curbengh curbengh marked this pull request as draft June 27, 2020 10:25
@curbengh curbengh marked this pull request as ready for review June 28, 2020 06:00
@curbengh curbengh requested a review from SukkaW June 28, 2020 06:16
@curbengh curbengh merged commit be420d1 into hexojs:master Jul 1, 2020
@curbengh curbengh deleted the trailing-html branch July 1, 2020 10:58
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.

3 participants