renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
16.85k stars 2.2k forks source link

Refactor away from `is.plainObject` #22666

Open JamieMagee opened 1 year ago

JamieMagee commented 1 year ago

Describe the proposed change(s).

We currently use the @sindresorhus/is library to simplify a lot of type checking throughout Renovate. During a refactor from our own clone function to structuredClone in #20885 we saw some issues with some of the limitations of structuredClone. Namely, the prototype chain is not preserved.

After the PR was merged another issue emerged, #22058. The root cause of this is the implementation of is.plainObject is not compatible with use of structuredClone:

is.plainObject = <Value = unknown>(value: unknown): value is Record<PropertyKey, Value> => {
    // From: https://github.com/sindresorhus/is-plain-obj/blob/main/index.js
    if (typeof value !== 'object' || value === null) {
        return false;
    }

    // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
    const prototype = Object.getPrototypeOf(value);

    return (prototype === null || prototype === Object.prototype || Object.getPrototypeOf(prototype) === null) && !(Symbol.toStringTag in value) && !(Symbol.iterator in value);
};

The prototype of the object being checked is compared with the prototype of the Object type. This will always return false with objects that are cloned with structuredClone.

I don't believe this is a bug with @sindresorhus/is, as it's explicitly called out in their documentation:

An object is plain if it's created by either {}, new Object(), or Object.create(null).

Therefore, if we want to continue using structuredClone (we do! See #20553) we need to migrate away from is.plainObject to something else.

The naive choice is is.object. However, the implementation is not exactly what

is.object = (value: unknown): value is object => !is.null_(value) && (typeof value === 'object' || is.function_(value));

And the documentation confirms this:

Keep in mind that functions are objects too.

That only really leaves is.nonEmptyObject (and its counterpart is.emptyObject). Its implementation is:

is.nonEmptyObject = <Key extends keyof any = string, Value = unknown>(value: unknown): value is Record<Key, Value> => is.object(value) && !is.map(value) && !is.set(value) && Object.keys(value).length > 0;

I think this is currently the best path forward, though I've also opened https://github.com/sindresorhus/is/issues/183 to get feedback from upstream

Describe why we need/want these change(s).

is.plainObject is incompatible with structuredClone

JamieMagee commented 1 year ago

After some more investigation, I think this is caused by us using @sindresorhus/is 4.x. The implementation of is.plainObject used to be:

is.plainObject = <Value = unknown>(value: unknown): value is Record<PropertyKey, Value> => {
    // From: https://github.com/sindresorhus/is-plain-obj/blob/main/index.js
    if (toString.call(value) !== '[object Object]') {
        return false;
    }

    // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
    const prototype = Object.getPrototypeOf(value);

    return prototype === null || prototype === Object.getPrototypeOf({});
};

Specifically

return prototype === null || prototype === Object.getPrototypeOf({})

is causing the issue. In 5.2.0, this was changed to

return (prototype === null || prototype === Object.prototype || Object.getPrototypeOf(prototype) === null) && !(Symbol.toStringTag in value) && !(Symbol.iterator in value);

I'll try and resolve the specific case of this bug in #22058, but this will ultimately be resolved by #9890