fix: Cannot serve off `/.../index.html` #1372

rgladwell · 2020-09-12T12:18:21Z

Summary

fixes #427

Docsify must be hosted on a server that supports a default directory index (i.e. maps /.../ -> /.../index.html).

Some platforms do not support this, however. For example, HTML apps hosted on the popular game/software platform, Itch.io.

This change supports hosting Docsify off an explicit path file, such as /index.html. It does this by:

Preventing the /index.html part of the path being removed during routing, and
Ensuring we can retrieve the source markdown using explicit path file URIs.

For example, http://example.org/index.html#/blah would be mapped to the markdown source file http://example.org/blah.md.

This fixes:

#427

What kind of change does this PR introduce? (check at least one)

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

Yes
No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

If adding a new feature, the PR's description includes:

A convincing reason for adding this feature
Related documents have been updated
Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

vercel · 2020-09-12T12:18:28Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/1nszbjtkr
✅ Preview: https://docsify-previe-git-fork-rgladwell-fix-base-path-file-not-f15f74.docsify-core.vercel.app

codesandbox-ci · 2020-09-12T12:20:07Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d94d259:

Sandbox	Source
docsify-template	Configuration

rgladwell · 2020-09-12T15:09:45Z

The e2e tests are reported by CI as failing, but are passing locally for me.

anikethsaha

Thanks a lot for fixing the bug.

It does seems to work and fixes the issue.

Few questions

It would be great to have some more test cases.
it would be great to have some comments for why we are doing those normalizing and those conditional check. May be linking to the issues/PR will work as well.

Also, I doubt this might be a semver-major as it can be a breaking change. cc @docsifyjs/reviewers

rgladwell · 2020-09-14T17:33:55Z

@anikethsaha Thanks for the feedback. Could you clarify what sort of test cases you are looking for? I notice you have Cypress and unit tests. Which would you prefer, and where would they go exactly?

sy-records · 2020-09-15T00:53:33Z

It looks good.

https://docsify-previe-git-fork-rgladwell-fix-base-path-file-not-70a56c.docsify-core.vercel.app/index.html#/quickstart

https://docsify.js.org/#/quickstart

anikethsaha · 2020-09-15T14:08:49Z

@anikethsaha Thanks for the feedback. Could you clarify what sort of test cases you are looking for? I notice you have Cypress and unit tests. Which would you prefer, and where would they go exactly?

You could add cypress tests for other links like

/index.md
/index.html#/
/#/index.html/
/index.html/#/ (it would be 404 but still good to cover. )

Also if there are other cases you think you can add

rgladwell · 2020-09-15T18:30:37Z

@anikethsaha You could add cypress tests for other links like

Done, let me know if they're sufficient.

anikethsaha

LGTM 🎉

My only concern is whether this should be considered as breaking or not!

rgladwell · 2020-09-16T07:46:07Z

It shouldn't be breaking, it's an additive change. You could argue that any breakages would be bugs.

Koooooo-7

@anikethsaha I think when it is not sure if it is a breaking change, we can make it as an option.
Giving the switch button to users' hands.:)

rgladwell · 2020-09-17T16:37:04Z

Hmmm I think this change breaks live reloads in development mode.

anikethsaha · 2020-09-18T05:30:25Z

I agree that it is not a breaking change as it was a bug so probably it wasn't used before in order to avoid the bug.

rgladwell · 2020-10-02T11:58:58Z

Been trying to add a feature flag for this, but the code is getting very complicated. If this isn't a breaking change, can we live without it?

anikethsaha · 2020-10-03T06:34:34Z

cc @docsifyjs/core

jhildenbiddle · 2020-10-08T00:55:28Z

Hey @rgladwell,

Thanks for the PR!

We recently merged #1276 into develop which is an overhaul of our CI and local test environments. As a result, the e2e tests in this PR will need to rewritten in Jest+Playwright instead of Cypress. The good news is that the test environments should be more stable and easier to work with. The PR contains a summary of the changes, help is available in /test/README.md directory, and I'm available if you have any questions.

rgladwell · 2020-10-16T11:28:38Z

Is there some way to get a screenshot or run non-headless for the Jest e2e tests?

Docsify must be hosted on a server that supports a default directory index (i.e. maps `/.../` -> `/.../index.html`). Some platforms do not support this, however. For example, HTML apps hosted on the popular game/software platform, Itch.io. This change supports hosting Docsify off an explicit path file, such as `/index.html`. It does this by: 1. Adding handling for paths like `index.html#/blah`, and 2. Normalising paths with fragments back to markdown paths For example, `http://example.org/index.html#/blah` would be mapped to `http://example.org/blah.md`. This fixes: docsifyjs#427

rgladwell · 2021-02-05T12:03:25Z

@jhildenbiddle @anikethsaha @Koooooo-7 As requested, added e2e tests using Jest framework.

src/core/router/history/hash.js

sy-records

LGTM.

note: Please use squash and merge for merging

rgladwell · 2021-02-07T16:26:28Z

Thanks!

docsifyjs/docsify#1372

vercel bot deployed to Preview September 12, 2020 12:18 View deployment

vercel bot deployed to Preview September 12, 2020 13:02 View deployment

vercel bot deployed to Preview September 13, 2020 04:25 View deployment

anikethsaha requested changes Sep 13, 2020

View reviewed changes

vercel bot deployed to Preview September 14, 2020 18:06 View deployment

vercel bot deployed to Preview September 15, 2020 18:22 View deployment

anikethsaha reviewed Sep 16, 2020

View reviewed changes

vercel bot deployed to Preview September 16, 2020 09:27 View deployment

Koooooo-7 reviewed Sep 17, 2020

View reviewed changes

anikethsaha previously approved these changes Sep 18, 2020

View reviewed changes

rgladwell dismissed anikethsaha’s stale review via d7cae0e October 8, 2020 09:29

rgladwell force-pushed the fix-base-path-file-not-folder branch from c375219 to d7cae0e Compare October 8, 2020 09:29

vercel bot deployed to Preview October 8, 2020 09:30 View deployment

rgladwell added 3 commits October 16, 2020 12:29

Add end-to-end test for index file hosting

36705c7

Add code comments for explicit file changes

e42c3df

vercel bot deployed to Preview February 5, 2021 11:56 View deployment

rgladwell requested review from Koooooo-7 and anikethsaha February 5, 2021 12:03

jhildenbiddle requested a review from a team February 5, 2021 16:20

jhildenbiddle requested changes Feb 5, 2021

View reviewed changes

src/core/router/history/hash.js Outdated Show resolved Hide resolved

Fix: endsWith() not supported for IE11

ac5ed77

vercel bot deployed to Preview February 6, 2021 13:12 View deployment

Refactor: utility method moved to utility file

831527f

vercel bot deployed to Preview February 6, 2021 13:41 View deployment

Fix IE11 error from use of String.includes()

d94d259

vercel bot deployed to Preview February 7, 2021 07:02 View deployment

jhildenbiddle self-requested a review February 7, 2021 07:03

sy-records mentioned this pull request Feb 7, 2021

[Release]: v4.12.0 #1493

Merged

3 tasks

sy-records approved these changes Feb 7, 2021

View reviewed changes

sy-records changed the title ~~Fix #427: Cannot serve off /.../index.html~~ fix: Cannot serve off /.../index.html Feb 7, 2021

Koooooo-7 approved these changes Feb 7, 2021

View reviewed changes

Koooooo-7 requested a review from a team February 7, 2021 07:35

jhildenbiddle approved these changes Feb 7, 2021

View reviewed changes

jhildenbiddle merged commit 759ffac into docsifyjs:develop Feb 7, 2021

rgladwell deleted the fix-base-path-file-not-folder branch February 7, 2021 16:26

rg-wood added a commit to grislyeye/three-meet that referenced this pull request Feb 8, 2021

[wip] Add deployment to Itch.io (requires PR)

a1ba867

docsifyjs/docsify#1372

rg-wood added a commit to grislyeye/three-meet that referenced this pull request Feb 8, 2021

[fixup] Hard link to wip PR commit

aa3c907

docsifyjs/docsify#1372

anikethsaha mentioned this pull request Feb 19, 2021

[Snyk] Security upgrade docsify from 4.11.6 to 4.12.0 #1518

Merged

dependabot bot mentioned this pull request Mar 17, 2021

Bump docsify from 4.9.1 to 4.12.1 Buzut/jamments#17

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Cannot serve off `/.../index.html` #1372

fix: Cannot serve off `/.../index.html` #1372

rgladwell commented Sep 12, 2020 •

edited

Loading

vercel bot commented Sep 12, 2020 •

edited

Loading

codesandbox-ci bot commented Sep 12, 2020 •

edited

Loading

rgladwell commented Sep 12, 2020

anikethsaha left a comment •

edited

Loading

rgladwell commented Sep 14, 2020

sy-records commented Sep 15, 2020

anikethsaha commented Sep 15, 2020 •

edited

Loading

rgladwell commented Sep 15, 2020

anikethsaha left a comment

rgladwell commented Sep 16, 2020

Koooooo-7 left a comment

rgladwell commented Sep 17, 2020 •

edited

Loading

anikethsaha commented Sep 18, 2020

rgladwell commented Oct 2, 2020

anikethsaha commented Oct 3, 2020

jhildenbiddle commented Oct 8, 2020

rgladwell commented Oct 16, 2020

rgladwell commented Feb 5, 2021

sy-records left a comment •

edited

Loading

rgladwell commented Feb 7, 2021

fix: Cannot serve off /.../index.html #1372

fix: Cannot serve off /.../index.html #1372

Conversation

rgladwell commented Sep 12, 2020 • edited Loading

vercel bot commented Sep 12, 2020 • edited Loading

codesandbox-ci bot commented Sep 12, 2020 • edited Loading

rgladwell commented Sep 12, 2020

anikethsaha left a comment • edited Loading

Choose a reason for hiding this comment

rgladwell commented Sep 14, 2020

sy-records commented Sep 15, 2020

anikethsaha commented Sep 15, 2020 • edited Loading

rgladwell commented Sep 15, 2020

anikethsaha left a comment

Choose a reason for hiding this comment

rgladwell commented Sep 16, 2020

Koooooo-7 left a comment

Choose a reason for hiding this comment

rgladwell commented Sep 17, 2020 • edited Loading

anikethsaha commented Sep 18, 2020

rgladwell commented Oct 2, 2020

anikethsaha commented Oct 3, 2020

jhildenbiddle commented Oct 8, 2020

rgladwell commented Oct 16, 2020

rgladwell commented Feb 5, 2021

sy-records left a comment • edited Loading

Choose a reason for hiding this comment

rgladwell commented Feb 7, 2021

fix: Cannot serve off `/.../index.html` #1372

fix: Cannot serve off `/.../index.html` #1372

rgladwell commented Sep 12, 2020 •

edited

Loading

vercel bot commented Sep 12, 2020 •

edited

Loading

codesandbox-ci bot commented Sep 12, 2020 •

edited

Loading

anikethsaha left a comment •

edited

Loading

anikethsaha commented Sep 15, 2020 •

edited

Loading

rgladwell commented Sep 17, 2020 •

edited

Loading

sy-records left a comment •

edited

Loading