Skip to content

v2.1.4 isArrayLike empty obj error #2242

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.

Sign up for GitHub

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

Jump to bottom
Closed
newenegue opened this issue Apr 29, 2015 · 18 comments
Closed

v2.1.4 isArrayLike empty obj error #2242

newenegue opened this issue Apr 29, 2015 · 18 comments

Comments

@newenegue
Copy link

I received this error
Uncaught TypeError: Cannot use 'in' operator to search for 'length' in

When trying to call $.map like so:
$.map("".match( /"[^"]+"|[^ ]+/g ) || "", function(word) {console.log(word);});

It seems that "length" in obj returns the error when the obj == "", but it works fine when obj == [""]

I just want to check that passing in the obj == [""] should be what I do moving forward. Thanks!

function isArraylike( obj ) {

    // Support: iOS 8.2 (not reproducible in simulator)
    // `in` check used to prevent JIT error (gh-2145)
    // hasOwn isn't used here due to false negatives
    // regarding Nodelist length in IE
    var length = "length" in obj && obj.length,
        type = jQuery.type( obj );

    if ( type === "function" || jQuery.isWindow( obj ) ) {
        return false;
    }

    if ( obj.nodeType === 1 && length ) {
        return true;
    }

    return type === "array" || length === 0 ||
        typeof length === "number" && length > 0 && ( length - 1 ) in obj;
}
@dmethvin
Copy link
Member

The documentation for jQuery.map doesn't say the method takes a string, but you're passing one in the case where the .match() method returns null. if you change the || "" to || [ "" ] it will pass in the correct type in all cases.

@Drarok
Copy link

Drarok commented Apr 29, 2015

This change also breaks DataTables as it has the following code:

$.map( search.match( /"[^"]+"|[^ ]+/g ) || '', function ( word ) {

@dmethvin
Copy link
Member

That should be fixed in the plugin, since $.map never supported strings.

@Drarok
Copy link

Drarok commented Apr 29, 2015

I don't disagree that the plugin should fix their incorrect usage, but should a minor point release of jQuery introduce such a backwards-incompatible change? Seeing as it breaks a very popular plugin, and countless other codebases that rely on the previous behaviour…

@mgol
Copy link
Member

mgol commented Apr 29, 2015

@Drarok Technically speaking, semver only describes the relation between a public API and versions and here we're talking about undocumented behavior therefore semver doesn't apply.

Unfortunately, such problems are unavoidable. People don't remember all possible method signatures and don't check the docs on every use which leads to sometimes relying on undocumented behavior. Any change we make, even in patch releases may break some of those invalid use cases, that's why we always say semver is just a hint but you should test your projects on every library upgrade, even if you just upgrade to a newer patch release.

Thus, the question is not if we broke some plugin that uses undocumented APIs incorrectly (I'd guess we almost always break something) but how much impact such breakage may have, e.g. how popular is a plugin etc. If anyone reported the issue with the patch in PR #2185 we'd most likely update the patch to avoid this issue but it's done, we've released 2.1.4 & 1.11.3 and they cannot be re-released. To make newest jQuery & DataTables work, there are only two options:

  1. Work around it in jQuery & release new jQuery versions, 2.1.5 & 1.11.4.
  2. Fix it in DataTables & release a new DataTables version.

Whatever happens, you'll need to update one of the libraries anyway to make it work. In this case I'd rather DataTables fixed their incorrect use of the jQuery API. Did you report this issue to them? If they have a public bug tracker, could you post a link here?

I'd say we should work around it & release a new patch update only if a lot of popular plugins relied on this behavior. So far we only know about one and they should IMO just fix it on their side.

@timmywil
Copy link
Member

Well said, @mzgol. There would have to be quite an uproar for us to do another patch release to support undocumented behavior.

@mgol mgol mentioned this issue Apr 29, 2015
Closed
@mgol
Copy link
Member

mgol commented Apr 29, 2015

I posted about the problem on their forums:
https://www.datatables.net/forums/discussion/27524/broken-in-jquery-2-1-4-1-11-3
From what I see, this is mostly a paid product which reinforces my view that they should fix it.

@DataTables
Copy link

The fix has been committed to DataTables now and its nightly version is up to date with the change.

Passing a string is was a bug that I didn't pick up before as it "just worked" in jQuery before so I didn't notice it! I completely concur that no change should be required in jQuery!

From what I see, this is mostly a paid product which reinforces my view that they should fix it.

DataTables is MIT licensed :-)

@mgol
Copy link
Member

mgol commented Apr 29, 2015

DataTables is MIT licensed :-)

Sorry, my quick glance at the site was not enough apparently. :)

Passing a string is was a bug that I didn't pick up before as it "just worked" in jQuery so I didn't notice it!

I fully understand, such issues are unavoidable, unfortunately.

Thanks for a quick reaction!

@markelog
Copy link
Member

Hm, should we add this behaviour to the migrate?

@mgol
Copy link
Member

mgol commented Apr 29, 2015

Hm, should we add this behaviour to the migrate?

Seems like a good idea.

@gibson042
Copy link
Member

I would say no. We only care about changes to documented behavior.

@markelog
Copy link
Member

@gibson042 we did it a lot for undocumented behaviour and undocumented methods, for example - https://github.com/jquery/jquery-migrate/blob/920afc6637645629e8cc22c9b76cd2fdc59e0d96/src/manipulation.js

@markelog markelog mentioned this issue Apr 30, 2015
Closed
@dmethvin dmethvin mentioned this issue May 6, 2015
Closed
@dstull dstull mentioned this issue Sep 5, 2015
Closed
@uptoeleven uptoeleven mentioned this issue Sep 21, 2015
Closed
@Ustes
Copy link

Ustes commented Sep 25, 2015

Attempting to use DataTables in a project and I am still receiving this error. I have attached my plunker,
http://plnkr.co/edit/bJ0RQQpcnX8STsw626mX?p=preview.

When you change the jquery version to 1.8.3 in the cdn link in the html all works well, if you leave as 2.1.4 and view the developer tools you see the following:
TypeError: Cannot use 'in' operator to search for 'length' in Category

@dmethvin
Copy link
Member

The first Google result for "jquery isarraylike datatables" is this ticket:

https://github.com/DataTables/DataTables/issues/546

Have you updated your DataTables plugin?

@Ustes
Copy link

Ustes commented Sep 25, 2015

yes, I have even tried using the nightlies in the cdn.

@dmethvin
Copy link
Member

Then you should report the problem to the DataTables team.

@Ustes
Copy link

Ustes commented Sep 25, 2015

Sorry, thought this was the DataTables issue. Posted in wrong tab.

@mrgreen222 mrgreen222 mentioned this issue Nov 27, 2015
Closed
@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@Drarok @dmethvin @timmywil @markelog @gibson042 @mgol @Ustes @newenegue