-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
fix: missing themes for root #2335
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
0e561e7
to
78570bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually need it because it's referenced in the documentation. see https://docsify.js.org/#/quickstart?id=specifying-docsify-versions
Good catch, however I do not think we should restore the /themes
directory.
Having duplicate files in multiple locations in a single repo is a bad idea. Doing so invites confusion among those unfamiliar with the details of why duplicate files exist, which can lead to public-facing mistakes that require long-term support. This is an excellent example, and I believe it is a mistake we should correct for v5:
- The next release is going to be a breaking change because our codebase no longer supports legacy browsers. If we want to make a change, this is the time to do it.
- Our documentation should not recommend loading unminified CSS or JS files on a production site. Similarly, our CLI output should not load unminified CSS or JS by default. We can offer unminified CSS/JS as options, but our recommendation should be to use minified CSS/JS.
- All distributable files belong in the same directory. For v4, this is
/lib
. For v5, the proposed location is/dist
. - If we want to offer minified and unminified CSS files, we should differentiate using the filename and not the location, just as we do with
docsify.js
anddocsify.min.js
. For v5, this would mean something like/dist/themes/vue.css
and/dist/themes/vue.min.css
.
I planned to address this issue in an upcoming PR as part of our v4 => v5 transition:
- Modify build to output distributable files to
/dist
instead of/lib
. This is part of the v4 => v5 transition. - Update URLs as needed in
docsify
anddocsify-cli
repos to reference/dist
instead of/lib
. - Create static redirects/imports from
/lib
and/themes
to@4/lib
and@4/themes
CDN URLs. This allows v4 sites without a URL version lock to continue working when they unintentionally load v5 after its release. - Add a console deprecation / upgrade notice for sites loading resources from
/lib
or/themes
redirects/imports.
The static redirects/imports in /lib
and /themes
are a temporary solution. They exist only to prevent sites that load docsify resources without URL version locks from breaking when v5 releases because v4 was released in 2017 but our docs did not recommend URL version locks until mid-2020. My hope is that they will can remove them by the time v6 is released.
okay, I created a new issue. goto #2336 |
ref #2316, Remove duplicate
/themes
directory in root (no longer needed)We actually need it because it's referenced in the documentation. see https://docsify.js.org/#/quickstart?id=specifying-docsify-versions
And it's uncompressed css,
lib/themes
is compressed.I think it should be keep it.