Added structured types for higer order function parameters #419

esx · 2014-01-18T11:48:46Z

Some function parameters of function type (e.g. handlers, callbacks) was
described with the signature in the name-attribute, eg. . This have been modified
to used nested argument-elements to describe the signatures more
consistently.

Some function parameters of function type (e.g. handlers, callbacks) was described with the signature in the name-attribute, eg. <argument name="function(index, value)" type="Function">. This have been modified to used nested <argument>-elements to describe the signatures more consistently.

Some callback functions was indicated to receive XMLHttpRequest rather than jsXHR.

The ajaxOptions/ajaxSettings options object is not documented as a seperate type, so now changed to PlainObject. Changed misspelling of jsXHR to jqXHR.

Argument names with whitespace like "jQuery object" are confusing, because it might easily be mistaken for two arguments when reading the signature. Argument names like "-index" are also confusing since they look like an expression rather than a name. I have renamed the argument names to be valid javascript identifiers, just like actual argument names have to be. Also changed argument type "Object" and "PlainObject" to "JQuery" where the prose documentation indicates that the type is a jQuery object.

tjvantoll · 2014-01-20T22:06:13Z

I took a brief look at this and I like what I see; it's nice to have these functions documented better. I'll take a more thorough look at the individual methods later this week.

I wonder if users get that the "=>" means "which should return a", for example Function( Integer index, String currentClass ) => String in addClass():

This is in the xsl and not your changes, but I'm wondering if we can come up with a better convention before using it across all of jQuery core's docs.

The text below the signature explains everything, so maybe this isn't a big deal; but I was confused by the arrow at first.

Of course, I bring this up but I don't have a better idea 😄

AurelioDeRosa · 2014-01-20T22:08:53Z

@tjvantoll Isn't colon the most used way to specify the return type of a function? UML uses this convention as well. What do you think?

mgol · 2014-01-20T22:10:21Z

=> resonates well with arrow functions. ;) Though that's new enough devs might not be familiar.

tjvantoll · 2014-01-21T15:22:14Z

Personally I don't really like using a colon or a fat arrow. I prefer being wordy, but given the space here, I don't think that's an option. I was mostly wondering whether anyone had a strong opinion here, otherwise we can just leave it as is.

esx · 2014-01-21T17:58:24Z

I don't have a strong opinion either way, but I prefer fat arrow, since I think the meaning is more obvious in the context.

esx · 2014-01-21T18:02:06Z

If we used colon for return value, we should probably be consistent and also use it in arguments also ("name : Type"), as opposed to the Java-like syntax ("Type name") we have now. But either way I think fat arrow is a clearer way to indicate return value.

scottgonzalez · 2014-01-21T18:07:33Z

If I remember correctly, we did the colon typing for arguments and changed it because people found it confusing. @jzaefferer do you recall that?

As per discussion on #jquery-content. Also changed type from 'Object' to 'jQuery' where prose indicates a jQuery object.

tjvantoll · 2014-01-29T13:27:33Z

entries/add.xml

@@ -21,7 +21,7 @@
  </signature>
  <signature>
    <added>1.3.2</added>
-    <argument name="jQuery object" type="jQuery object ">
+    <argument name="jQueryObject" type="jQuery">


Maybe I'm missing something, but I'm not seeing anything effect of this change.

tjvantoll · 2014-01-29T14:22:52Z

@esx Thanks a lot for putting this together. I found a few things but they're all pretty minor. Let me know when you've had a chance to go through this and we can move forward with this.

Some fixes to add missing parameters and naming consistency for callback signatures, based on code review by tjvantoll.

Changed for consistency

esx · 2014-02-13T16:34:47Z

Hi, I fixed the issues you pointed out, except the xslt issue which is in a different project. scottgonzalez explained why I changed the parameter names to valid identifiers: It looks really confusing in the signature documentation when the parameter names contain whitespace or otherwise are invalid identifiers.

tjvantoll · 2014-02-13T23:50:07Z

@esx It looks like you still need to add arguments for append(), before(), filter(), and not().

esx · 2014-02-15T18:12:23Z

Hi tjvantoll, in commit 6aa5119 I added the extra arguments you pointed out was missing. Or do I misunderstand you?

tjvantoll · 2014-02-17T13:44:16Z

@esx Ah ok, sorry about that. I think we're good to go other than the discussion of how to handle callback arguments that can be multiple types.

The second overload was a bit confusingly specified.

tjvantoll · 2014-02-17T18:41:16Z

@esx I think we're good other than the change you have for planned for grunt-jquery-content. When you have a pull request for that repo ready please list it here. Thanks.

esx · 2014-02-17T19:12:56Z

Related pull request for grunt-jquery-content:
jquery/grunt-jquery-content#50

esx · 2014-05-16T12:25:10Z

Hi, I would like to understand the state of this pull request. Has it been rejected, or are there some outstanding questions that need to be resolved, or something I need to do?

tjvantoll · 2014-05-16T12:52:04Z

Apologies. It seems this got lost when the conversation moved to this PR. Your last comment was this:

How to display "option"-types in callback signatures. A bug in the XSLT meant that no type was displayed at all. I have fixed this in the grunt-jquery-content project and sent a pull request.

You fixed the issue in $.map with your last commit. Are there any other signature in this PR where that comment applies? If not I think we're good to go.

tjvantoll · 2014-05-16T13:06:30Z

I looked through the changes and I found two situations where this comes up:

$.each()—Shouldn't this only list Integer?
prop()—Per the conversation in Fix template for parameter types in callbacks grunt-jquery-content#50, should we use a meta type of some sort here?

scottgonzalez · 2014-05-16T13:16:32Z

$.each() should have two signatures, one for object, one for array.

.prop() should probably use Anything (see #391).

tjvantoll · 2014-05-16T13:19:30Z

+1 for Anything on prop(). @esx can you make those two changes? If so I can land this. Everything else looks good.

esx · 2014-05-16T17:20:41Z

Done.

tjvantoll · 2014-05-16T17:56:08Z

For prop() I was specifically referring to this <argument> as that's what you changed. If we're going to change prop() it seems like we need a more thorough pass that is handled by a separate issue.

I'll clean this up and land it.

tjvantoll · 2014-05-16T20:48:07Z

@esx Thanks a lot for making these changes and seeing them through. It's much appreciated.

tjvantoll · 2014-05-16T20:50:39Z

I created this issue for using Anything in prop().

esx added 5 commits January 18, 2014 12:44

Changed some argument type from XMLHttpRequest to jsXHR

d8093a5

Some callback functions was indicated to receive XMLHttpRequest rather than jsXHR.

Fixed spelling and changed 'ajaxOptions' to 'PlainObject'

0d7d9b3

The ajaxOptions/ajaxSettings options object is not documented as a seperate type, so now changed to PlainObject. Changed misspelling of jsXHR to jqXHR.

Fix spelling in type attribute

7455ae5

Changed argument name 'jQueryObject' to 'selection'

41ce261

As per discussion on #jquery-content. Also changed type from 'Object' to 'jQuery' where prose indicates a jQuery object.

tjvantoll reviewed Jan 29, 2014
View reviewed changes

esx added 2 commits February 13, 2014 17:13

Some fixies in signatures

6aa5119

Some fixes to add missing parameters and naming consistency for callback signatures, based on code review by tjvantoll.

Name fix

9d4aa93

Changed for consistency

Clarify jQuery.map

49a296c

The second overload was a bit confusingly specified.

rwaldron added the help wanted label May 15, 2014

Signature fixes for $.each() and prop()

40db666

tjvantoll closed this in 11c93b9 May 16, 2014

tjvantoll mentioned this pull request May 16, 2014

prop() should use the Anything type #496

Closed

Added structured types for higer order function parameters #419

Added structured types for higer order function parameters #419

Uh oh!

Conversation

esx commented Jan 18, 2014

Uh oh!

tjvantoll commented Jan 20, 2014

Uh oh!

AurelioDeRosa commented Jan 20, 2014

Uh oh!

mgol commented Jan 20, 2014

Uh oh!

tjvantoll commented Jan 21, 2014

Uh oh!

esx commented Jan 21, 2014

Uh oh!

esx commented Jan 21, 2014

Uh oh!

scottgonzalez commented Jan 21, 2014

Uh oh!

tjvantoll Jan 29, 2014

Choose a reason for hiding this comment

Uh oh!

tjvantoll commented Jan 29, 2014

Uh oh!

esx commented Feb 13, 2014

Uh oh!

tjvantoll commented Feb 13, 2014

Uh oh!

esx commented Feb 15, 2014

Uh oh!

tjvantoll commented Feb 17, 2014

Uh oh!

tjvantoll commented Feb 17, 2014

Uh oh!

esx commented Feb 17, 2014

Uh oh!

esx commented May 16, 2014

Uh oh!

tjvantoll commented May 16, 2014

Uh oh!

tjvantoll commented May 16, 2014

Uh oh!

scottgonzalez commented May 16, 2014

Uh oh!

tjvantoll commented May 16, 2014

Uh oh!

esx commented May 16, 2014

Uh oh!

tjvantoll commented May 16, 2014

Uh oh!

tjvantoll commented May 16, 2014

Uh oh!

tjvantoll commented May 16, 2014

Uh oh!

Uh oh!