Skip to content

Commit

Permalink
reimplement noncommutative sort
Browse files Browse the repository at this point in the history
the workaround for handlebars-lang/handlebars.js/issues/748 employed a sorting
routine that was not commutative; its output relied on the order of its
arguments, and with the V8 engine upgrade in node11, that order has
changed[1]

the order of the arguments to Array#sory is considered an implementation
detail that we can't rely on

this patch reimplements that sorting routine to always return the same
result regardless of the order of its operands, and so it should work on
node <= 10 and >= 11

test plan: there is already a case for this, so I just added node12 to
           the travis language matrix

[1]: nodejs/node#24294
  • Loading branch information
amireh committed Nov 18, 2020
1 parent 6ebdcba commit 36a569a
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 42 deletions.
5 changes: 2 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
before_script: 'npm install -g grunt-cli'
language: node_js
node_js:
- "6"
- "5"
- "4"
- "10"
- "12"
27 changes: 8 additions & 19 deletions dist/lib/pre_processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,6 @@ function concatNode(string, tempMap) {
return new SexprNode(parts);
}

function sortBy(array, fn) {
return array.map(function (item, i) {
return [fn(item), i, item];
}).sort(function (left, right) {
var leftSort = left[0],
rightSort = right[0];
if (leftSort !== rightSort) {
if (leftSort > rightSort) return 1;
if (leftSort < rightSort) return 0;
}
return left[1] - right[1];
}).map(function (obj) {
return obj[2];
});
}

var PreProcessor = {
process: function process(ast) {
var statements = ast.statements,
Expand Down Expand Up @@ -180,9 +164,14 @@ var PreProcessor = {
* https://github.com/wycats/handlebars.js/issues/767
*/
applySubExpressionHack: function applySubExpressionHack(node) {
node.hash.pairs = sortBy(node.hash.pairs, function (pair) {
var type = pair[1].type;
return type === "sexpr" ? 0 : 1;
node.hash.pairs = node.hash.pairs.sort(function (a, b) {
if (a[1].type === b[1].type) {
return 0;
} else if (a[1].type === 'sexpr') {
return -1;
} else if (b[1].type === 'sexpr') {
return 1;
}
});
var pairs = node.hash.pairs;
if (pairs.length > 1 && pairs[1][1].type === "sexpr") throw new _errors2.default.MultipleSubExpressionsError(node.firstLine, "Handlebars 1.3 doesn't support multiple sub-expressions in the options hash");
Expand Down
29 changes: 10 additions & 19 deletions lib/pre_processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,6 @@ function concatNode(string, tempMap) {
return new SexprNode(parts);
}

function sortBy(array, fn) {
return array.map(function(item, i) {
return [fn(item), i, item];
}).sort(function(left, right) {
var leftSort = left[0]
, rightSort = right[0];
if (leftSort !== rightSort) {
if (leftSort > rightSort) return 1;
if (leftSort < rightSort) return 0;
}
return left[1] - right[1];
}).map(function(obj) {
return obj[2];
});
}

var PreProcessor = {
process: function(ast) {
var statements = ast.statements
Expand Down Expand Up @@ -176,9 +160,16 @@ var PreProcessor = {
* https://github.com/wycats/handlebars.js/issues/767
*/
applySubExpressionHack: function(node) {
node.hash.pairs = sortBy(node.hash.pairs, function(pair) {
var type = pair[1].type;
return type === "sexpr" ? 0 : 1;
node.hash.pairs = node.hash.pairs.sort(function(a,b) {
if (a[1].type === b[1].type) {
return 0
}
else if (a[1].type === 'sexpr') {
return -1;
}
else if (b[1].type === 'sexpr') {
return 1;
}
});
var pairs = node.hash.pairs;
if (pairs.length > 1 && pairs[1][1].type === "sexpr")
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"grunt-contrib-clean": "~1.0.0",
"grunt-contrib-copy": "~1.0.0",
"gruntify-eslint": "~3.1.0",
"handlebars": ">=1.3 <3",
"handlebars": "~1.3.0",
"i18nliner": "~0.2.0",
"jsdom": "~8.5.0",
"matchdep": "~1.0.1",
Expand Down

0 comments on commit 36a569a

Please sign in to comment.