-
-
Notifications
You must be signed in to change notification settings - Fork 154
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: Included docsify version in url #107
feat: Included docsify version in url #107
Conversation
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.
LGTM
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.
Using only the major version number is the correct implementation (as @aemmadi has done). This will load the latest v4 minor and point releases, but not the next major release (v5).
Two change requests:
-
Remove the file path and name from the URL:
<script src="//cdn.jsdelivr.net/npm/docsify@4"></script>
JSDelivr will automatically deliver the file specified in
package.json
using thejsdelivr
,browser
ormain
property and minify it if necessary. Docsify specifies"main": "lib/docsify.js"
in its package.json file, so specifying only//cdn.jsdelivr.net/npm/docsify@4
will result in the minified version oflib/docsify.js
being delivered. The reason to do this is because it gives maintainers the ability to change the file location of the main file without requiring an update to the CDN URL. If interested, you can read more about it here: https://www.jsdelivr.com/features#publishing-packages -
Add a docsify version to the stylesheet as well
docsify-cli/lib/template/index.html
Line 9 in 22314d3
<link rel="stylesheet" href="//cdn.jsdelivr.net/npm/docsify/lib/themes/vue.css"> Should be:
<link rel="stylesheet" href="//cdn.jsdelivr.net/npm/docsify@4/lib/themes/vue.css">
Note here that, unlike above, the full URL (path and filename) are required. This is because only one default file can be specified in
package.json
.
These same changes should be made in docsifyjs/docsify#1225.
Great, this is what I was looking for. 👍 for this |
@anikethsaha @jhildenbiddle Pushed the requested changes 😄 |
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.
Looks good!
@anikethsaha @trusktr -- You guys want to handle the merge? Not sure if one of you own merges and/or releases. Thx!
@jhildenbiddle done 👍 @aemmadi thanks for your contribution 👍 |
Fixes #58
init
now createsindex.html
with major version in URL.Updated docsify docs accordingly #1225