Eslint-plugin-jsdoc: `check‑types`: Allow lower‑case `object`

[ExE-Boss]

TypeScript recommends object over Object, which is enforced by my ESLint config for the deprecated valid‑jsdoc rule, and would require changes to a lot of my code.

[brettz9]

If the preferType option from valid-jsdoc is carried over from ESLint (#107), one could define this rule for oneself (unless you also wanted to change the current behavior of check-types).

[brettz9]

Closed by #247. See https://github.com/gajus/eslint-plugin-jsdoc/tree/master/.README#settings-to-configure-check-types-and-no-undefined-types for use with check-types.

You can get this behavior now (in master) by:

{
  settings: {
    jsdoc: {
      preferredTypes: {
        'Object': 'object'
     }
  }
}

However, I've asked @gajus if we might change the default, as I agree it makes more sense given that objects made with Object.create(null) are not instanceof Object and as mentioned, Typescript recommends "object".

[gajus]

However, I've asked @gajus if we might change the default, as I agree it makes more sense given that objects made with Object.create(null) are not instanceof Object and as mentioned, Typescript recommends "object".

Go for it.

When merging, the commit message must include "BREAKING CHANGE:" and a description of the breaking change. This will cause new major version release. See https://github.com/semantic-release/semantic-release#commit-message-format

[gajus]

:tada: This issue has been resolved in version 6.0.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[dietergeerts]

So because TS is using object, the default is now object? What happened to the reasoning behind having it Object in the first place? I liked the reasoning because it made sense towards the JS language....

[brettz9]

Yes, in part because of TS which has become very popular (whether for compiling into JavaScript or using the subset of TS which actually is valid JS). But there was a case missed by the original logic. Objects made with Object.create(null) (or on which Object.setPrototypeOf(obj, null); has been called or the old form obj.__proto__ = null;) are not instanceof Object. So if a function accepts arbitrary objects, they may indeed not all be instances of Object.

[dietergeerts]

Who would ever do that? I mean yes, technically it's possible, but why?

[brettz9]

Using Object.create(null) allows you to create objects which aren't at risk for inheriting from Object.prototype and for which for...in guards are not needed.

So if some code library adds this to a project you or your users are using:

/**
* @example
* ({a: 1, b: 2}).getKeys()
*/
Object.prototype.getKeys = () => {
    return 'Object ' + Object.keys(this).join(', ');
};

...your code can confidently do this without fear of logging/acting on items like getKeys which have elsewhere been added to Object.prototype:

const map = Object.create(null);
map.someKey = 5;
for (const key in map) {
  console.log(key); // Logs `someKey` but not `getKeys`
}

whereas this approach will show the inherited getKeys as well:

const map = {};
map.someKey = 5;
for (const key in map) {
  console.log(key); // Logs `getKeys` as well as `someKey`
}

For the latter, you have to manually do a hasOwnProperty check to make sure you are not getting getKeys.

[dietergeerts]

Thx for the explanation, that makes it all clear.

This shows what we know as 'prototype pollution', and must be banned Imho. No library should ever need to do this. That's why FP is way better in this regard.

I also think that this method of creating objects is not well known, and one could ask if it should be needed.

[brettz9]

Yes, extending the prototype is generally not a good practice, though it is useful for allowing polyfills. It can make for an attractive API, but I agree the cost isn't worth it.