Remove redundant file writing #1090
Conversation
Before #522, we had |
I see! Since we don't have them now, should be ok to remove the file writing? |
|
Yes. I wonder how much it improves build time. |
src/Page.js
Outdated
| @@ -797,8 +797,7 @@ class Page { | |||
| .then(result => this.insertFooterFile(result)) | |||
| .then(result => Page.insertTemporaryStyles(result)) | |||
| .then(result => markbinder.resolveBaseUrl(result, fileConfig)) | |||
| .then(result => fs.outputFileAsync(this.tempPath, result)) | |||
| .then(() => markbinder.renderFile(this.tempPath, fileConfig)) | |||
| .then(result => markbinder.renderFile(result, this.tempPath, fileConfig)) | |||
There was a problem hiding this comment.
Since we skip the intermediate output:
this.tempPathis no longer meaningful, and- this function should no longer be called
renderFile.
There was a problem hiding this comment.
Thanks! I've looked through the logic and removed Page#tempPath altogether.
Also, I've removed context as an argument for _parse in parser.js, since _parse just passes around context without doing anything to it.
I tested it on the and got 45 minutes on the original and 41 minutes on this branch. However, the variance was quite high when I ran the tests individually (all between 3:57-5:23 minutes), so don't think the build times improve by a lot. |
# Conflicts: # src/lib/markbind/src/parser.js
src/lib/markbind/src/parser.js
Outdated
| }); | ||
| const fileExt = utils.getExt(filePath); | ||
| if (utils.isMarkdownFileExt(fileExt)) { | ||
| const inputData = md.render(content); |
There was a problem hiding this comment.
One tiny nit:
inputData seems strange now that we are no longer reading from a file.
How about renderedContent?
* 'master' of https://github.com/MarkBind/markbind: Update test files (MarkBind#1138) Remove OK button from modals (MarkBind#1134) Add start from line number functionality to code blocks (MarkBind#1115) Allow special tags to be self-closing (MarkBind#1101) Simplify baseUrl resolving process (MarkBind#1087) Remove redundant file writing (MarkBind#1090) Bump acorn from 5.7.3 to 5.7.4 (MarkBind#1121) Bump acorn from 7.1.0 to 7.1.1 in /src/lib/markbind (MarkBind#1120) Unify markdown-it parser variants (MarkBind#1056) Remove dynamic include feature (MarkBind#1037) Fix flex shrink not applying in content wrapper (MarkBind#1135) Escape Nunjucks for special tags (MarkBind#1049) Update documentation on icon slot for boxes (MarkBind#1123)
…x-pageNav * 'master' of https://github.com/MarkBind/markbind: Update test files (MarkBind#1138) Remove OK button from modals (MarkBind#1134) Add start from line number functionality to code blocks (MarkBind#1115) Allow special tags to be self-closing (MarkBind#1101) Simplify baseUrl resolving process (MarkBind#1087) Remove redundant file writing (MarkBind#1090) Bump acorn from 5.7.3 to 5.7.4 (MarkBind#1121) Bump acorn from 7.1.0 to 7.1.1 in /src/lib/markbind (MarkBind#1120) Unify markdown-it parser variants (MarkBind#1056) Remove dynamic include feature (MarkBind#1037) Fix flex shrink not applying in content wrapper (MarkBind#1135) Escape Nunjucks for special tags (MarkBind#1049) Update documentation on icon slot for boxes (MarkBind#1123) Convert code in boxes to code block (MarkBind#1086)
We are writing to a temp path, and then reading it immediately. Let's avoid this by passing the string directly instead.
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [ ] Enhancement to an existing feature
What is the rationale for this request?
It seems like there is an uneeded file write at
markbind/src/Page.js
Lines 799 to 802 in f4036cc
We should be able to just pass the string directly to
renderFileWhat changes did you make? (Give an overview)
Pass the string directly to
renderFileProvide some example code that this change will affect:
none
Is there anything you'd like reviewers to focus on?
Maybe there was a reason for the file write?
Testing instructions:
Proposed commit message: (wrap lines at 72 characters)