expose render function

[doodzik]

100 #102 for reference

This should make it easier to exchange consolidate for a different render engine and removes the need to expose the requires option explicitly.

The aligned equals signs are necessary for me because I don't use syntax highlighting. This helps me navigate in the code.

no tests were added because this was a refactoring. In the next step I would add some tests. Also I would suggest rewriting some test that aren't necessary because we can now mock consolidate.

thoughts?

[ismay]

Hey @doodzik, sorry about the late replies. After looking at this I personally think that this solution is too complicated for what it's trying to solve.

As I see it we have:

1. The consolidate.requires problem
2. Flexibility for end users (who by supplying a custom render function have more freedom in how to render their templates)

Regarding 1: the consolidate.requires problem is of course also something that this'll solve, but instead of solving it this way the switch to jstransformers would also take care of that problem.

Regarding 2: the level of complexity of providing your own render function is pretty high for the average user. Being able to supply your own render function is nice of course. But instead of a custom render function I think at that point people should create their own plugin.

That'll be easier to use for others and easier to maintain for us and them. That way people's custom rendering solutions will also be published and available for others as a plugin, instead of hidden away in their metalsmith setup. And also, jstransformers should already give people a lot of freedom in their choice of rendering engines.

Hope you don't mind my 2 cents on this. It's just that I'd like to avoid getting into a situation where we're complicating things for ourselves. Hope you understand and lemme know what you think!

[doodzik]

I would argue that this change isn't necessarily the best for the user, but the number of users who want their own render function should be fairly limited and the ones who need it will have a lot of flexibility.

This change is more for the codebase. If we provide the render capabilities as a dependency injection the code will get better to maintain in the long run. We can isolate the code better during tests, which leads to less and better tests. If we see this step as one of many we can bring down the complexity, which leads us to make changes to the code faster and better. Furthermore we shouldn't mirror the api of a third party library, which has different constrains.

[ismay]

Allright, that seems like a sensible reason for abstracting the rendering function. It's already largely abstracted in the new pr, so I'll see what I can do!

[doodzik]

I saw that and it should be good enough. As long as we can stub it.

[ismay]

👍