Skip to content

Commit

Permalink
Invert boolean argument on OrderedElements set()
Browse files Browse the repository at this point in the history
As @xymostech pointed out in code review

> I'd assume that preserveOrder would be the default in something called
> OrderedElements

This makes sense to me, so I am inverting this boolean and explicitly
passing it in everywhere it is used now.
  • Loading branch information
lencioni committed Oct 11, 2017
1 parent a60aaa8 commit 1ca597d
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export const generateCSS = (
// If none of the handlers handled it, add it to the list of plain
// style declarations.
if (!foundHandler) {
plainDeclarations.set(key, val);
plainDeclarations.set(key, val, true);
}
});

Expand Down Expand Up @@ -224,7 +224,7 @@ const runStringHandlers = (
// Preserve order here, since we are really replacing an
// unprocessed style with a processed style, not overriding an
// earlier style
true
false
);
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/ordered-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export default class OrderedElements {
}
}

set(key /* : string */, value /* : any */, preserveOrder /* : ?boolean */) {
set(key /* : string */, value /* : any */, shouldReorder /* : ?boolean */) {
if (!this.elements.hasOwnProperty(key)) {
this.keyOrder.push(key);
} else if (!preserveOrder) {
} else if (shouldReorder) {
const index = this.keyOrder.indexOf(key);
this.keyOrder.splice(index, 1);
this.keyOrder.push(key);
Expand All @@ -40,7 +40,7 @@ export default class OrderedElements {
? this.elements[key]
: new OrderedElements();
value.forEach((value, key) => {
nested.set(key, value);
nested.set(key, value, shouldReorder);
});
this.elements[key] = nested;
return;
Expand All @@ -54,7 +54,7 @@ export default class OrderedElements {
: new OrderedElements();
const keys = Object.keys(value);
for (let i = 0; i < keys.length; i += 1) {
nested.set(keys[i], value[keys[i]]);
nested.set(keys[i], value[keys[i]], shouldReorder);
}
this.elements[key] = nested;
return;
Expand All @@ -74,12 +74,12 @@ export default class OrderedElements {
addStyleType(styleType /* : any */) /* : void */ {
if ((MAP_EXISTS && styleType instanceof Map) || styleType instanceof OrderedElements) {
styleType.forEach((value, key) => {
this.set(key, value);
this.set(key, value, true);
});
} else {
const keys = Object.keys(styleType);
for (let i = 0; i < keys.length; i++) {
this.set(keys[i], styleType[keys[i]]);
this.set(keys[i], styleType[keys[i]], true);
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions tests/ordered-elements_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe("OrderedElements", () => {
}, elems);
});

it('moves overridden elements to the end', () => {
it('preserves original order when overriding', () => {
const elems = new OrderedElements();

elems.set("a", 1);
Expand All @@ -62,14 +62,14 @@ describe("OrderedElements", () => {

assert.deepEqual({
elements: {
b: 1,
a: 2,
b: 1,
},
keyOrder: ["b", "a"],
keyOrder: ["a", "b"],
}, elems);
});

it('can preserve order when setting', () => {
it('can reorder when overriding', () => {
const elems = new OrderedElements();

elems.set("a", 1);
Expand All @@ -78,10 +78,10 @@ describe("OrderedElements", () => {

assert.deepEqual({
elements: {
a: 2,
b: 1,
a: 2,
},
keyOrder: ["a", "b"],
keyOrder: ["b", "a"],
}, elems);
});

Expand Down

0 comments on commit 1ca597d

Please sign in to comment.