下面的代码工作得很好,但是我想知道我是否不能用更好的方式来写我的承诺。
我把我的承诺称为SSG.start();
const yaml = require('js-yaml');
const fs = require('fs-extra');
const ejs = require('ejs');
const md = require('markdown-it')({
html: true,
linkify: true,
typographer: true
});
let SSG = {
outputFolder: '../output/',
contentFolder: '../content/',
assetsFolder: '../assets/',
contentFiles: async function (dir, filelist) {
return new Promise((resolve) => {
files = fs.readdirSync(dir);
filelist = filelist || [];
files.forEach((file) => {
if (fs.statSync(dir + '/' + file).isDirectory()) {
return resolve(contentFiles(dir + '/' + file, filelist));
} else {
filelist.push(file);
return resolve(filelist);
}
});
})
},
generateHTMLFile: async function (file) {
return new Promise((resolve) => {
let fstream = fs.readFileSync(file, 'utf8');
let params = fstream.match(/\+Params\+([\s\S]*?)\+\+\+/g);
let content = fstream.match(/(\+Content[0-9]*\+|\+Content\+)([\s\S]*?)*(\+\+\+)/g);
let contents = [];
params = params[0].replace('+Params+', '').replace('+++', '');
content = content[0].split('+++');
try {
params = yaml.safeLoad(params, 'utf8');
for (it of content) {
it = it.replace('+++', '').replace(/(\+Content[0-9]*\+|\+Content\+)/g, '');
contents.push(md.render(it));
}
const funs = require('./functions.js');
ejs.renderFile('../templates/' + params.template + '.ejs', {funs: funs, params: params, content: contents}, {}, (err, html) => {
if (err) console.log(err);
resolve({html: html, url: params.url});
});
} catch (e) {
console.log(e);
}
});
},
saveHTML: function (html, url) {
if (!fs.existsSync(this.outputFolder)) {
fs.mkdir(this.outputFolder, { recursive: true }, (err) => {
if (err) throw err;
});
}
fs.writeFile(this.outputFolder + url + '.html', html, (err) => {
if (err) return console.log(err);
});
},
saveStaticAssets: function () {
fs.copySync(this.assetsFolder, this.outputFolder);
},
start: async function () {
try {
let time = Date.now();
console.log('Génération des fichiers HTML...');
let files = await this.contentFiles(this.contentFolder);
for (file of files) {
await this.generateHTMLFile(this.contentFolder + file).then(({html, url}) => {
this.saveHTML(html, url);
});
}
this.saveStaticAssets();
console.log('Généré en ' + (Date.now() - time) + 'ms');
} catch (e) {
console.error(e.message);
}
}
};
(async () => {
await SSG.start();
})();你对此有何看法?
发布于 2019-05-27 10:09:13
让我烦恼的第一件事是为什么是let SSG?为什么它是一个物体?如果您正在使用ES6+ (就像您所做的那样),我将把它变成一个类。它更明显,更易读懂。注意,在执行此操作时,您必须更改函数签名,即generateHtmlFile: async function (file) { ... }变为async generateHtmlFile(file) { ... }。
由于您的文件夹名存储为对象的属性(outputFolder、contentFolder和assetsFolder),所以现在应该将它们移到构造函数中。第51行的另一个新属性也有一个很好的候选:也应该提取../templates/。这是因为JS类ATM中没有属性。此外,在讨论类属性的主题时,我建议您传递一个带有文件夹名称的对象作为构造函数参数。这样,您就可以通过使用其他文件夹名实例化一个对象来轻松地更改所使用的文件夹。当我们总结有关属性的信息时,应该是这样的:
class SGG {
constructor(folders = {}) {
this.outputFolder = folders.outputFolder || '../output/'
this.contentFolder = folders.contentFolder || '../content/'
this.assetsFolder = folders.assetsFolder || '../assets/'
this.templates = folders.templates || '../templates/'
}
}当我看这里的代码时,我没有注意到的第二件事是,您有一些隐式变量:第19行是files,第45行是it。所以,只要在这两个前面加上const,你就会没事的。
在第49行,有一个require。我会将它移到文件的顶部,因为这样您就可以正确地了解文件的依赖项。而且它的代码更干净,没有意外的变化。
现在,让我们谈谈方法。首先是contentFiles,它只是一个递归调用,用于查找contentFolder目录中的所有文件。我看到您可能对承诺感到困惑,因为您在同一个函数中使用了async和new Promise。当使函数成为async时,您应该知道,无论从函数返回什么,它都将封装在承诺中,并且它还使您能够在其中使用await。但是,由于这里没有await,所以不需要将这个函数变成async函数。你已经在兑现承诺了。但是,我认为你根本不需要承诺!您使用的这些fs-extra方法既不返回承诺,也没有回调,因此它们是完全同步的函数。而且,.forEach把所有这些都弄得乱七八糟,所以如果您删除了它并使用了for..of,您就可以自己拆解,得到一个更干净的代码。
下一个方法,generateHTMLFile与上面的承诺有类似的问题。从ejs的github中,我注意到如果您不传递回调函数,它将返回一个承诺。因此,这里应该做的是删除new Promise...,保留async,重写ejs.renderFiles以避免使用回调函数。
saveHTML方法有点混乱:您在任何地方都使用sync方法,但是这里您决定使用mkdir而不是mkdirSync或writeFileSync。
一旦你应用了我所说的所有内容,你就应该得到这样的东西:
const yaml = require('js-yaml');
const fs = require('fs-extra');
const ejs = require('ejs');
const md = require('markdown-it')({
html: true,
linkify: true,
typographer: true,
});
const funs = require('./functions.js');
class SSG {
constructor(folders = {}) {
this.outputFolder = folders.output || '../output/'
this.contentFolder = folders.contentFolder || '../content/'
this.assetsFolder = folders.assetsFolder || '../assets/'
this.templateFolder = folders.templateFolder || '../templates/'
}
contentFiles(dir, filelist) {
const files = fs.readdirSync(dir)
filelist = filelist || []
for (const file of files) {
if (fs.statSync(dir + '/' + file).isDirectory()) {
contentFiles(dir + '/' + file, filelist)
} else {
filelist.push(file)
}
}
return filelist
}
async generateHTMLFile(file) {
const fstream = fs.readFileSync(file, 'utf8');
let params = fstream.match(/\+Params\+([\s\S]*?)\+\+\+/g);
let content = fstream.match(/(\+Content[0-9]*\+|\+Content\+)([\s\S]*?)*(\+\+\+)/g);
let contents = [];
params = params[0].replace('+Params+', '').replace('+++', '');
content = content[0].split('+++');
try {
params = yaml.safeLoad(params, 'utf8');
for (let it of content) {
it = it.replace('+++', '').replace(/(\+Content[0-9]*\+|\+Content\+)/g, '');
contents.push(md.render(it));
}
const html = await ejs.renderFile(this.templateFolder + params.template + '.ejs', {
funs: funs,
params: params,
content: contents,
}, {})
return {
html,
url: params.url,
}
} catch (e) {
console.log(e);
}
}
async saveHTML(html, url) {
if (!fs.existsSync(this.outputFolder)) {
await fs.mkdirSync(this.outputFolder, {recursive: true});
}
await fs.writeFileSync(this.outputFolder + url + '.html', html);
}
saveStaticAssets() {
fs.copySync(this.assetsFolder, this.outputFolder);
}
async start() {
try {
let time = Date.now();
console.log('Génération des fichiers HTML...');
const files = this.contentFiles(this.contentFolder);
for (const file of files) {
await this.generateHTMLFile(this.contentFolder + file).then(({html, url}) => {
this.saveHTML(html, url);
});
}
this.saveStaticAssets();
console.log('Généré en ' + (Date.now() - time) + 'ms');
} catch (e) {
console.error(e.message);
}
}
};
(async () => {
const ssg = new SSG()
await ssg.start();
})();我要指出的最后一点是,Node已经有了相当不错的文件系统实用程序。我注意到的唯一与fs-extra不同的地方是,sync函数不返回承诺,而是充当常规函数。无论如何,我认为您不需要使用fs-extra,而是使用fs。但这取决于你。
我可能遗漏了一些东西,也可能深入到细节中,挑剔得超出了需要,但这是我对您的代码所做的基本重构。我希望我在做这件事的时候没有破坏一些东西:)
编辑:当我第二次查看代码时,我注意到我错过了start中的这一行:
await this.generateHTMLFile(this.contentFolder + file).then(({html, url}) => {
this.saveHTML(html, url);
});`也许只有我一个人,但我不喜欢await和then的混合物。我只是建议,如果你要和await一起去,就这样吧。我在这里的建议是这样的:
const {html, url} = await this.generateHTMLFile(this.contentFolder + file)
await this.saveHTML(html, url)我还注意到,您可以通过等待多个承诺来优化解决方案的速度:
await Promise.all(files.map(async (file) => {
const {html, url} = await this.generateHTMLFile(this.contentFolder + file)
await this.saveHTML(html, url)
}))通过这种方式,您可以并行地启动多个承诺(同样,由于JS是单线程的)而不是真正的并行化,并等待它们全部完成才能继续前进。这样你就能得到更快的执行。请注意,如果一个承诺抛出并出错,这一行也会在等待另一个承诺结束之前抛出一个错误。
https://codereview.stackexchange.com/questions/221087
复制相似问题