Skip to content

Commit 812c226

Browse files
jaylinskijaylinski
committed
Iterate lazily in #each and strip whitespace in linear time
`#each` collected every value of an iterable before rendering the block. It now renders each value as it goes, like `for...of`, and closes the iterator if the block throws, so generators can run their cleanup code. Values added to the iterable while the block renders, e.g. to a `Set`, are now visited too, as with `for...of`. The regexes that find trailing whitespace in whitespace control were retried at every position of a whitespace run, which takes quadratic time on long runs. They are now anchored so that matching takes linear time.
1 parent 609d1b1 commit 812c226

4 files changed

Lines changed: 192 additions & 21 deletions

File tree

‎lib/handlebars/compiler/whitespace-control.js‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ WhitespaceControl.prototype.Program = function(program) {
4141
// If we are on a standalone node, save the indent info for partials
4242
if (current.type === 'PartialStatement') {
4343
// Pull out the whitespace from the final line
44-
current.indent = /([ \t]+$)/.exec(body[i - 1].original)[1];
44+
current.indent = /(?:^|[^ \t])([ \t]+)$/.exec(
45+
body[i - 1].original
46+
)[1];
4547
}
4648
}
4749
}
@@ -159,9 +161,12 @@ function isPrevWhitespace(body, i, isRoot) {
159161
}
160162

161163
if (prev.type === 'ContentStatement') {
162-
return (sibling || !isRoot ? /\r?\n\s*?$/ : /(^|\r?\n)\s*?$/).test(
163-
prev.original
164-
);
164+
// Matches when the trailing whitespace contains a newline (or, for the
165+
// root's first node, is the whole string). See omitLeft for the anchoring.
166+
return (sibling || !isRoot
167+
? /(?:^|\S)[^\S\n]*\n\s*$/
168+
: /^\s*$|(?:^|\S)[^\S\n]*\n\s*$/
169+
).test(prev.original);
165170
}
166171
}
167172
function isNextWhitespace(body, i, isRoot) {
@@ -226,7 +231,13 @@ function omitLeft(body, i, multiple) {
226231

227232
// We omit the last node if it's whitespace only and not preceded by a non-content node.
228233
let original = current.value;
229-
current.value = current.value.replace(multiple ? /\s+$/ : /[ \t]+$/, '');
234+
// The match must start after a non-whitespace character (or at the start):
235+
// a plain /\s+$/ is retried at every position of a whitespace run, which
236+
// takes quadratic time on long runs.
237+
current.value = current.value.replace(
238+
multiple ? /(^|\S)\s+$/ : /(^|[^ \t])[ \t]+$/,
239+
'$1'
240+
);
230241
current.leftStripped = current.value !== original;
231242
return current.leftStripped;
232243
}

‎lib/handlebars/helpers/each.js‎

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,26 @@ import {
77
} from '../utils';
88
import Exception from '../exception';
99

10+
/**
11+
* Close an unfinished iterator after the block body threw, like for...of does,
12+
* so that generators can run their cleanup code.
13+
*
14+
* Errors thrown while closing, including by a `return` getter, are ignored:
15+
* the block's error is the cause and is the one the caller must see.
16+
*
17+
* @param {Iterator} iterator the iterator that was being consumed
18+
*/
19+
function closeIterator(iterator) {
20+
try {
21+
let close = iterator.return;
22+
if (isFunction(close)) {
23+
close.call(iterator);
24+
}
25+
} catch (closeError) {
26+
// NOP
27+
}
28+
}
29+
1030
export default function(instance) {
1131
instance.registerHelper('each', function(context, options) {
1232
if (!options) {
@@ -33,7 +53,7 @@ export default function(instance) {
3353
data = createFrame(options.data);
3454
}
3555

36-
function execIteration(field, index, last) {
56+
function execIteration(field, index, last, value) {
3757
if (data) {
3858
data.key = field;
3959
data.index = index;
@@ -47,31 +67,34 @@ export default function(instance) {
4767

4868
ret =
4969
ret +
50-
fn(context[field], {
70+
fn(value, {
5171
data: data,
52-
blockParams: blockParams(
53-
[context[field], field],
54-
[contextPath + field, null]
55-
)
72+
blockParams: blockParams([value, field], [contextPath + field, null])
5673
});
5774
}
5875

5976
if (context && typeof context === 'object') {
6077
if (isArray(context)) {
6178
for (let j = context.length; i < j; i++) {
6279
if (i in context) {
63-
execIteration(i, i, i === context.length - 1);
80+
execIteration(i, i, i === context.length - 1, context[i]);
6481
}
6582
}
6683
} else if (typeof Symbol === 'function' && context[Symbol.iterator]) {
67-
const newContext = [];
6884
const iterator = context[Symbol.iterator]();
69-
for (let it = iterator.next(); !it.done; it = iterator.next()) {
70-
newContext.push(it.value);
71-
}
72-
context = newContext;
73-
for (let j = context.length; i < j; i++) {
74-
execIteration(i, i, i === context.length - 1);
85+
let current = iterator.next();
86+
while (!current.done) {
87+
let next = iterator.next();
88+
try {
89+
execIteration(i, i, next.done, current.value);
90+
} catch (e) {
91+
if (!next.done) {
92+
closeIterator(iterator);
93+
}
94+
throw e;
95+
}
96+
current = next;
97+
i++;
7598
}
7699
} else {
77100
let priorKey;
@@ -81,13 +104,13 @@ export default function(instance) {
81104
// the last iteration without have to scan the object twice and create
82105
// an itermediate keys array.
83106
if (priorKey !== undefined) {
84-
execIteration(priorKey, i - 1);
107+
execIteration(priorKey, i - 1, false, context[priorKey]);
85108
}
86109
priorKey = key;
87110
i++;
88111
});
89112
if (priorKey !== undefined) {
90-
execIteration(priorKey, i - 1, true);
113+
execIteration(priorKey, i - 1, true, context[priorKey]);
91114
}
92115
}
93116
}

‎spec/builtins.js‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,114 @@ describe('builtin helpers', function() {
553553
'each with array argument ignores the contents when empty'
554554
)
555555
.toCompileTo('cruel world!');
556+
557+
expectTemplate('{{#each it}}{{#if @last}}{{text}}!{{/if}}{{/each}}')
558+
.withInput({
559+
it: new Iterable([
560+
{ text: 'goodbye' },
561+
{ text: 'Goodbye' },
562+
{ text: 'GOODBYE' }
563+
])
564+
})
565+
.withMessage('each on iterable sets @last for the final item')
566+
.toCompileTo('GOODBYE!');
567+
});
568+
569+
describe('each on iterable with lazy iterator', function() {
570+
var pulled, closed;
571+
572+
function countingIterable(length, withReturn) {
573+
var iterable = {};
574+
iterable[global.Symbol.iterator] = function() {
575+
var iterator = {
576+
next: function() {
577+
pulled++;
578+
return { value: pulled, done: pulled > length };
579+
}
580+
};
581+
if (withReturn === 'throwing getter') {
582+
Object.defineProperty(iterator, 'return', {
583+
get: function() {
584+
closed = true;
585+
throw new Error('getter failed');
586+
}
587+
});
588+
} else if (withReturn) {
589+
iterator['return'] = function() {
590+
// Only count calls with the iterator as the receiver.
591+
closed = this === iterator;
592+
if (withReturn === 'throw') {
593+
throw new Error('cleanup failed');
594+
}
595+
return { done: true };
596+
};
597+
}
598+
return iterator;
599+
};
600+
return iterable;
601+
}
602+
603+
function failOn(failingValue) {
604+
return function(value) {
605+
if (value === failingValue) {
606+
throw new Error('render failed');
607+
}
608+
};
609+
}
610+
611+
beforeEach(function() {
612+
pulled = 0;
613+
closed = false;
614+
});
615+
616+
it('renders while iterating instead of collecting all values first', function() {
617+
expectTemplate('{{#each items}}{{pulledCount}}{{/each}}')
618+
.withInput({ items: countingIterable(3, true) })
619+
.withHelper('pulledCount', function() {
620+
return pulled;
621+
})
622+
.toCompileTo('234');
623+
});
624+
625+
it('closes the iterator when the block throws', function() {
626+
expectTemplate('{{#each items}}{{fail this}}{{/each}}')
627+
.withInput({ items: countingIterable(3, true) })
628+
.withHelper('fail', failOn(2))
629+
.toThrow(Error, 'render failed');
630+
expect(closed).to.equal(true);
631+
expect(pulled).to.equal(3);
632+
});
633+
634+
it('does not close an exhausted iterator when the last block throws', function() {
635+
expectTemplate('{{#each items}}{{fail this}}{{/each}}')
636+
.withInput({ items: countingIterable(3, true) })
637+
.withHelper('fail', failOn(3))
638+
.toThrow(Error, 'render failed');
639+
expect(closed).to.equal(false);
640+
});
641+
642+
it('rethrows the block error when closing the iterator fails', function() {
643+
expectTemplate('{{#each items}}{{fail this}}{{/each}}')
644+
.withInput({ items: countingIterable(3, 'throw') })
645+
.withHelper('fail', failOn(2))
646+
.toThrow(Error, 'render failed');
647+
expect(closed).to.equal(true);
648+
});
649+
650+
it('rethrows the block error when reading the return method fails', function() {
651+
expectTemplate('{{#each items}}{{fail this}}{{/each}}')
652+
.withInput({ items: countingIterable(3, 'throwing getter') })
653+
.withHelper('fail', failOn(2))
654+
.toThrow(Error, 'render failed');
655+
expect(closed).to.equal(true);
656+
});
657+
658+
it('rethrows block errors for iterators without a return method', function() {
659+
expectTemplate('{{#each items}}{{fail this}}{{/each}}')
660+
.withInput({ items: countingIterable(3, false) })
661+
.withHelper('fail', failOn(2))
662+
.toThrow(Error, 'render failed');
663+
});
556664
});
557665
}
558666
});

‎spec/whitespace-control.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,33 @@ describe('whitespace control', function() {
154154
.withInput({ foo: 'bar' })
155155
.toCompileTo('barbar bar ');
156156
});
157+
158+
describe('performance', function() {
159+
// Each template below takes several seconds to compile, and so exceeds the
160+
// test timeout, if finding trailing whitespace is not linear in its length.
161+
var spaces = new Array(100001).join(' '),
162+
newlines = new Array(100001).join('\n');
163+
164+
it('should strip whitespace in linear time', function() {
165+
expectTemplate(spaces + 'x' + spaces + '{{~foo}}')
166+
.withInput({ foo: 'bar' })
167+
.toCompileTo(spaces + 'xbar');
168+
});
169+
170+
it('should detect standalone lines in linear time', function() {
171+
expectTemplate(newlines + 'x{{foo}}')
172+
.withInput({ foo: 'bar' })
173+
.toCompileTo(newlines + 'xbar');
174+
175+
expectTemplate('{{foo}}' + newlines + 'x{{foo}}')
176+
.withInput({ foo: 'bar' })
177+
.toCompileTo('bar' + newlines + 'xbar');
178+
});
179+
180+
it('should find partial indentation in linear time', function() {
181+
expectTemplate(spaces + 'x\n {{> dude}}\n')
182+
.withPartials({ dude: 'bar\n' })
183+
.toCompileTo(spaces + 'x\n bar\n');
184+
});
185+
});
157186
});

0 commit comments

Comments
 (0)