Object mysteriously updated without update function called

Question

The expected behavior is for the user to be able to style all three elements individually.

How it works:

When each element is clicked, it is stored in selected.elements
When "Bold/Italic" are clicked, it is stored in selected.settings
When "Apply" is clicked, the DOM is updated using the elements & settings in the "selected" object, and selected.elements is cleared

Steps to reproduce:

Click "elementOne"
Select "Bold"
Click "Apply"
Click "elementTwo"
Select "Italic"
Click "Apply"

Expected: elementOne is Bold; elementTwo is Italic

Actual: Both are Italic

At Step 5, model.elementOne.settings is updated even though there is no function called to do that.

const selected = {
  elements: [],
  settings: {}
}
const model = {
  elementOne: {
    slug: 'elementOne',
    settings: {}
  },
  elementTwo: {
    slug: 'elementTwo',
    settings: {}
  },
  elementThree: {
    slug: 'elementThree',
    settings: {}
  }
}
const elementContainer = document.getElementById('elementContainer')
const buttonContainer = document.getElementById('buttonContainer')
const fontStyleContainer = document.getElementById('fontStyleButtonContainer')

const setupEventListeners = _ => {
  elementContainer.addEventListener('click', e => {
    // add the selected element to selected.elements
    selected.elements.push(e.target.id)
  })

  fontStyleContainer.addEventListener('click', e => {
    // add the selected font style to the selected.settings
    for (let i = 0; i < selected.elements.length; i++) {
      if (e.target.hasAttribute('data-bold')) {
        selected.settings.fontStyle = 'bold'
      } else if (e.target.hasAttribute('data-italic')) {
        selected.settings.fontStyle = 'italic'
      }
    }
  })

  buttonContainer.addEventListener('click', e => {
    // update the model for each selected element using the selected settings
    for (let i = 0; i < selected.elements.length; i++) {
      model[selected.elements[i]].settings = selected.settings
    }
    // update the DOM based on the new model
    updateDOMFromModel()
    // reset the selected elements & settings
    selected.elements.length = 0
  })
}
const updateDOMFromModel = _ => {
  // update the DOM based on the new model
  for (const el in model) {
    if (model.hasOwnProperty(el)) {
      const element = model[el]
      const elementDOM = document.getElementById(element.slug)
      switch (element.settings.fontStyle) {
        case 'bold':
          elementDOM.style = 'font-weight: 800;'
          break
        case 'italic':
          elementDOM.style = 'font-style: italic;'
          break
        default:
      }
    }
  }
}
setupEventListeners()

<body>
  <div id="elementContainer" style="cursor: pointer;">
    <p>1. Select an element to style</p>
    <ul>
      <li id="elementOne">elementOne</li>
      <li id="elementTwo">elementTwo</li>
      <li id="elementThree">elementThree</li>
    </ul>
  </div>
  <div id="fontStyleButtonContainer" style="cursor: pointer;">
    <p>2. Select a font style</p>
    <div id="fontStyleButton" data-bold>Bold</div>
    <div id="fontStyleButton" data-italic>Italic</div>
  </div>
  <div id="buttonContainer" style="cursor: pointer;">
    <p>3. Apply changes</p>
    <button id="btnApply">Apply</button>
  </div>
</body>

@mplungjan: thank you. I read the posting guidelines and have tried to keep it as short as possible while still allowing it to be reproduced. If you run the code snippet, you will see my steps to reproduce work. — richie, Apr 12 '18 at 05:16
How it should work, how it actually works, hints of debugging, snippet to reproduce the problem in the question -> I love it! *thumbs up* — Andreas, Apr 12 '18 at 05:20
`model[selected.elements[i]].settings = selected.settings` - This line stores a reference to `selected.settings` in the models settings but not a copy ([reference vs. primitive type](https://stackoverflow.com/questions/518000/is-javascript-a-pass-by-reference-or-pass-by-value-language)). In step 5 the one and only `settings` object is modified which is also referenced by the first clicked element, hence both get updated in `updateDOMFromModel`. — Andreas, Apr 12 '18 at 05:37
@andreas Yep, you're right as well. I appreciate the link so I could better understand the distinction. Thanks! — richie, Apr 12 '18 at 06:00

score 2 · Accepted Answer · answered Apr 12 '18 at 05:41

By using always the same object in selected.settings, you bind the settings of your model's elements between them because they eventually point on the same thing.

Here I just re-set a new object for selected.settings and it works! (Check in the fontStyleContainer click listener) Watch out if you use values or references ;)

const selected = {
  elements: [],
  settings: {}
}
const model = {
  elementOne: {
    slug: 'elementOne',
    settings: {}
  },
  elementTwo: {
    slug: 'elementTwo',
    settings: {}
  },
  elementThree: {
    slug: 'elementThree',
    settings: {}
  }
}
const elementContainer = document.getElementById('elementContainer')
const buttonContainer = document.getElementById('buttonContainer')
const fontStyleContainer = document.getElementById('fontStyleButtonContainer')

const setupEventListeners = _ => {
  elementContainer.addEventListener('click', e => {
    // add the selected element to selected.elements
    selected.elements.push(e.target.id)
  })

  fontStyleContainer.addEventListener('click', e => {
    // add the selected font style to the selected.settings
    for (let i = 0; i < selected.elements.length; i++) {
      if (e.target.hasAttribute('data-bold')) {
        // Each time a new object!
        selected.settings = {fontStyle: 'bold'}
      } else if (e.target.hasAttribute('data-italic')) {
        // Each time a new object!
        selected.settings = {fontStyle: 'italic'}
      }
    }
  })

  buttonContainer.addEventListener('click', e => {
    // update the model for each selected element using the selected settings
    for (let i = 0; i < selected.elements.length; i++) {
      model[selected.elements[i]].settings = selected.settings
    }
    // update the DOM based on the new model
    updateDOMFromModel()
    // reset the selected elements & settings
    selected.elements.length = 0
  })
}
const updateDOMFromModel = _ => {
  // update the DOM based on the new model
  for (const el in model) {
    if (model.hasOwnProperty(el)) {
      const element = model[el]
      const elementDOM = document.getElementById(element.slug)
      switch (element.settings.fontStyle) {
        case 'bold':
          elementDOM.style = 'font-weight: 800;'
          break
        case 'italic':
          elementDOM.style = 'font-style: italic;'
          break
        default:
      }
    }
  }
}
setupEventListeners()

<body>
  <div id="elementContainer" style="cursor: pointer;">
    <p>1. Select an element to style</p>
    <ul>
      <li id="elementOne">elementOne</li>
      <li id="elementTwo">elementTwo</li>
      <li id="elementThree">elementThree</li>
    </ul>
  </div>
  <div id="fontStyleButtonContainer" style="cursor: pointer;">
    <p>2. Select a font style</p>
    <div id="fontStyleButton" data-bold>Bold</div>
    <div id="fontStyleButton" data-italic>Italic</div>
  </div>
  <div id="buttonContainer" style="cursor: pointer;">
    <p>3. Apply changes</p>
    <button id="btnApply">Apply</button>
  </div>
</body>

Wow thank you so much! This one had me stuck for a while! I had a feeling there was a reference between the selected.settings object and elementOne, but I could not understand how and had no vocabulary to usefully google it. Appreciate the help!! — richie, Apr 12 '18 at 05:56
@mplungjan Comments are already there: search for `// Each time a new object! ` — sjahan, Apr 12 '18 at 07:13

Object mysteriously updated without update function called

1 Answers1