-
Notifications
You must be signed in to change notification settings - Fork 11
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: break up api rendering #611
Conversation
Break up the rendering of a packages documentation into multiple pages. The root package page now renders the readme, and each type within the packages API reference is rendered on its own page. Items within the readme and types are still linkable via hash links that are scrolled to by the PackageDocs component. The rendering is broken up by splitting the incoming markdown on the `API Reference` header and then on subsequent headers within the api reference markdown string. These are then parsed into menu items and a hash map of `typeid -> type markdown content`. Adds unit tests for markdown parsing logic to ensure outputs remain stable. fix: #560
…ct-hub-webapp into feat/separate-type-pages
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.
Yeah!
Curious- did you have a chance to benchmark the load time for large modules against the previous version?
@MrArnoldPalmer It should be easy to adapt the performance test script to test and compare the performance before and after. LMK if you'd |
Only with local builds, which is not a super accurate number because of dev assets instead of our optimized product assets. Though the difference was clear and that should stand. I can run some comparisons in my account and report back. |
…eat/separate-type-pages
@eladb @addihorowitz here are numbers comparing the ec2 package rendering performance before and after this PR. Root documentation page rendering time with loading assets: These screenshots are the browser tools timeline showing the first frames that include rendered markdown content of the readme. Here are also the lighthouse performance reports that you can view in your browsers dev tools for more detailed information. You can see that though performance is obviously greatly improved, the performance score in fact goes DOWN. This is (as far as we know) because the old version of the site takes so long to render that lighthouse and pagespeedinsights actually assume that it is done much earlier than in reality, and show first contentful paint around 2 seconds, and time to interactive on desktop around 5, though we know this isn't true. |
Hell yeah! Looks like amazing improvement |
…eat/separate-type-pages
The type links construct-hub creates are broken because they still format links using the old `#` pattern. We dropped this in cdklabs/construct-hub-webapp#611 but didn't adjust the link formatter we pass to docgen to account for it. This video shows the experience with links now. Note that clicking on a type link now reloads the page, which is worse experience that we used to have, and what we currently have with readme links. https://user-images.githubusercontent.com/1428812/142417380-d4045665-c775-4ae6-a491-3bce2381743a.mov ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Break up the rendering of a packages documentation into multiple pages.
The root package page now renders the readme, and each type within the
packages API reference is rendered on its own page. Items within the
readme and types are still linkable via hash links that are scrolled to
by the PackageDocs component.
The rendering is broken up by splitting the incoming markdown on the
API Reference
header and then on subsequent headers within the apireference markdown string. These are then parsed into menu items and a
hash map of
typeid -> type markdown content
.Adds unit tests for markdown parsing logic to ensure outputs remain
stable.
Removes 1MB size limit for rendering markdown documents.
NOTE:
setext
style mdheadings are not currently supported and will be added in a follow up. See
#629
fix: #560