Skip to content

Commit

Permalink
IIFE breaks ability to specify execution context
Browse files Browse the repository at this point in the history
  • Loading branch information
mde committed Jun 29, 2015
1 parent 33f0d7a commit 8cdbee3
Showing 1 changed file with 7 additions and 13 deletions.
20 changes: 7 additions & 13 deletions lib/ejs.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,8 @@ Template.prototype = {
var src
, fn
, opts = this.opts
, prepended = ''
, appended = ''
, escape = opts.escapeFunction;

if (opts.rmWhitespace) {
Expand All @@ -413,24 +415,16 @@ Template.prototype = {
this.templateText =
this.templateText.replace(/\r/g, '').replace(/^\s+|\s+$/gm, '');
}

if (!this.source) {
this.generateSource();

var prepended = ''
, appended = '';

prepended += ' var __output = [], __append = __output.push.bind(__output);' + '\n';
if (opts._with !== false) {
prepended += ' with (' + exports.localsName + ' || {}) {' + '\n';
prepended += ' return (function(){' + '\n';
appended += ' }());' + '\n'; // closes IIFE
appended += ' }' + '\n'; // closes `with`
prepended += ' with (' + exports.localsName + ' || {}) {' + '\n';
appended += ' }' + '\n';
}

prepended += ' var __output = [], __append = __output.push.bind(__output);' + '\n';
appended = ' return __output.join("");' + '\n' + appended;

appended += ' return __output.join("");' + '\n';
this.source = prepended + this.source + appended;

}

if (opts.compileDebug) {
Expand Down

3 comments on commit 8cdbee3

@mde
Copy link
Owner Author

@mde mde commented on 8cdbee3 Jun 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dominykas, see this and the following commit -- opts.context was totally broken, and fixing it required removing the IIFE. Happy to work with you to come up with a better way to ensure good minification.

@dominykas
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But there were no tests failing :(

The IIFE itself can be bound like (function () {}.bind(something))() :D I'll have a look after I'm back from holidays.

@mde
Copy link
Owner Author

@mde mde commented on 8cdbee3 Jun 30, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was documented functionality, but there were no tests for it. I know that sucks. Good call on binding the IIFE. Shoot me a PR, if I don't get to it first.

Please sign in to comment.