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

$.param(undefined) - throws Cannot read property 'jquery' of undefined #2633

Closed
MarkPieszak opened this issue Oct 6, 2015 · 14 comments
Closed

$.param(undefined) - throws Cannot read property 'jquery' of undefined #2633

MarkPieszak opened this issue Oct 6, 2015 · 14 comments
Assignees
Labels
Milestone

Comments

@MarkPieszak
Copy link

@MarkPieszak MarkPieszak commented Oct 6, 2015

Using jQuery 1.11.3 in Chrome, I'm not sure if this is a bug or a "nice to have feature", but if you try to do a $.param() on something undefined it throws the error:

Cannot read property 'jquery' of undefined

This mainly happens when let's say your passing parameters into a function and sometimes you don't want/need to pass any:

function Example (parametersToPass) {
    $.param( parametersToPass );
}


Example(); // <-- will error out.

The easiest fix is to simply:

$.param( parametersToPass || '' );
@mgol
Copy link
Member

@mgol mgol commented Oct 6, 2015

Thanks for the report.

$.param expects an object which undefined is not so passing undefined is unsupported. I'll treat it as a feature request then and leave open for now so that others can voice their opinions.

We've recently made $.map & $.each accept undefined/null input so I'm not opposed to treating them lightly here. We should determine, though, how many more methods would have to get similar treatment.

@MarkPieszak
Copy link
Author

@MarkPieszak MarkPieszak commented Oct 7, 2015

Thanks, yeah I figured it's more of a "nice to have" feature, just wanted to get it out there just incase no one ran into it!

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Oct 20, 2015

This is really easy to fix on the caller's side though. If you know the value may be undefined, use the || "" trick to define it as an empty string. If we put a bunch of this inside jQuery itself, there is a good chance it will mask errors and make the API harder for people to use.

@mgol
Copy link
Member

@mgol mgol commented Oct 20, 2015

@dmethvin You could say the same about $.map/$.each, though and yet we accepted to treat null & undefined differently.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Oct 20, 2015

I agree. If those weren't regressions (in undocumented behavior) I don't think we would have changed them. Or at least I wouldn't have been in favor of it. 😄

@jaubourg
Copy link
Member

@jaubourg jaubourg commented Oct 20, 2015

I'd tend to think consistency trumps history in the matter and I'd be in favor of gracefully handling the nul/undefined case.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Oct 20, 2015

So is the suggestion to handle just $.param(undefined) here, or are we creating a bigger ticket to review all the APIs and allow null/undefined where possible? Will we update all the docs as well? The APIs have been around for almost a decade without someone needing this so I'm concerned we're moving into yak-shaving territory with so much else we have to do.

@mgol
Copy link
Member

@mgol mgol commented Oct 21, 2015

@mgol
Copy link
Member

@mgol mgol commented Oct 27, 2015

Should we treat it as a breaking change or a new feature? In the latter case this could be moved to 3.1.0; I'd rather not add new issues to the 3.0.0 milestone at this point.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Oct 27, 2015

Definitely a feature, since we're talking about expanding the signature.

@mgol mgol added this to the 3.1.0 milestone Oct 27, 2015
@mgol
Copy link
Member

@mgol mgol commented Oct 27, 2015

OK, milestone 3.1.0 added.

@jtrumbull
Copy link
Contributor

@jtrumbull jtrumbull commented Mar 17, 2016

(Hopefully not too far off topic) Since a change is being considered to handle null and undefined, would it be a bad idea to also handle a function argument? Similar to how object iterator invokes attributes that are functions.

$.param(function(){
  var params = {};
  // Build params
  return params;
});

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Mar 17, 2016

I'm not a fan of adding more behavior to this API (and documenting it, don't forget). Unfortunately I suppose we've already crossed the Rubicon by calling functions at all, which prevent $.param() from being used as a way to passively serialize an object the way you would with JSON. But I'm still not a fan!

@jtrumbull
Copy link
Contributor

@jtrumbull jtrumbull commented Mar 17, 2016

@dmethvin I moved this question to separate issue.

@timmywil timmywil removed this from the 3.2.0 milestone Mar 6, 2017
@timmywil timmywil added this to the 3.3.0 milestone Mar 6, 2017
@timmywil timmywil added this to the 3.3.0 milestone Mar 6, 2017
@timmywil timmywil removed this from the 3.2.0 milestone Mar 6, 2017
@timmywil timmywil self-assigned this Mar 27, 2017
@timmywil timmywil removed this from the 3.3.0 milestone Dec 18, 2017
@timmywil timmywil added this to the 3.4.0 milestone Dec 18, 2017
timmywil added a commit that referenced this issue Jun 20, 2018
Fixes gh-2633
Close gh-4108
@lock lock bot locked as resolved and limited conversation to collaborators Dec 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants