From b9753dcc7156d8471a5aa5b6c9b85af47f630aa8 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 22 Mar 2021 03:23:29 -0400 Subject: [PATCH] Changeset: Return a new op object by default when iterating Reusing the same op object for each iteration can result in very weird behaviors because previously yielded op objects will get a surprise mutation. It is unclear why the code was written to reuse the same object. There was no comment, nor is there a commit message providing rationale (it has behaved this way since the very first commit). Perhaps the objects were reused to improve performance (fewer object allocations that need to be garbage collected). I do expect this change to reduce performance somewhat, but not enough to warrant reverting this commit. --- src/static/js/Changeset.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 8c3627b04..6c236129b 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -126,10 +126,9 @@ exports.opIterator = (opsStr, optStartIndex) => { return result; }; let regexResult = nextRegexMatch(); - const obj = exports.newOp(); - const next = (optObj) => { - const op = (optObj || obj); + const next = (optOp) => { + const op = optOp || exports.newOp(); if (regexResult[0]) { op.attribs = regexResult[1]; op.lines = exports.parseNum(regexResult[2] || 0);