Repository navigation
doc: move to redesign - #66713
doc: move to redesign#66713avivkeller wants to merge 6 commits into
Conversation
|
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.65% 90.66% +0.01%
==========================================
Files 797 797
Lines 278268 278268
Branches 53511 53513 +2
==========================================
+ Hits 252258 252302 +44
+ Misses 16384 16344 -40
+ Partials 9626 9622 -4 🚀 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). |
| 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 🤔
There was a problem hiding this comment.
It's much easier to check JS with JS, no?
There was a problem hiding this comment.
It might aswell be, but this is supposed to be a simple config file. Moving it to the Makefile also allows us to use the same checks before it even reaches doc-kit and other collaborators to be aware said checks even exist instead of burying it down here.
IMO all other sort of checks of this nature already live there... Except in tests, tests have their own checks of sorts... IDK, wdyt @aduh95 do you have any opinions on this?
There was a problem hiding this comment.
TL;DR we should validate the environment can run doc-kit from within the Makefile.
@avivkeller also which environments wouldn't be able to run rolldown or lightningcss, are these missing Rust bindings or?
There was a problem hiding this comment.
We can absolutely reach out to VoidZero and ask them to port such bindings for us.
Reapplies the doc redesign and the follow-ups reverted in nodejs#67015, so the rest of this pull request applies on top of them: - 18a9ba5 build, doc: move to redesign - 37d5c80 build: toggle doc-kit verbosity based on V - df775ec, 28194a3, 2192f7f tools: bump the doc group The last doc group bump (1bc3cda) is left out, since the next commit bumps the same packages to the same versions. Refs: nodejs#66571 Assisted-by: a closed-source coding agent
Signed-off-by: avivkeller <me@aviv.sh>
Co-authored-by: Brian Muenzenmeyer <brian.muenzenmeyer@gmail.com>
The doc rule used a grouped target (`&:`) so that one doc-kit run produces every page. Grouped targets need GNU Make 4.3, and Make 4.2.1 (RHEL 8's) reads the rule as one rule per file instead. When an output is still missing after a run, e.g. the HTML on machines where it isn't built, it reran the whole doc build for each of them: 69 runs and ~25 minutes on the rhel8 CI agents. A stamp file has the same effect on every Make version. Refs: nodejs/doc-kit#1174 Assisted-by: a closed-source coding agent
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).