Skip to content

Conversation

@Henridv
Copy link
Contributor

@Henridv Henridv commented Jun 9, 2018

Fixes issue #10

@Henridv Henridv changed the title Support boolean values in this.get() #1 Support boolean values in this.get() #10 Jun 9, 2018
@Henridv Henridv changed the title Support boolean values in this.get() #10 Support boolean values in this.get() Jun 9, 2018
let thisValue = {
get(property) {
return get(changes, property) || get(content, property);
return get(changes, property) != null
Copy link
Owner

@iamdtang iamdtang Jul 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work for false, but not undefined or null. How about this instead?

let thisValue = {
  get(property) {
    if (property.includes('.')) {
      return get(changes, property) || get(content, property);
    }

    if (changes.hasOwnProperty(property)) {
      return get(changes, property);
    } else {
      return get(content, property);
    }
  }
};

If the property exists in changes, then return that. This way, it should work with undefined and null too. Otherwise, return the property from content.

I'm not sure how best to handle property paths (i.e. this.get('a.b')), so that might need some more thinking, hence the check for a dot in the first conditional.

Copy link

@adambedford adambedford Mar 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has just become an issue for us. Setting a property to null doesn't behave as you'd expect because it gets proxied to the model, which still has the value.

@iamdtang
Copy link
Owner

iamdtang commented Jul 8, 2018

Hey @Henridv, sorry for the delay. I am back from vacation now. :)

@iamdtang iamdtang merged commit 1bf06ac into iamdtang:master Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants