tj / consolidate.js

Template engine consolidation library for node.js
3.48k stars 357 forks source link

Added Support for Twing #327

Closed noel-schenk closed 4 years ago

noel-schenk commented 4 years ago

Twing is a template engine which supports 100% of the syntax defined by Twig. https://github.com/ericmorand/twing

noel-schenk commented 4 years ago

Twing starts with node version 6.0.0 so it's excluded from the node tests under 6.0.0. If someone were to use a node version older than 6.0.0 it would throw an error message explaining that it has to be upgraded.

doowb commented 4 years ago

Thanks for the PR. I'll review closer this weekend, but for now, I don't think bringing in semver for this check is the right thing to do.

I'm pretty sure there are some other template engines that don't support older versions of Node.js and are handled in the tests (I'll look into this later). I think doing a simple check in the tests would be sufficient.

I'd like to get this library updated to drop older versions of Node.js anyway so it won't be an issue then.

ericmorand commented 4 years ago

I think dropping node 4 support should be safe. Don't know what the usage stats are exactly but I'm very enclined to believe that node 4 is barely used anymore.

DerekRoth commented 4 years ago

Any update on this ? Twing is now in the official list of ExpressJS "out of the box" supported template engines but i believe it depends on this PR being merged ? See https://expressjs.com/en/resources/template-engines.html

noel-schenk commented 4 years ago

@doowb is there any way we can circumvent the node 4 tests?

doowb commented 4 years ago

@noelelias would you move the semver dependency to devDependencies and only use it in the tests (just like you already did).

Since consolidate itself doesn't require in the engine until it's used this should be fine. We can see that the Node.js 4 tests pass, which I'm fine with.

noel-schenk commented 4 years ago

@doowb it should be ready to merge

doowb commented 4 years ago

Thanks!