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

Ember.isEmpty({}) returns false #5543

Closed
ultimatemonty opened this issue Sep 4, 2014 · 25 comments
Closed

Ember.isEmpty({}) returns false #5543

ultimatemonty opened this issue Sep 4, 2014 · 25 comments

Comments

@ultimatemonty
Copy link

Currently Ember.isEmpty returns false if I pass in an empty object when I would expect it to return true. This feels like a gap in the functionality.

@taras suggested using Ember.keys({}).length === 0 and while this certainly works it doesn't feel intuitive when you have Ember.isEmpty available.

It's trivial to implement - I'd be glad to submit a PR if it's agreed that it is indeed a gap.

@taras
Copy link
Contributor

taras commented Sep 4, 2014

Ember.keys({}).length === 0 could be added to Em.isEmpty for testing objects.

@workmanw
Copy link

workmanw commented Sep 4, 2014

I think there is a distinction between Ember.isEmpty and Ember.isNone. If you're looking for null checking, you should use Ember.isNone.

Completely misread. Sorry about that.

@ultimatemonty
Copy link
Author

@workmanw my usecase isn't null checking. I was passing an object to store.find and had a need to remove a specific property on the object if present and then check to see if the object had other properties or if it was empty.

...

if (typeof query.procedure !== 'undefined') {
   var proc = query.procedure;
   var httpRequestType = 'GET';

   delete query.procedure;

   // check to see if there are additional query params
   // Currently Ember.isEmpty(query) will always return false even if query has no additional properties
   if (!Ember.isEmpty(query)) {
      httpRequestType = 'POST';
   }
}

...

Ember.isNone({}) returns false which is correct because the object isn't null - it's empty.

@stefanpenner
Copy link
Member

This was decided because we couldn't guarentee the same results across all our browsers. As es3 browsers have no concept of user defined non enumerable methods. At the point in time we where we drop them we could revisit this.

@ultimatemonty
Copy link
Author

thanks @stefanpenner will close for now and use @taras suggested method.

@ronco
Copy link
Contributor

ronco commented May 21, 2015

With 2.0 dropping support for IE 8 could we consider fixing this behavior to be true on {}? That would seem more consistent with the behavior for [] and 2.0 would be a good occasion to introduce an API breaking change.

@BlueRaja
Copy link

Bump, makes no sense that an empty object isn't empty.

@timini
Copy link

timini commented Jul 2, 2015

Confusing.

@stefanpenner
Copy link
Member

We can likely revisit this in 2.0 which drops support for IE8, so now there is a concept of enumerable vs non enumerable properties across all our target platforms.

In-fact aspects of ember already rely on this concept. {{each-in}

If someone is interested in championing and RFC we may be able to re-open things thought.

@BillBrower
Copy link

Was this revisited in 2.0?

@gnapse
Copy link

gnapse commented Feb 8, 2017

I don't think so. And it should have been IMO. It does not make sense that Ember.isEmpty({}) is false.

@akshayrawat
Copy link
Contributor

Also this is directly present in JQuery – https://api.jquery.com/jQuery.isEmptyObject/

@joshkg
Copy link

joshkg commented Mar 21, 2018

Seems silly to dip into jQuery for this check - can we revisit this issue now that we're 3.0?

@snewcomer
Copy link
Contributor

Anybody coming across this - looks like in a 2015 rfc, it was decided that returning true for {} would have desired unintended side effects.

If that is still the current thought process and the addon is a path forward, I would be happy to put that out there. However, it's super simple to implement ourselves, so perhaps that is why nobody has done so ¯_(ツ)_/¯

emberjs/rfcs#82

@rwjblue
Copy link
Member

rwjblue commented May 22, 2018

Almost certainly Ember.{isEmpty,isBlank,isNone,isPresent} should be deprecated. They don’t really add value over the normal JS that you would write in each case, and they generally have negative cognitive impact on the code in question...

@BlueRaja
Copy link

We've been using lodash _.isEmpty(), _.isNil(), etc. which actually work correctly

@snewcomer
Copy link
Contributor

I agree. Should I/we put together an RFC? And potentially extracted to an addon as well (for those that gratuitously used them)?

@locks
Copy link
Contributor

locks commented May 22, 2018

@snewcomer forge ahead :) emberjs/rfcs#333

@snewcomer
Copy link
Contributor

@rwjblue Getting some feedback that these methods will still be bundled in Ember core. How should we handle the deprecation side of things for these util methods?

@jfschaff
Copy link

isEmpty is not meant to check for empty objects:

https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/lib/is_empty.js#L6-L7

The function name has just been poorly chosen... IsEmpty should tell you yes when given... an empty "set"...

@gtsop
Copy link

gtsop commented Oct 31, 2019

@jfschaff @marclundgren Old comment, I referenced the description on the jsdoc that didn't mention checking for an empty object. Currently lives here:

https://github.com/emberjs/ember.js/blob/master/packages/%40ember/-internals/metal/lib/is_empty.ts#L6-L7

Not to say that I didn't expect the method to check an empty object as well, just saying that there wasn't such a claim in the first place.

(updated original comment)

@ashish2199
Copy link

Where are we on this? Will this ever be incorporated?

@locks
Copy link
Contributor

locks commented Oct 5, 2020

@ashish2199 the current proposal is to deprecate the APIs, not change them: emberjs/rfcs#334.

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

No branches or pull requests