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

Deprecate jQuery.isArray #2961

Closed
markelog opened this issue Feb 29, 2016 · 24 comments
Closed

Deprecate jQuery.isArray #2961

markelog opened this issue Feb 29, 2016 · 24 comments
Milestone

Comments

@markelog
Copy link
Member

@markelog markelog commented Feb 29, 2016

Seems fulfilled with Array.isArray

@markelog markelog added this to the 3.1.0 milestone Feb 29, 2016
@mgol
Copy link
Member

@mgol mgol commented Feb 29, 2016

👍

@markelog markelog added the Core label Mar 3, 2016
@tkdphoenix
Copy link

@tkdphoenix tkdphoenix commented Mar 16, 2016

agreed

@FarSeeing
Copy link
Contributor

@FarSeeing FarSeeing commented Apr 18, 2016

I guess it's worth moving Array.isArray into /var and re-use:

   raw     gz Compared to master @ 44cb97e0cfc8d3e62bef7c621bfeba6fe4f65d7c    
  +601   +147 dist/jquery.js                                                   
   -60    -12 dist/jquery.min.js
@markelog
Copy link
Member Author

@markelog markelog commented Apr 18, 2016

Nice find, but with this ticket we would need to move it to deprecated.js module

@FarSeeing
Copy link
Contributor

@FarSeeing FarSeeing commented Apr 18, 2016

That's a calculation with the following code in deprecated.js:

jQuery.extend( {
    isArray: isArray
} );
@markelog
Copy link
Member Author

@markelog markelog commented Apr 18, 2016

You mean gzip size will also change in identical manner if we move it to deprecated.js as if we moved it to /var?

@FarSeeing
Copy link
Contributor

@FarSeeing FarSeeing commented Apr 18, 2016

That's the result of having both /var file creation for storing variable and moving declaration to deprecated.js.

@mgol
Copy link
Member

@mgol mgol commented Apr 18, 2016

Deprecation would mean that we:

  1. Move jQuery.isArray to deprecated.js
  2. Change all internal uses from jQuery.isArray to Array.isArray

so we can't have the var-module containing the old code here.

@FarSeeing
Copy link
Contributor

@FarSeeing FarSeeing commented Apr 18, 2016

Right, but as other built-in methods (slice, concat, toString etc) isArray could be stored in /var.

@mgol
Copy link
Member

@mgol mgol commented Apr 18, 2016

So you propose to put Array.isArray in a var-module, not jQuery.isArray? I understood it differently before.

@FarSeeing
Copy link
Contributor

@FarSeeing FarSeeing commented Apr 18, 2016

That's right. Sorry for not explaining that before.

@markelog
Copy link
Member Author

@markelog markelog commented Apr 18, 2016

I wouldn't want size to influence our file hierarchy, it would be pretty confusing, also all deprecated modules should be stored in deprecated module, since it would also be confusing but now for users too who want to exclude or use deprecated stuff

@mgol
Copy link
Member

@mgol mgol commented Apr 18, 2016

@markelog I agree to an extent but @FarSeeing only proposes to put Array.isArray in a var-module, not the soon-to-be-deprecated jQuery.isArray; see our last 2 comments.

@markelog
Copy link
Member Author

@markelog markelog commented Apr 18, 2016

Oh, i see, but i wouldn't do that too :), don't really see the point

@ShashankaNataraj
Copy link
Contributor

@ShashankaNataraj ShashankaNataraj commented Apr 24, 2016

Guys, is there a possibility of me submitting a PR for this?

@mgol
Copy link
Member

@mgol mgol commented Apr 24, 2016

@ShashankaNataraj Sure! Do you know what needs to be done?

@markelog
Copy link
Member Author

@markelog markelog commented Apr 24, 2016

I would wait until we would have discussion about this, it seems some team members feel "deprecation" process of core methods might be inappropriate.

@ShashankaNataraj
Copy link
Contributor

@ShashankaNataraj ShashankaNataraj commented Jul 11, 2016

@mgol @markelog Correct me if Im wrong, tasks for this issue are:

  • Move jquery.isArray to deprecated.js
  • Change all usages of jQuery.isArray to Array.isArray
@mgol
Copy link
Member

@mgol mgol commented Jul 11, 2016

Also, the third item - its unit tests have to be moved to jQuery.isArray as
well.

Also, if this hasn't been mentioned before - each deprecation, apart from a
jQuery PR and an API site issue needs a jQuery Migrate issue to polyfill
the API and warn against using it.

Michał Gołębiowski

@ShashankaNataraj
Copy link
Contributor

@ShashankaNataraj ShashankaNataraj commented Jul 11, 2016

@mgol Didnt get what you meant by

its unit tests have to be moved to jQuery.isArray as well.

I understand unit tests need to be modified but jQuery.isArray tests need to be removed right?

Actually I have been raising issues like this for every deprecation issue Im working on..

Raised one for this issue too.

@mgol
Copy link
Member

@mgol mgol commented Jul 14, 2016

I understand unit tests need to be modified but jQuery.isArray tests need to be removed right?

Not removed but moved to test/unit/deprecated.js.

Actually I have been raising issues like this for every deprecation issue Im working on..

I've seen them, thanks for that! I meant that apart from the API issue we also need an issue at https://github.com/jquery/jquery-migrate/issues about a need to restore the API (because people may exclude the deprecated module) with a warning against using it.

@kumarmj
Copy link
Contributor

@kumarmj kumarmj commented Aug 14, 2016

@ShashankaNataraj seems not to be working on this. Can I please take this ?

@kumarmj
Copy link
Contributor

@kumarmj kumarmj commented Aug 14, 2016

Correct me, for any changes

  • Move jquery.isArray to deprecated.js
  • Change all usages of jQuery.isArray to Array.isArray
  • Modify unit tests
  • Create a jquery-migrate issue for jQuery.isArray
@Rikk
Copy link

@Rikk Rikk commented Dec 11, 2017

preetpalS added a commit to preetpalS/jQuery-Autocomplete that referenced this issue Jan 28, 2018
`$.isArray` is deprecated in jQuery 3.2, so Array.isArray should be the way to go.

jquery/jquery#2961
https://blog.jquery.com/2017/03/16/jquery-3-2-0-is-out/
@lock lock bot locked as resolved and limited conversation to collaborators Jun 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

8 participants
You can’t perform that action at this time.