tj / consolidate.js

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

Updates for Marko v4 #281

Closed hoichi closed 7 years ago

hoichi commented 7 years ago

Turns out there’s already PR for that: https://github.com/tj/consolidate.js/pull/268. This one should probably be closed.

  1. Updated Marko engine to v4.4.2.
  2. Changed tmpl.render() to tmpl.renderToString(), cause in Marko v4 .render() returns a stream, not a string.
  3. Fixed syntax in the test template.
  4. Fixed one test ('should not cache by default').

Now, I don’t like the test fix. The problem was that Marko itself caches rather aggressively. You can tell it not to write compiled templates to disk, but even before looking for those, it looks in require.cache, and that behavior is not configurable.

My fix was to change expected readFile()/readFileSync() calls to zero. That was the minimal code change I’ve come up with, but it’s not semantic, and I’m not sure 0 calls are actually always guaranteed. Maybe we should skip both cache-related tests for Marko instead. Maybe we could go even further and turn off Consolidate.js caching for Marko.

And yes, we could also make a request to make Marko built-in caching optional, but I’ve no idea if they’ll accept it.

doowb commented 7 years ago

Closing in favor of #268