escaping null #427

Closed
lazylester opened this Issue Jan 10, 2012 · 5 comments

Comments

Projects
None yet
5 participants
@lazylester

I was surprised that _.escape(null) produces the string "null". Wouldn't the "principle of least surprise" suggest that the result should be a blank string, or maybe even null? Of course, I realize that the principle of least surprise is subjective, but I raise this since it might be an oversight rather than the intended and expected operation.

Thanks for underscore, and for your contribution to open source.

Les

@jashkenas

This comment has been minimized.

Show comment
Hide comment
@jashkenas

jashkenas Jan 10, 2012

Owner

In a dynamically typed language, you can pass any value to any function ... but that doesn't mean you should. The Underscore collection functions have undefined behavior when passed anything other than Arrays or Objects; _.escape has undefined behavior when passed anything that's not a string. This isn't something that should be expected by Underscore itself. You can use:

_.escape(value || "");
Owner

jashkenas commented Jan 10, 2012

In a dynamically typed language, you can pass any value to any function ... but that doesn't mean you should. The Underscore collection functions have undefined behavior when passed anything other than Arrays or Objects; _.escape has undefined behavior when passed anything that's not a string. This isn't something that should be expected by Underscore itself. You can use:

_.escape(value || "");

@jashkenas jashkenas closed this Jan 10, 2012

@mattyurka

This comment has been minimized.

Show comment
Hide comment
@mattyurka

mattyurka Mar 27, 2012

In my opinion _.escape(null) should evaluate to an empty string. The change only adds like 6 characters and it's common for developers to, for example, fetch a model with null attributes and then try to render it in an escaped template.

The current version also causes an inconsistency in _.template where <%= null_value %> evaluates to "" but <%- null_value %> evaluates to "null". It seems more elegant to make this change in _.escape rather than force developers to use <%= _.escape(value || "") =>.

In my opinion _.escape(null) should evaluate to an empty string. The change only adds like 6 characters and it's common for developers to, for example, fetch a model with null attributes and then try to render it in an escaped template.

The current version also causes an inconsistency in _.template where <%= null_value %> evaluates to "" but <%- null_value %> evaluates to "null". It seems more elegant to make this change in _.escape rather than force developers to use <%= _.escape(value || "") =>.

@octatone

This comment has been minimized.

Show comment
Hide comment
@octatone

octatone Mar 27, 2012

Contributor

@mattyurka

_.escape escapes all keywords and objects in the same fashion and returns strings. Are you suggesting making exceptions to the behavior per keyword/object type?

http://jsfiddle.net/octatone/FxPLS/

I think the expected behavior for _.escape is to return an escaped string representation of whatever is passed to it.

Contributor

octatone commented Mar 27, 2012

@mattyurka

_.escape escapes all keywords and objects in the same fashion and returns strings. Are you suggesting making exceptions to the behavior per keyword/object type?

http://jsfiddle.net/octatone/FxPLS/

I think the expected behavior for _.escape is to return an escaped string representation of whatever is passed to it.

@mattyurka

This comment has been minimized.

Show comment
Hide comment
@mattyurka

mattyurka Mar 27, 2012

@octatone

I was suggesting treating input as (input || '') in _.escape. This would only change undefined behavior. If you want to display NaN, undefined, false or null just pass in (input + '').

But I see how this is inelegant. In my case I have nulls in my models and don't want to spit nulls at the user. My gut tells me this is what other developers would want but I don't really know.

As a side note, in my browser Array.join converts nulls and undefineds into empty strings, which is the cause of the inconsistency I mentioned above. Limiting ourselves to just nulls and undefineds, however, would add more cruft: (_.isNull(input) || _.isUndefined(input)) ? '' : input

@octatone

I was suggesting treating input as (input || '') in _.escape. This would only change undefined behavior. If you want to display NaN, undefined, false or null just pass in (input + '').

But I see how this is inelegant. In my case I have nulls in my models and don't want to spit nulls at the user. My gut tells me this is what other developers would want but I don't really know.

As a side note, in my browser Array.join converts nulls and undefineds into empty strings, which is the cause of the inconsistency I mentioned above. Limiting ourselves to just nulls and undefineds, however, would add more cruft: (_.isNull(input) || _.isUndefined(input)) ? '' : input

@russplaysguitar

This comment has been minimized.

Show comment
Hide comment
@russplaysguitar

russplaysguitar Jun 11, 2012

Contributor

+1
right now i'm adding a wrapper for escape() to my code base that adds || "" so users see "" and not "undefined"

Contributor

russplaysguitar commented Jun 11, 2012

+1
right now i'm adding a wrapper for escape() to my code base that adds || "" so users see "" and not "undefined"

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