-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
v5 preparation: updates and refactors #2428
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
- Change rollup build from API to config file - Change output dir from lib to dist - Update lib to dist path in related files - Update dependencies - Add banner comment to bundles - Add unminified plugin bundles
- Change CSS build from API to CLI - Change output dir from lib to dist - Update lib to dist path in related files - Update dependencies - Add sourcemaps
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
The docsify@version
import way should be the only recommend way since now and then.
TODO:
CLI
generated paths, resources.- Discussion of the
CLI
scope with build CSS and more.
Would love to remove Stylus and just use plain CSS, as it now has pretty much everything we used stylus for. Maybe it would have been a better spend of time if you just deleted Stylus build and converted to CSS? Stylus deletion is listed here: The more tools we can remove, the better. Most people will be fine without CSS minification, most Docsify sites are not million-user e-commerce sites. And if a team or company has that many users, then they have the skill and resources to minify source code. They can also get in touch with us and get on a sponsorship plan in exchange for optimization here: https://github.com/sponsors/docsifyjs (Btw we have a couple sponsors there we should give a shout out to!)
Is it necessary? I'd only add it if a problem is encountered (did that happen?), otherwise no need to unnecessarily block someone if they're on an older version of Node.
I think the only way is to continue to publish the old lib files in the new version for some time, but add a deprecation warnings to them (f.e. with console.warn, or maybe even going as far as injecting a small message at the bottom of the sidebar or something) telling people to stop using versionless URLs and that these files will be removed in N number of months (6 months?). Speaking of this, how can we enforce it for v5 to v6? And idea is while we're on v5, the dist folder could be |
Agreed. Happy to make this change either to this PR or as a separate PR. Since this PR has already been approved by @Koooooo-7, I'd prefer we get this one merged and handle removing Stylus in a separate PR.
Agreed on minimizing dependencies and removing unnecessary tools, but I don't we should do so at the expense of established best practices like optimizing the JS and CSS output we provide our users. It's easy to do and, more importantly, I believe it is what users familiar with these types of optimizations expect a project like Docsify to do.
Yes. The new rollup build uses I think requiring the latest LTS version of node is acceptable and non-controversial. I do not think the same is true for non-LTS versions (21) or "current" versions that have not yet entered LTS support (22). Node v20 is the latest LTS version and all of our tooling supports it (I've already made the change from node 18 to 20 in our Vercel preview settings). Node 22 is the "current" release and will enter long-term support (LTS) in October of this year, so I don't see requiring node 20 today as an issue. As for developers being blocked, tools like nvm make version switching version trivial and the
Yep. This is the option we discussed previously on Discord and the one I'm leaning towards. There are a few details to go over that are better handled in Discord v. PR comments. I mentioned the need to have that discussion because I didn't want maintainers to think this PR represented everything I think we need to do to release v5. It isn't. It is one of several PRs that I intend to file, but I'm trying to create them 1) in an order that makes sense and 2) with high-level consensus among the maintainers. |
# Conflicts: # package-lock.json # package.json
@trusktr -- One last item I forget to address:
CDNs handle this for us. jsDelivr provide access to all published versions via Other CDNs (like bootcdn.cn) do the same using the version as path of the path. For example: https://cdn.bootcdn.net/ajax/libs/docsify/4.13.1/docsify.js If the only outstanding issue is the removal of Stylus, let's approve this PR so we can move forward and I will remove Stylus in a separate PR (likely later tonight). Thanks! |
Good enough for me!
SGTM
Yeah, people should use versioned URLs. Totally. What I mean is that someone out there might (for whatever reason that might not be good) use an unversioned URL like So, the idea was that if we publish something like Basically this would force people to have a version in their URLs, at the cost of us having some more steps during major version bumps and having to eventually delete the old folder. Would this be worth it? Or should we not worry about helping people who may still use unversioned URLs, and we let their sites break without a warning? |
@trusktr -- Moving the versioned URL discussion to Discord. Sine you appear to be okay with the changes in this PR, please approve it so that we unblock other PRs that are waiting on this one to be merged. Thx! |
Summary
This PR includes refactor and clean-up work in preparation for the next major release (v5). Only changes to build and docs are included. No functional or test-related changes have been made aside from those required to facilitate the new build output directory (see below).
Update build output from
/lib
to/dist
in preparation for v5 releaseChanging the output directory ensures v4 users won't accidentally load v5 resources. This change is required due issues with CDN URL version locks in our documentation and templates (see next item).
Update docs to include
@5
version lock on all CDN URLsMany CDN URLs in our documentation and templates included
@latest
as the version lock or omitted a version lock altogether. CDN URLs without a proper version lock will redirect to the latest version, potentially causing previously working sites to load breaking changes when a new version of Docsify is released. Adding a@5
version lock to our CDN URLs will prevent future users from having to worry about this issue.Update JS, CSS, and test-related dependencies
These updates reduce our dependency security vulnerabilities from 15 to 0.
Update Emoji
New emoji are now available.
Refactor JS build from
/build
script to a rollup config fileRollup configuration files are easier to work. Making the switch reduced LOC for the JS build by ~50% and allowed for several dev dependencies to be removed. Each bundle now contains a "banner" comment to easily identify the version.
Refactor CSS builds from
/build
scripts to CLI toolsNo need for multiple build scripts. CLI tools are easier to work with and offer built-in watch functionality. Sourcemaps are now generated for both minified and un-minified versions, making it easy to see line numbers from the stylus source while using the web inspector for troubleshooting. Making the switch allowed for two build scripts and several dev dependencies to be removed.
Clean up Quick Start, Themes, and CDN pages
Ensure all CDN URLs reference minified files and deprioritize uncompressed versions. Briefly explain semantic versioning. Clarify benefits of locking to major or specific versions.
Clean up package.json
Reordered items for easier scanning. Added keywords and "engines" prop to specify node >= 20. Fixed incorrect "exports" path. Removed unnecessary comments.
Separate but related to this PR:
/lib
directory of the latest Docsify versions.docsify-cli
to align with these changes.These are separate discussions and will be handled in a separate PRs before the release of v5.
Related issue, if any:
#2336
What kind of change does this PR introduce?
Refactor
Docs
Build-related changes
For any code change,
Does this PR introduce a breaking change?
No
Tested in the following browsers: