Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Concerns regarding lang's internal isObject function #45

Closed
kfranqueiro opened this issue Jun 18, 2015 · 4 comments
Closed

Concerns regarding lang's internal isObject function #45

kfranqueiro opened this issue Jun 18, 2015 · 4 comments
Assignees
Milestone

Comments

@kfranqueiro
Copy link
Member

This function is used to determine when something should be copied/assigned recursively. It currently returns true for things that report typeof as object that are not Arrays or RegExps. But I would think there are other things (e.g. Date) that should also return false here.

@mwistrand suggested Object.prototype.toString.call(item) === '[object Object]' as a possible alternative that would avoid this slippery slope. Can anyone think of a reason that would be undesirable?

@kitsonk
Copy link
Member

kitsonk commented Jun 18, 2015

I tried to "break" Object.prototype.toString.call(item) === '[object Object]' and it seems to behave correctly, including Iterators and Symbols.

@mwistrand
Copy link

Thanks for thinking to test Symbols; I neglected those. As far as I can tell, this rather ugly method is the most reliable means of correctly determining whether something is a POJO.

@kfranqueiro
Copy link
Member Author

It's way less ugly than the song-that-never-ends that the current code would be tending towards :P and I've seen it elsewhere, I just always forget about it.

@bryanforbes
Copy link
Member

I'm more concerned that @mwistrand used "POJO" than I am with using this method...

@eheasley eheasley modified the milestones: Milestone 1, Milestone 2 Jun 24, 2015
jdonaghue added a commit to jdonaghue/core that referenced this issue Jul 15, 2015
kitsonk pushed a commit that referenced this issue Jan 17, 2016
msssk added a commit to msssk/core that referenced this issue Jan 22, 2016
Also updates unit test to include testing Date and RegExp objects as properties

Related to dojo#45, fixes dojo#62
kitsonk pushed a commit that referenced this issue Feb 14, 2016
Also updates unit test to include testing Date and RegExp objects as properties

Related to #45
Fixes #62
Closes #110
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants