metalsmith / layouts

A metalsmith plugin for layouts
MIT License
116 stars 49 forks source link

wrap consolidate.js into a render function #100

Closed doodzik closed 7 years ago

doodzik commented 8 years ago

This should be done so we can exchange our render engine easier. Which in turn can let us ease the way to support multiple render engines.

for reference #96

the consolidate.requires can be handled in a custom render function. So there is no need to expose it anymore.

ismay commented 8 years ago

I like the idea of metalsmith-layouts being agnostic to the library that does the rendering, but after thinking about I don't think it's doable for this lib. It needs a consistent api, and so far it's already enough of a challenge doing that for consolidate, let alone multiple rendering engines.

If we're going to refactor, I think it would be good to focus on supporting a single engine, and not try to support everything. That'll probably make things to cluttered and prone to breakage. Also, after looking at jstransformer, apparently they have a transformer for consolidate :). So we could just support jstransformer, and offload render engine support to jstransformer. That way, we can always extend support by adding a jstransformer and our api remains the same.

I like the modular approach that jstransformers use. Should be very maintainable, and seems like the modern way to go. That being said, since https://github.com/RobLoach/metalsmith-jstransformer exists, I don't know how necessary it is that we switch to jstransformers as well.

RobLoach commented 8 years ago

As I mentioned in https://github.com/RobLoach/metalsmith-jstransformer/issues/9 , the easiest way to switch would be conslidate-jstransformer.

There are a few things metalsmith-jstransformer does on top of metalsmith-in-place/layouts. Worth a read over at https://github.com/RobLoach/metalsmith-jstransformer/issues/9 . Again, an easy way to try JSTransformers out would be replacing Consolidate.js with conslidate-jstransformer .

doodzik commented 8 years ago

I think it would be a good choice to support only one engine, too. We wouldn't achieve being agnostic to the library that does the rendering anyway. But I would be in favour of refactoring the render related logic into its own function. This function should be overwritable, so people can use their own logic.

ismay commented 8 years ago

Given the intended switch to jstransformers I think we can close this. What do you think @doodzik

This function should be overwritable, so people can use their own logic.

We could give this a try, see if the rendering can be isolated well enough for people to modify the logic without creating an entirely new plugin.

doodzik commented 8 years ago

I would still want to keep this one open, because that are two different issues.

... without creating an entirely new plugin.

I have a vague idea on how to accomplish that. If you don't mind I'll give it a try and we discuss it later.

ismay commented 8 years ago

Ok yeah, no problem! 👍