Repository navigation
doc: move to redesign - #66713
doc: move to redesign#66713avivkeller wants to merge 3 commits into
Conversation
Signed-off-by: avivkeller <me@aviv.sh>
|
Review requested:
|
nodejs-github-bot
added
build
| const urlFromRoot = (path) => new URL(path, root).href; | ||
|
|
||
| const canBundle = async () => { | ||
| try { |
There was a problem hiding this comment.
We should remove this, I think we already have conditionals to make it work or not?
bmuenzenmeyer
left a comment
bmuenzenmeyer
left a comment
There was a problem hiding this comment.
I think we need to do better, align on fallback strategy here, or at least affirm that #66613 or this test is enough (not both) . If canBundle ever returns false, we don't get HTML docs at all, which we identified as a production issue worthy of an expensive new release.
Could we add this check to the release process too? Is CI enough? Perhaps
| const mdURL = new URL('../../doc/api/', import.meta.url); | ||
|
|
||
| // Kept in sync with `skip_apidoc_files` in the Makefile. | ||
| const skippedDocs = ['quic']; |
There was a problem hiding this comment.
https://github.com/nodejs/node/pull/66613/changes does a better job of keeping this in sync by reading the Makefile directly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66713 +/- ##
==========================================
+ Coverage 90.48% 90.50% +0.01%
==========================================
Files 791 797 +6
Lines 277204 277598 +394
Branches 53311 53334 +23
==========================================
+ Hits 250828 251228 +400
+ Misses 16796 16780 -16
- Partials 9580 9590 +10 🚀 New features to boost your workflow:
|
We know that lightningcss resolves on release machines, so I assume Rolldown will as well, but I 100% want to double-check that on a release-ci after regular CI passes (a regular CI is not enough). |
Co-authored-by: Brian Muenzenmeyer <brian.muenzenmeyer@gmail.com>
| const canBundle = async () => { | ||
| try { | ||
| require('lightningcss'); | ||
| await import('rolldown'); |
There was a problem hiding this comment.
Part of me feels this should come from the Makefile and not be in a config file 🤔
4 participants
This PR shouldn't change much from it's current form. Most changes will go in
doc-kit.test-make-docwas removed, since our new generator minifies the output HTML (so it's format is structurally inconsistent) and can't be easily parsed with the existing regular expressions.Furthermore, such tests are not needed due to our tests in doc-kit.
Preview: https://beta.docs.nodejs.org
Notable Change:
Node.js is thrilled to announce a brand new redesign of our documentation experience! The updated documentation now features HTML structure and styling that are much more closely aligned with the main Node.js website UI, creating a more consistent experience across the brand. You can check out the redesigned documentation today at
https://nodejs.org/api(Striked out since this isn't correct until this PR lands).