search

docsifyjs / docsify

🃏 A magical documentation site generator.
https://docsify.js.org
MIT License
27.47k stars 5.67k forks source link

action: refactor config vercel and pages publish. #2317

Closed Koooooo-7 closed 9 months ago

Koooooo-7 commented 10 months ago

Summary

Redirect output root path to docs/index.html since we remove the root index.html under #2316 . If it is acceptable, we could merge it into #2316 and move on.

Update: Custom github-pages publish flow to adapt index.html changes.

Related issue, if any:

What kind of change does this PR introduce?

Repo settings

For any code change,

Does this PR introduce a breaking change?

No

Tested in the following browsers:

vercel[bot] commented 10 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docsify-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 4, 2023 9:16am
Koooooo-7 commented 10 months ago

But it is not without preview. use cdn resources instead of lib.

Because we import the CND resources in index.html. I gonna update it.

Koooooo-7 commented 10 months ago

It works with local resources now.

jhildenbiddle commented 10 months ago

Thanks for trying to fix this, @Koooooo-7.

Unfortunately, the changes in this PR fix the preview, but they will break https://docsify.js.org/ once published they are published.

The repo previously had two index files: ./index.html and ./docs/index.html. The one in the root directory loaded docsify files from /lib/ while the one in the ./docs directory loaded files from a CDN. The Vercel preview loaded ./index.html (local URLs) while docsify.js.org loaded ./docs/index.html (CDN URLs). This worked, but there were a few issues:

  1. Having two index.html files was confusing.
  2. Having two index files meant both files needed to be maintained. This is tedious and error-prone.
  3. As a result of the two index files not been kept in sync, the docs site used during local development was not the same as our live docs site.

2316 removed the need for a second index.html in the root folder by having our local server (BrowserSync rewrite the CDN URLs to local URLs. You can see how this is done by viewing the new server configuration in that PR:

https://github.com/docsifyjs/docsify/blob/4756e59f8c71bcc9d2dd23d8581df0136cc83fd6/server.config.js#L10-L35

The end result is one index.html file that serves local files during local development and CDN files when served on docsify.js.org.

This PR simply hard-codes local URLs into the ./docs/index.html file which (as stated above) fixes the preview but will break the live site once published.

Hope this explanation helps. :smile:

Koooooo-7 commented 10 months ago

Hi @jhildenbiddle , I see. I think the problem is the default github pages flow won't pre-build project, hence there is no /lib resources folder. If we accept the redirect way to make the vercel works, I think I could re-write the gitpage publish with custom github action ( pre-build the /lib folder out ) to make the /doc/index.html works as well. Or instead, I rewrite the path to CDN path in actions. I could have a try on it.

jhildenbiddle commented 10 months ago

If we accept the redirect way to make the vercel works, I think I could re-write the gitpage publish with custom github action ( pre-build the /lib folder out ) to make the /doc/index.html works as well. Or instead, I rewrite the path to CDN path in actions.

If we do that, we're just introducing a different version of the previous index.html issue by having duplicate URL rewrite logic locally (BrowserSync) and in CI (GitHub actions).

A simpler way to accomplish the same thing:

  1. Copy the /lib/ folder to /docs/ during the build process
  2. Use local paths instead of CDN URLs for docsify files in /docs/index.html.
  3. Use the redirect as you've implemented in this PR.

I've explored a few options that solve the Vercel preview issue already. I think the above approach is best for a few reasons which I will outline in #2316 shortly. I will push these changes to that PR in the next few minutes.

Koooooo-7 commented 10 months ago

If we do that, we're just introducing a different version of the previous index.html issue by having duplicate URL rewrite logic locally (BrowserSync) and in CI (GitHub actions).

A little concern here, I prefer to rewrite the path to CND when we wanna make publish to Pages work.

We need the local resources import in index.html on PR/CI pipeline to make vercel works and get the latest changes in files.

But on Publish-Pages, I think we don't need the local resources since we should use the CDN/released resource instead of a local build, although it should be the same since we always sync the latest release to the latest main commit. (The rewrite is change the local import path to CDN @5 or @x, we need not change the pipeline very frequently either. )

Generally, we keep only one index.html with local resource import ( /docs/index.html ) , and remove the rewrite logic locally (BrowserSync). Then:

WDYT?

Koooooo-7 commented 9 months ago

Update: The pages branch gh-pages for publish (GithubPages branch). Github Action: deploy-gh Rewrite result: index.html (Copy /lib action is similar operations)

jhildenbiddle commented 9 months ago

@Koooooo-7 --

I've addressed the Vercel deployment issue in #2316.