mesqueeb / is-what

JS type check (TypeScript supported) functions like `isPlainObject() isArray()` etc. A simple & small integration.
https://mesqueeb.github.io/is-what/
MIT License
169 stars 18 forks source link

Correctly handle null prototypes #32

Closed msealand closed 1 year ago

msealand commented 1 year ago

This fixes an error thrown when using is-what on an object with a null prototype (i.e. an object created via Object.create(null)).

Before:

> var iw = require('./dist/index.cjs');
> iw.isObject(Object.create(null));
Uncaught TypeError: Cannot read properties of null (reading 'constructor')
    at isPlainObject (/Users/mspoly/Documents/Development/3rdParty/is-what/dist/index.cjs:40:22)
    at Object.isObject (/Users/mspoly/Documents/Development/3rdParty/is-what/dist/index.cjs:49:12)

After:

> var iw = require('./dist/index.cjs');
> iw.isObject(Object.create(null));
true
> iw.isFullObject(Object.create(null));
false
> iw.isPlainObject(Object.create(null));
true
mesqueeb commented 1 year ago

@msealand i'm open to merging but what's the use case of creating objects like this ?

msealand commented 1 year ago

@mesqueeb It's just a method of creating a valid javascript object that doesn't have a prototype attached. The created object otherwise works as you would expect:

Welcome to Node.js v19.7.0.
Type ".help" for more information.
> var obj = Object.create(null);
undefined
> obj.a = 1;
1
> obj
[Object: null prototype] { a: 1 }
> Object.getPrototypeOf(obj);
null

I don't actually use it much personally, so I'm hesitant to say what the typical use case is. I suspect it's just to get an even lighter weight object. It is used quite a bit in the javascript ecosystem: https://github.com/search?q=Object.create%28null%29+extension%3Ats+extension%3Ajs&type=Code&ref=advsearch&l=&l=. I found the issue with is-what when trying to use it on objects coming from Koa.

jcbhmr commented 1 year ago

what's the use case of creating objects like this ?

@mesqueeb Use case is usually a Record<string, SomeThing> so that setting myMap[userInput] = anything doesn't have the ability to set __proto__ or .constructor or any other "special" keys. For instance in @msealand's case it was a Koa object that probably had a bunch of user-specific query params or something like ?name=Tim&age=34 into [null prototype] { name: "Tim", age: "34" } which is good! That means sending ?__proto__=hi won't cause the object to magically become a String. It's just [null prototype] { __proto__: "hi" } instead of [String(hi)] { } (instanceof String is true).

mesqueeb commented 1 year ago

@msealand sorry for taking so long. But i've decided the following:

your proposal

isPlainObject(Object.create(null)) // true
isAnyObject(Object.create(null)) // true

my decision

isPlainObject(Object.create(null)) // false
isAnyObject(Object.create(null)) // true

I made it so because I believe an object without any prototype should not be regarded as a "plain object".

However, I did made sure the isPlainObject / isAnyObject does not crash anymore thanks to this PR!

If you want to keep using the Object.create(null) method of creating objects, then please just use isAnyObject to make your checks : )

If you really want we can also add a new isPrototypelessObject function, which returns true if it's any object without a prototype.

msealand commented 1 year ago

Thanks @mesqueeb, that works for me! And no worries about the timing; we're all busy 🙂. Also, thanks @jcbhmr for explaining the use case!