-
Notifications
You must be signed in to change notification settings - Fork 264
jQuery.get:jQuery.post: Document issues with data: null with 3 params
#1208
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
base: main
Are you sure you want to change the base?
Conversation
| @@ -15,7 +15,7 @@ | |||
| <argument name="data" type="PlainObject" /> | |||
| <argument name="textStatus" type="String"/> | |||
| <argument name="jqXHR" type="jqXHR"/> | |||
| <desc>A callback function that is executed if the request succeeds. Required if <code>dataType</code> is provided, but you can use <code>null</code> or <a href="/jQuery.noop/"><code>jQuery.noop</code></a> as a placeholder.</desc> | |||
| <desc>A callback function that is executed if the request succeeds. Required if <code>dataType</code> is provided, but you can use <code>null</code> or <a href="/jQuery.noop/"><code>jQuery.noop</code></a> as a placeholder. <strong>NOTE:</strong> In jQuery 3.x and older, when providing a <code>null</code> value for <code>success</code> you also have to provide the <code>data</code> parameter; you can set it to <code>undefined</code>.</desc> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can set it to undefined or null right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically - probably yes - but I'd rather not advertise that. For example, in the success case, you can set it to null but not to undefined to trigger the documented behavior, which makes us treat the undefined value more as "parameter not passed" and null having some extra meanings in some cases.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk, it seems inconsistent to recommend null in one place and undefined in another. I'd probably use null, null, dataType myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to document null for the second parameter as well, we'd have to have some tests in jQuery for this. Currently, I believe we have none.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these tests in jquery/jquery#5640. When merged & backported to 3.x-stable, I'll update this PR as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two more PRs to merge before I return to this API PR:
4ee332d to
03bdec9
Compare
03bdec9 to
66abf44
Compare
66abf44 to
d8fa4f5
Compare
d8fa4f5 to
5e558e0
Compare
Also, fix `mock.php` formatting to not fail the `jQuery.get( String, null, String )` test in PHP mode. Ref jquerygh-4989 Ref jquery/api.jquery.com#1208
5e558e0 to
05cab6f
Compare
Also, fix `mock.php` formatting to not fail the `jQuery.get( String, null, String )` test in PHP mode. Closes gh-5640 Ref gh-4989 Ref jquery/api.jquery.com#1208
Also, fix `mock.php` formatting to not fail the `jQuery.get( String, null, String )` test in PHP mode. Ref jquerygh-5640 Ref jquerygh-4989 Ref jquery/api.jquery.com#1208
Also, fix `mock.php` formatting to not fail the `jQuery.get( String, null, String )` test in PHP mode. Ref jquerygh-5640 Ref jquerygh-4989 Ref jquery/api.jquery.com#1208
05cab6f to
3c1c919
Compare
In jQuery 3.x and older, when providing a `null` value for `success` you also have to provide the `data` parameter; you can set it to `undefined`. Document this restriction of `jQuery.get` & `jQuery.post`. Ref jquery/jquery#4989 Ref jquery/jquery#5139
3c1c919 to
eda8b21
Compare
In jQuery 3.x and older, when providing a
nullvalue forsuccessyou also have to provide thedataparameter; you can set it toundefined.Document this restriction of
jQuery.get&jQuery.post.Ref jquery/jquery#4989
Ref jquery/jquery#5139