-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[docs] generate code paths for webpack raw-loader efficiency #27301
Conversation
0914350
to
f10acb4
Compare
Deploy preview for dagster-docs-beta ready! Preview available at https://dagster-docs-beta-p8n9pgb1m-elementl.vercel.app Direct link to changed pages: |
@@ -5,7 +5,7 @@ | |||
"scripts": { | |||
"docusaurus": "docusaurus", | |||
"start": "docusaurus start -p 3050", | |||
"build": "docusaurus build", | |||
"build": "yarn generate-code-imports && docusaurus build", |
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.
do we also need to generate code imports before running yarn start
?
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, I can add it there too! was a little worried about performance impact, but it would be a much better user experience.
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.
lgtm, could get better bundling by generating individual components for each example but I think this is a good trade off of complexity and making a big improvement. Also 2Mb for strings aint that much and reducing it to 1kb wont yield THAT much improvement.
Thanks for all the help Marco! 🙏 |
//if (isServer) { | ||
// const module = CODE_EXAMPLE_PATH_MAPPINGS[path]; | ||
// processModule({cacheKey, module, lineStart, lineEnd, startAfter, endBefore}); | ||
//} | ||
|
||
if (!contentCache[cacheKey]) { |
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.
This means that docusaurus supports Suspense on the server!
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.
(the fact that we didn't need the isServer logic).
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.
heck yeah! 🤯
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.
should've probably removed that comment, whoops. Can do that in the future.
Summary & Motivation
Edit looks like the regex is missing one of the entries as indicated by the failed Vercel build. Will look into this tomorrow.
raw-loader
was previously scanning the entireexamples/
folder, and leaving many artifacts. Resulting in excessive memory usage (see memory snapshot).This introduces a build step to create a mapping of
CodeExample
paths to their correspondingimport
, see improved memory snapshot.@salazarm we discussed this offline at some point, but would definitely appreciate your input on how this can be improved. Right off the bat, I'm not sure if the regex works for components that span multiple lines. Also want to get your input on if the server conditional logic is required. It seems to work with out it.
How I Tested These Changes
Changelog