Merge pull request #450 from marko-js/v3-render-to-string

add warnings for using render methods when you want string output
marko-js · Nov 30, 2016 · 591b912 · 591b912
2 parents 91c215c + 99e7873
commit 591b912
Show file tree

Hide file tree

Showing 3 changed files with 130 additions and 53 deletions.
diff --git a/docs/javascript-api.md b/docs/javascript-api.md
@@ -103,7 +103,7 @@ The `Template` type.
 
 ## Methods
 
-### renderSync(templateData) : String
+### renderToString(templateData) : String
 
 Synchronously renders a template to a `String`.
 
@@ -113,10 +113,25 @@ Example usage:
 
 ```javascript
 var template = require('./template.marko');
-var html = template.renderSync({ name: 'Frank' });
+var html = template.renderToString({ name: 'Frank' });
 console.log(html);
 ```
 
+### renderToString(templateData, callback)
+
+Asynchronously renders a template and provides the output to the provided callback function.
+
+```javascript
+var template = require('./template.marko');
+template.renderToString({ name: 'Frank' }, function(err, html, out) {
+	if (err) {
+		// Handle the error...
+	}
+
+	console.log(html);
+});
+```
+
 ### render(templateData, stream.Writable)
 
 Renders a template to a writable stream.
@@ -130,29 +145,15 @@ template.render({ name: 'Frank' }, process.stdout);
 
 ### render(templateData, AsyncWriter)
 
-Renders a template to an [AsyncWriter](https://github.com/marko-js/async-writer) instance that wraps an underlying stream.
+Renders a template to an [AsyncWriter](https://github.com/marko-js/async-writer) instance that wraps an underlying stream.  When rendering to an AsyncWriter, the writer will not be ended, automatically.  You must call `out.end()` yourself.
 
 Example usage:
 
 ```javascript
 var template = require('./template.marko');
 var out = require('marko').createWriter(process.stdout);
 template.render({ name: 'Frank' }, out);
-```
-
-### render(templateData, callback)
-
-Asynchronously renders a template and provides the output to the provided callback function.
-
-```javascript
-var template = require('./template.marko');
-template.render({ name: 'Frank' }, function(err, html, out) {
-	if (err) {
-		// Handel the error...
-	}
-
-	console.log(html);
-});
+out.end();
 ```
 
 _NOTE: The `out` argument will rarely be used, but it will be a reference to the [AsyncWriter](https://github.com/marko-js/async-writer) instance that was created to facilitate rendering of the template._
@@ -168,6 +169,12 @@ var template = require('./template.marko');
 template.stream({ name: 'Frank' }).pipe(process.stdout);
 ```
 
+### renderSync(templateData) : String
+> Deprecated in v3, use `renderToString(templateData)` instead.
+
+### render(templateData, callback)
+> Deprecated in v3, use `renderToString(templateData, callback)` instead.
+
 # require('marko/compiler')
 
 ## Methods

diff --git a/runtime/deprecate.js b/runtime/deprecate.js
@@ -1,23 +1,78 @@
 var logger = typeof console !== 'undefined' && console.warn && console;
+var messageCounts = {};
 
 module.exports = function(o, methodName, message) {
-    if (!logger) {
-        return;
-    }
+    if (!logger) return;
 
     var originalMethod = o[methodName];
 
-    var maxWarn = 20;
-    var currentWarn = 0;
-
     o[methodName] = function() {
-        if (currentWarn < maxWarn) {
-            if (++currentWarn === maxWarn) {
-                o[methodName] = originalMethod;
-            }
-            logger.warn(message, 'Stack: ' + new Error().stack);
+        var remainingWarns = warn(message);
+
+        if (!remainingWarns) {
+            o[methodName] = originalMethod;
         }
 
-        return originalMethod.apply(o, arguments);
+        return originalMethod.apply(this, arguments);
     };
-};
+};
+
+module.exports.warn = warn;
+
+function warn(message) {
+    if (!logger) return 0;
+
+    var maxWarn = 20;
+    var stack;
+    messageCounts[message] = messageCounts[message] || 0;
+
+    if (messageCounts[message] < maxWarn) {
+        messageCounts[message]++;
+        try {
+            stack = new Error().stack.split('\n').slice(4).join('\n');
+        } catch(e) {}
+        logger.warn(red('WARNING!!') + '\n' + message + '\n' + grey(stack || ''));
+    }
+
+    return maxWarn - messageCounts[message];
+}
+
+var canFormat = (function () {
+    try {
+        if (process.stdout && !process.stdout.isTTY) {
+            return false;
+        }
+
+        if (process.platform === 'win32') {
+            return true;
+        }
+
+        if ('COLORTERM' in process.env) {
+            return true;
+        }
+
+        if (process.env.TERM === 'dumb') {
+            return false;
+        }
+
+        if (/^screen|^xterm|^vt100|color|ansi|cygwin|linux/i.test(process.env.TERM)) {
+            return true;
+        }
+    } catch(e){}
+
+    return false;
+})();
+
+function format(str, begin, end) {
+    begin = canFormat ? '\u001b['+begin+'m' : '';
+    end = canFormat ? '\u001b['+end+'m' : '';
+    return begin + str + end;
+}
+
+function red(str) {
+    return format(str, 31, 39);
+}
+
+function grey(str) {
+    return format(str, 90, 39);
+}
diff --git a/runtime/marko-runtime.js b/runtime/marko-runtime.js
@@ -59,20 +59,6 @@ exports.AsyncStream = AsyncStream;
 
 var stream = require('./stream');
 
-function renderCallback(renderFunc, data, globalData, callback) {
-    var out = new AsyncStream();
-    if (globalData) {
-        extend(out.global, globalData);
-    }
-
-    renderFunc(data, out);
-    return out.end()
-        .on('finish', function() {
-            callback(null, out.getOutput(), out);
-        })
-        .once('error', callback);
-}
-
 function Template(path, func, options) {
     this.path = path;
     this._ = func;
@@ -92,19 +78,33 @@ Template.prototype = {
     c: function(createFunc) {
         this._ = createFunc(helpers);
     },
-    renderSync: function(data) {
+    renderToString: function(data, callback) {
         var localData = data || {};
         var out = new AsyncStream();
-        out.sync();
+
+        if(!callback) {
+            out.sync();
+        }
 
         if (localData.$global) {
-            out.global = extend(out.global, localData.$global);
+            extend(out.global, localData.$global);
             delete localData.$global;
         }
 
         this._(localData, out);
+
+        if(callback) {
+            out.end();
+            return out.on('finish', function() {
+                callback(null, out.getOutput(), out);
+            }).once('error', callback);
+        }
+
         return out.getOutput();
     },
+    renderSync: function(data) {
+        return this.renderToString(data);
+    },
 
     /**
      * Renders a template to either a stream (if the last
@@ -126,6 +126,14 @@ Template.prototype = {
      * @return {AsyncStream} Returns the AsyncStream instance that the template is rendered to
      */
     render: function(data, out, callback) {
+        if(typeof out === 'function') {
+            require('./deprecate').warn(
+                'The render callback will no longer receive a string in Marko v4.  '+
+                'Use `renderToString(data, callback)` instead.'
+            );
+            return this.renderToString(data, out /* callback */);
+        }
+
         var renderFunc = this._;
         var finalData;
         var globalData;
@@ -141,11 +149,6 @@ Template.prototype = {
             finalData = {};
         }
 
-        if (typeof out === 'function') {
-            // Short circuit for render(data, callback)
-            return renderCallback(renderFunc, finalData, globalData, out);
-        }
-
         // NOTE: We create new vars here to avoid a V8 de-optimization due
         //       to the following:
         //       Assignment to parameter in arguments object
@@ -154,6 +157,11 @@ Template.prototype = {
         var shouldEnd = false;
 
         if (arguments.length === 3) {
+            require('./deprecate').warn(
+                'Support for `render(data, out, callback)` will be removed in v4. ' +
+                'If you would like to discuss, see: https://github.com/marko-js/marko/issues/451'
+            );
+
             // render(data, out, callback)
             if (!finalOut || !finalOut.isAsyncStream) {
                 finalOut = new AsyncStream(globalData, finalOut);
@@ -202,6 +210,13 @@ Template.prototype = {
     }
 };
 
+require('./deprecate')(
+    Template.prototype,
+    'renderSync',
+    'Please use `renderToString` instead of `renderSync`.  '+
+    'The behavior of `renderSync` is changing in the next version of Marko.'
+);
+
 if (stream) {
     Readable = function(template, data, options) {
         Readable.$super.call(this);