elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

Update applyFacts default case #135

Closed decioferreira closed 6 years ago

decioferreira commented 6 years ago

the previous logic (key !== 'value' || key !== 'checked' || domNode[key] !== value) would always resolve to true.

Here are some test cases trying to better explain the reasoning behind the proposed solution:

function applyFactsDefaultCase(key, domNodeValue, value) {
  var domNode = {};
  domNode[key] = domNodeValue;

  // ORIGINAL
  // return (key !== 'value' || key !== 'checked' || domNode[key] !== value);

  // PROPOSED SOLUTION
  return ((key !== 'value' && key !== 'checked') || domNode[key] !== value);
}

// Test cases
console.log([
  // key === 'value', same value, no update to the DOM
  applyFactsDefaultCase('value', 'foo', 'foo') === false,
  // key === 'value', updated value, updates the DOM
  applyFactsDefaultCase('value', 'foo', 'foo2') === true,
  // key === 'value', initial value, updates the DOM
  applyFactsDefaultCase('value', '', 'foo') === true,
  // key === 'value', initial empty value, no update to the DOM
  applyFactsDefaultCase('value', '', '') === false,

  // key === 'checked', same value, no update to the DOM
  applyFactsDefaultCase('checked', 'foo', 'foo') === false,
  // key === 'checked', updated value, updates the DOM
  applyFactsDefaultCase('checked', 'foo', 'foo2') === true,
  // key === 'checked', initial value, updates the DOM
  applyFactsDefaultCase('checked', '', 'foo') === true,
  // key === 'checked', initial empty value, no update to the DOM
  applyFactsDefaultCase('checked', '', '') === false,

  // key !== 'value' && key !== 'checked', same value, updates the DOM
  applyFactsDefaultCase('bar', 'foo', 'foo') === true,
  // key !== 'value' && key !== 'checked', updated value, updates the DOM
  applyFactsDefaultCase('bar', 'foo', 'foo2') === true,
  // key !== 'value' && key !== 'checked', initial value, updates the DOM
  applyFactsDefaultCase('bar', '', 'foo') === true,
  // key !== 'value' && key !== 'checked', initial empty value, updates the DOM
  applyFactsDefaultCase('bar', '', '') === true
]);

fixes elm/virtual-dom/issues/134