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

can.sub should return null if value is undefined or null #429

Merged
merged 4 commits into from Jun 14, 2013

Conversation

ccummings
Copy link
Contributor

Currently can.sub only returns null if the property it is trying to replace is undefined. It should do the same if the value is null.

This bug rears its ugly head in can.Control when templated event handlers get parsed.

Say you have a compute named val1 in your defaults which is null and you have a templated event that looks like '{val1} change. When can.sub is run on that string and the options you get ' change' which will setup a binding on the Control's element for any change event from the DOM (from form elements).

So in this case, you get this very weird behavior of having the handler for a compute change run when form elements are updated.

Example fiddle: http://jsfiddle.net/jCjBw/1/

@@ -183,7 +183,7 @@ steal('can/util', function (can) {
// Convert inside to type.
var ob = can.getObject(inside, data, remove === true ? false : undefined);

if (ob === undefined) {
if (ob == undefined) {//undefined or null
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make this if(ob === undefined || ob === null) to be more explicit?

daffl added a commit that referenced this pull request Jun 14, 2013
can.sub should return null if value is undefined or null
@daffl daffl merged commit 34c1215 into master Jun 14, 2013
@daffl daffl deleted the can-sub-null-values branch June 14, 2013 22:11
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

2 participants