-
Notifications
You must be signed in to change notification settings - Fork 265
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
jQuery.get:jQuery.post: Document issues with data: null
with 3 params
#1208
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
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
05cab6f
to
3c1c919
Compare
In jQuery 3.x and older, when providing a
null
value forsuccess
you also have to provide thedata
parameter; you can set it toundefined
.Document this restriction of
jQuery.get
&jQuery.post
.Ref jquery/jquery#4989
Ref jquery/jquery#5139