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

Return empty string instead of 'undefined' #8

Closed
wants to merge 1 commit into from
Closed

Return empty string instead of 'undefined' #8

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 21, 2012

No description provided.

@timoxley
Copy link
Member

What if obj[name] === 0?

@timoxley
Copy link
Member

The only cases I can see where you'd probably not want to print out the toString() of a falsey obj[name] is:

  • if obj doesn't have the name key or
  • the value of obj[name] is undefined
  • the value of obj[name] is null

Printing 'false' and '0' seems reasonable to me.
Perhaps something like this would be better:

var value = obj[name];
if (typeof value === "undefined" || value === null) return '';
return value;

@tj
Copy link
Member

tj commented Dec 29, 2012

hmm yeah -.5 from me haha, typically you'd want to do data-show etc and toggle an entire element, the text content of one blank may be a little hard to track down if we're converting to an empty string. Also since .value() is public IMO it should be the canonical value and if we were to convert to empty string it should be done in the data-text binding and have a value == null check like @timoxley mentioned to handle undefined/null

@tj
Copy link
Member

tj commented Jan 5, 2013

going to close this one for now, we do need a way to opt-out of setting the text at all for some cases as well, as a sort of default mechanism, but it without opting in it would be annoying and not really all that obvious

@tj tj closed this Jan 5, 2013
@timoxley timoxley mentioned this pull request Jan 8, 2013
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.

None yet

3 participants