Skip to content

Fix template for parameter types in callbacks#50

Closed
esx wants to merge 1 commit into
jquery:masterfrom
esx:master
Closed

Fix template for parameter types in callbacks#50
esx wants to merge 1 commit into
jquery:masterfrom
esx:master

Conversation

@esx
Copy link
Copy Markdown

@esx esx commented Feb 13, 2014

Support for multiple types (like 'Number or String') in callback
signatures. See for example signatures in documentation for jQuery.map.

Support for multiple types (like 'Number or String') in callback
signatures. Se for example signatures in documentation for jQuery.map.
@scottgonzalez
Copy link
Copy Markdown
Member

jQuery.map() has an incorrect signature. arrayOrObject is not a valid parameter for the signature. That signature should only be documenting objects.

@esx
Copy link
Copy Markdown
Author

esx commented Feb 17, 2014

jQuery.map() was a bad example since there was an error in how the signature was described. ".prop( propertyName, function(index, oldPropertyValue) )" is a better example, since the callback can receive multiple different types depending on the property requested. So we need to be able to show "union"-types in callback signatures, just as with ordinary parameters and return values.

@scottgonzalez
Copy link
Copy Markdown
Member

I don't see how listing every single data type provides any benefit. We should use a meta type like Mixed or Any.

@esx
Copy link
Copy Markdown
Author

esx commented Feb 17, 2014

Do you mean in the general case, or just in the case of "prop()"? I think I agree with you in the case of "prop()", since properties can be many different types (e.g. "ownerDocument" is a Document for example, so at least four different types.). But this is really an issue regarding the documentation of specific methods. This commit is just a general xslt fix so that multi-types is rendered the same way in callback signatures as in other parameters.

@scottgonzalez
Copy link
Copy Markdown
Member

I refuse to land a change without a specific example of where the functionality is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants