[docs] Parse markdown on mount #20601
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Details of bundle changes.Comparing: 310fcb1...4d63852 Details of page changes
|
bloats bundle of unrelated pages probably because prism has side-effects
req -> requireDemo reqSource -> requireRaw
it required svg sprites. We now unshift this in prepareMarkdown rather than relying on the callsite doing this
some pages still need this
- remove unused useMarkdownDocs - squash parseMarkdown and prepareMarkdown
eps1lon
force-pushed
the
docs/markdown-build-time
branch
from
c1ae528 to
d4cf31f
Compare
|
@eps1lon Thanks for the explanation on the difference between
Thanks for raising it. I will look at #20652 back once we get/if we can get |
Just a heads up: Summarizing the past months I probably spent days on similar issues. It's not as straight forward as you think it is because routing and rendering are so spread out. |
|
@eps1lon Is it ready to be merged? |
|
Might cause some bad merged for outdated PRs that change /docs/pages. CI will tell :) Thanks for the review! |
|
Sorry if you guys got notifications about those comments I made. I'm a total noob when it comes to pull requests on Github. Anyways, there's an issue with line 6 in |
|
@Andrew5569 Wow, how did you find that out? Confirmed at https://deploy-preview-20601--material-ui.netlify.app/api/button/. |
ghost
mentioned this pull request
1 task
|
I noticed it while working on that pull request ^ 😆 |
|
@Andrew5569 Ah ok, speaking on which, I was working on this very pull request (#20747), and noticed that we do no longer get a live update when editing the markdown in dev mode. |
oliviertassinari
added a commit
to oliviertassinari/material-ui
that referenced
this pull request
Apr 25, 2020
oliviertassinari
added a commit
to oliviertassinari/material-ui
that referenced
this pull request
Apr 25, 2020
oliviertassinari
added a commit
to oliviertassinari/material-ui
that referenced
this pull request
Apr 25, 2020
oliviertassinari
added a commit
to oliviertassinari/material-ui
that referenced
this pull request
Apr 25, 2020
This was referenced Apr 29, 2020
This was referenced May 6, 2020
oliviertassinari
added a commit
to oliviertassinari/material-ui
that referenced
this pull request
May 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes all markdown parsing during render. All of the use cases only passed code so we introduced a HighlightedCode component instead.
require.contextis a bit un-ergonomic for the single-markdown-file use case. They will get swapped out for vanilla fs.readFile once we move to getStaticProps./docs/pages is mostly noise. Reviewing one file is enough to understand the new template.
getStaticPropsdoes not work1 as long as we have getInitialProps on _app. So we still parse the markdown at runtime but at least don't parse it when re-rendering the page. This means that we AppTableOfContets is StrictMode compatible.Testing on /components/buttons/ first before I codemod the full docs.
1 I wouldn't be surprised if there's an issue how we build our docs. I honestly don't understand how exportPathMap, rewrites, getInitialProps, getStaticProps, custom _app and custom _document interact. I find the nextjs docs severly lacking. This will hopefully resolve itself once rewrites get stable and _app allows getStaticProps
internal changes
useMarkdownDocsis nowprepareMarkdowninparseMarkdown.js.prepareMarkdownnow handlestitleanddescriptionlogic. It will not throw if these values could not be determined. It's up to the caller to decide whether these should be set.MarkdownElementno longer parses markdown. Rather it accepts either raw html inrenderedMarkdownor a ReactNode inchildren.All use cases of
MarkdownElementin pages only needed code highlighting. Introduced<HighlightedCode code={string} language={string} />to support this use case.Results
We're trading more bytes sent for less CPU (render) time. This is better in the long run since we render far more often than we download assets. Asserts are cached. Calls to ReactDOM.render aren't.
For initial load you won't see significant results. Differences are too small. What will be faster is once you hit the cache.
/
https://www.webpagetest.org/video/compare.php?tests=200419_HS_5862aab14fdeaeb6f56e4653691ad5c6,200419_TY_e1621d7fa2db0a5da4db1719083d8101
Quite significant reduction in CPU usage (5.5s down from 6.2).
/components/alert/
https://www.webpagetest.org/video/compare.php?tests=200419_BG_2cd12b109ad9b38cf88a2fcd8edd6a63,200419_72_7314dd19046d6d1593a80f96917fd9da
Similar results as /