Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boolean controller properties set to null parse as truthy strings #13686

Closed
allthesignals opened this issue Jun 15, 2016 · 6 comments
Closed

Comments

@allthesignals
Copy link
Contributor

allthesignals commented Jun 15, 2016

When a queryParam on a controller is defaulted to null (such that it won't be serialized in a request to the API unless changed), and when that property is toggled (via this.toggleProperty), it produces this sequence of values: true -> false -> false -> true. I think this is because at some point it's parsed as a string, which is truthy.

Twiddle: https://ember-twiddle.com/3afa1091106a91ce2c1734ae2998bc3f?openFiles=controllers.application.js%2C&route=%2F%3Fredevelopment%3Dfalse

Conditions:
Controller property is set to null.
Controller property is a queryParam.
Controller property is toggled to true or false.

Behavior:
Toggling the property produces this sequence of values: true -> false -> false -> true.

While setting the default value to false solves the issue, null queryParams aren't serialized in API requests to the server, which is helpful.

Controller:

App.IndexController = Ember.Controller.extend({
  queryParams: ["redevelopment"],
  redevelopment: null,
  actions: {
    toggleIt: function() {
      this.toggleProperty("redevelopment");
    }
  }
});

Sorry if this is already covered, it is mentioned here: #4570. I'm just trying to understand if this is intentional or not.

@Serabe Serabe added the Bug label Jun 16, 2016
@Serabe
Copy link
Member

Serabe commented Jun 16, 2016

I can confirm this is happening in canary too. Thank you for your report!

@allthesignals
Copy link
Contributor Author

allthesignals commented Jun 17, 2016

Thanks. I think the query parameter is overwriting the value as a string, "false", which toggles to false. !!"false" == true.

It would be nice to have something like this in the API:

export default Ember.Controller.extend({
  queryParams: {
    category: {
      type: "boolean"
    }
  },
  category: null
});

EDIT:
It does! This seems to fix the issue:

export default Ember.Controller.extend({
  queryParams: [{ 
     redevelopment: { 
         type: 'boolean' 
     } 
  }],
  redevelopment: null
});

Maybe an enhancement would be more intelligent coercion of values, but that might not be possible, since it would be based on the future lifetime of the value.

@mixonic
Copy link
Sponsor Member

mixonic commented Jun 26, 2016

@allthesignals is this perhaps ready to be closed? The coercion cannot even be perfect (as you identified we cannot know all future values) and I think you should be strongly encouraged to use a default type matching your eventual value and/or to use the explicit type syntax.

@knownasilya
Copy link
Contributor

It needs to be documented before we close.

@ghost
Copy link

ghost commented Apr 7, 2017

this ambiguity cost our team numerous hours of debugging. I started some documentation. This could use some further exposition in the guides as well; could throw together a more verbose description of the behavior as well.

@locks
Copy link
Contributor

locks commented Apr 8, 2017

Documentation merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants