Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
tools: build doc/api/all.html by combining generated HTML#21544
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Combine the toc and api contents from the generated doc/api/*.html files. This ensures that the single page version of the documentation exactly matches the individual pages. Fixesnodejs#20100
Trott commented Jun 26, 2018
/ping @nodejs/documentation especially @vsemozhetbyt |
Trott commented Jun 26, 2018
I added two commits, one to update package-lock.json (and add a missing dependency or two apparently) and another to update the LICENSE file. Those commits can be squashed out upon landing (or sooner). |
b95d56a to 800713bComparevsemozhetbyt commented Jun 26, 2018
I'll try to review this PR and #21490 this week, but I am not sure how thoroughly I will be able to do this: I have much less time recently and this is a whole new module set I am not experienced with. So if anybody knows this toolchain well, please, chime in) |
Trott commented Jun 26, 2018
The title of the <title>About this Documentation | Node.js v11.0.0 Documentation</title>On this branch: <title>Node.js v11.0.0 Documentation</title>For some reason, the links in the doc-version-picker are different ( |
also update Makefile to make apidocs depend on the tools
800713b to 5e9cf65Comparerubys commented Jun 27, 2018
A brief overview of JSDOM: it is a faithful implementation of the browser DOM. So if anybody here has experience writing JavaScript in the browser, this should look very familiar. I said the overview would brief. :-) As to doc-version-picker lnks: good catch! I've pushed a fix. Take a look at that fix to get an idea of what I mean by this code should look very familiar to anybody who has experience writing javascript in the browser. As to the title: that change is intentional, though perhaps it could be improved. Previously, all.html was produced by concatenating the markdown input and running it through the publishing pipeline. For the table of contents and api content, this produced mostly the desirable results (with a few bugs as noted in this issue). In the case of the title, instead of concatenating the titles of each of the 50 documents, it picked the title of the first document. Instead I went with the part of the title that is common to all documents. A good case could be made that this should be prefixed by something indicating 'single page'. Suggestions welcome. In any case, this would be a one line change - search for |
jasnell commented Jun 27, 2018
First off. Hi @rubys! Great to see you here! It's been quite a while, my friend! Second, great to see work here. I won't be able to do a review until later this week. |
rubys commented Jun 27, 2018
Hi @jasnell! It hasn't really been that long. Looking forward to working with you again. |
TimothyGu commented Jun 27, 2018
Hi, this PR is great. However, as one of the maintainers of jsdom I'm reluctant to recommend the inclusion of the entire package in Node.js tree, just because of its sheer size. I'd like to explore any possible alternatives that would allow us to accomplish the same goal in a more efficient way. Moreover, as I noted in #21490 (comment), it seems like we would be duplicating a lot of npm modules we already include in the code base, like acorn. I know this is already somewhat an issue with npm and ESLint in tree, but it would be awesome if we could dedupe a bit (especially with ESLint, which is also in the |
rubys commented Jun 27, 2018
@TimothyGu Perhaps you would prefer a version based on RegExps? Something like https://gist.github.com/rubys/26fb5b75d624098b021575c4b58874b0 perhaps? Much faster, zero dependencies, but perhaps a little less robust in that it depends on a number of HTML serialization choices; but overall that's probably acceptable for a tool that is only used at build time. |
TimothyGu commented Jun 27, 2018
@rubys Yeah, I think that’s a better choice. |
jasnell commented Jun 29, 2018
Zero dependencies is much better, even if a bit less robust. |
rubys commented Jun 29, 2018
Alternate version of this pull request, with zero dependencies can be found here: #21568 |
Combine the toc and api contents from the generated doc/api/*.html files. This ensures that the single page version of the documentation exactly matches the individual pages. Fixesnodejs#20100
rubys commented Jul 3, 2018
Closing as #21568 was merged. |
Combine the toc and api contents from the generated doc/api/*.html
files. This ensures that the single page version of the documentation
exactly matches the individual pages.
Fixes#20100
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes