Closed howarddierking closed 5 years ago
Hi @howarddierking, thanks for bringing this to my attention. Actually, since jstransformer's renderFileAsync
falls back to the synchronous methods, I think it should be fine to start using that instead of the current renderFile
. We're already using promises here so it should be fine to start using async rendering.
In the callback passed to renderFileAsync we'd just need to update file.contents, call debug and resolve. Other than that adding some tests for this new behaviour should be all that's needed. Let me know what you think and feel free to submit a PR if this would work for you.
ah - I had actually missed the fallback detail in renderFileAsync
.
I have it working with a switch using the following -
if(settings.renderAsync){
return transform.renderFileAsync(layoutPath, settings.engineOptions, locals)
.then(rendered => {
file.contents = Buffer.from(rendered.body);
debug(`done rendering ${filename}`);
})
} else {
// Transform the contents
contents = transform.renderFile(layoutPath, settings.engineOptions, locals).body;
// Update file with results
// eslint-disable-next-line no-param-reassign
file.contents = Buffer.from(contents);
debug(`done rendering ${filename}`);
return Promise.resolve();
}
I'll pull out the switch to just use async, verify with the tests, and send you a PR.
I'm currently working to get my blog caught up with the changes in layouts. I'm using qejs as my template engine, so was happy to find that there was an existing jstransformer. Unfortunately, when running, I get the error
The Transform qejs does not support rendering synchronously.
I dug into the code a bit for metalsmith-layouts, jstransformer, and jstransformer-qejs, and the issue appears to be a combination of conflicting designs.
First, metalsmith-layouts expects to call the layout engine synchronously.
https://github.com/metalsmith/metalsmith-layouts/blob/1e633cb02f5b4feb04e8f6e79525950c472edaf8/lib/index.js#L60
Second, jstransformer provides different render methods for sync and async
https://github.com/jstransformers/jstransformer/blob/69fd94c47500fe4d4c8d9f1ac2918c2a55075db0/index.js#L318-L359
Finally, jtransformer-qejs only supports async rendering
https://github.com/jstransformers/jstransformer-qejs/blob/2ec3c78a302d3c361a45622ae49d0c0bab44754e/index.js#L10
Because it isn't really possible (nor desirable) to have metalsmith-layouts be aware of the jstransformer being used, it seems reasonable to add an
renderAsync:bool
option to the plugin and branch accordingly in metalsmith-layouts. I'm happy to put a PR together but first wanted to get some feedback on the issue and proposed solution.