/jquery

New issue

$.param() returns "null" and "undefined" literal if attribute is function #3005

Closed
jtrumbull opened this Issue Mar 16, 2016 · 9 comments

Assignees

No one assigned

Labels

Serialize

Projects

None yet

Milestone

  3.0.0

4 participants

@jtrumbull @gibson042 @dmethvin @mgol
@jtrumbull
Contributor
jtrumbull commented Mar 16, 2016 
var param = $.param({
  a: function () {},
  b: function () { return null; }
});

Will result in a=undefined&b=null

jsfiddle

The value == null check on Line 61 should happen after the ternary statement, since both value and the result of value() can be null. i.e.

        add = function( key, value ) {

            // If value is a function, invoke it and return its value
            value = jQuery.isFunction( value ) ? value() : value;
            value = value == null ? "" : value;
            s[ s.length ] = encodeURIComponent( key ) + "=" + encodeURIComponent( value );
        };
@gibson042
Member
gibson042 commented Mar 16, 2016

Makes sense to me. Would you like to submit a PR, @jtrumbull?
@gibson042 gibson042 added the Serialize label Mar 16, 2016
@jtrumbull jtrumbull added a commit to jtrumbull/jquery that referenced this issue Mar 17, 2016
@jtrumbull jtrumbull Fix: Issue # 3005 
jquery#3005
 773e138
@jtrumbull jtrumbull referenced this issue Mar 17, 2016
Closed

Serialize: set empty string if value function returns null #3007

@jtrumbull
Contributor
jtrumbull commented Mar 17, 2016

@gibson042 Yeah, just wanted to confirm that there wasn't reasoning behind the current setup.
@jtrumbull jtrumbull added a commit to jtrumbull/jquery that referenced this issue Mar 17, 2016
@jtrumbull jtrumbull Update unit tests for Issue #3005 9cdef40
@dmethvin
Member
dmethvin commented Apr 4, 2016

This is a breaking change so it either needs to land ASAP or wait for 4.0.
@mgol mgol added the Behavior Change label Apr 4, 2016
@mgol
Member
mgol commented Apr 4, 2016

The PR looks good to me. Should we just land it?
@dmethvin
Member
dmethvin commented Apr 4, 2016

👍 from me
@gibson042 gibson042 added a commit that closed this issue Apr 5, 2016
@jtrumbull @gibson042 jtrumbull + gibson042 Serialize: Treat literal and function-returned null/undefined the same 
Fixes gh-3005

Closes gh-3007
 9fdbdd3
@gibson042 gibson042 closed this in 9fdbdd3 Apr 5, 2016
@gibson042
Member
gibson042 commented Apr 5, 2016

I don't think this is a breaking change, since "the return value of a function is used instead of the function as a String" still holds (http://api.jquery.com/jQuery.param/ ). But please make an issue or PR in https://github.com/jquery/api.jquery.com if you disagree.
@mgol
Member
mgol commented Apr 5, 2016

Hmm, OK. But maybe it's still best to not backport this to 1.12/2.2.
@gibson042
Member
gibson042 commented Apr 5, 2016

I'm fine with that.
@dmethvin
Member
dmethvin commented Apr 5, 2016

I see your point, let's call this a 3.0 feature/fix.
@gibson042 gibson042 added this to the 3.0.0 milestone Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment