Skip to content

Commit

Permalink
Changeset: Return a new op object by default when iterating
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rhansen committed Mar 29, 2021
1 parent 718da6f commit b9753dc
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions src/static/js/Changeset.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit b9753dc

Please sign in to comment.