Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
jtrumbull opened this issue Mar 16, 2016 · 9 comments
Closed

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

jtrumbull opened this issue Mar 16, 2016 · 9 comments
Labels
Serialize
Milestone
3.0.0

Comments

@jtrumbull
Copy link
Contributor 

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
Copy link
Member

Makes sense to me. Would you like to submit a PR, @jtrumbull?

jtrumbull added a commit to jtrumbull/jquery that referenced this issue Mar 17, 2016
@jtrumbull
Fix: Issue # 3005
773e138 
jquery#3005
@jtrumbull jtrumbull mentioned this issue Mar 17, 2016
Closed
@jtrumbull
Copy link
Contributor Author

@gibson042 Yeah, just wanted to confirm that there wasn't reasoning behind the current setup.

jtrumbull added a commit to jtrumbull/jquery that referenced this issue Mar 17, 2016
@jtrumbull
Update unit tests for Issue jquery#3005
9cdef40
@dmethvin
Copy link
Member

dmethvin commented Apr 4, 2016

This is a breaking change so it either needs to land ASAP or wait for 4.0.

@mgol
Copy link
Member

mgol commented Apr 4, 2016

The PR looks good to me. Should we just land it?

@dmethvin
Copy link
Member

dmethvin commented Apr 4, 2016

👍 from me

@gibson042
Copy link
Member

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
Copy link
Member

mgol commented Apr 5, 2016

Hmm, OK. But maybe it's still best to not backport this to 1.12/2.2.

@gibson042
Copy link
Member

I'm fine with that.

@dmethvin
Copy link
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
@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Serialize
Milestone
3.0.0
Development

No branches or pull requests
4 participants
@dmethvin @gibson042 @mgol @jtrumbull