Use Object.setPrototypeOf as __proto__ could potentially be disabled

[bmeck]

Due to CVEs and accidents, Node is thinking of allowing disabling Object.prototype.__proto__. This PR achieves the same functionality using Object.setPrototypeOf which does not suffer the same issues and removes a lint error.

See: nodejs/node#31951

[ljharb]

Note that this is a breaking change; as there are many old browsers that support __proto__ but not Object.setPrototypeOf, including all IE.

It would be better to use something like the setprototypeof package, so it uses the newest technique available (which would also still work in the unlikely event of node removing dunder proto)

[sindresorhus]

@ljharb Chalk is not a browser package.

[ljharb]

lol fair, but s/browser/engine. node 0.10 and below don't have Object.setPrototypeOf, which still makes this PR a breaking change - using the setprototypeof package or similar would avoid that.

[bmeck]

If it is considered breaking, that seems fine to neutral from my perspective, but I would prefer not to use the package which would add a dependency and have questionable gains. The last release in question on that node release line is from 2016 and has multiple known security issues.

[ljharb]

The gain is that by not being a breaking change, the fix you want will filter out to the maximum number of users quickly. You could certainly after that publish a major version that drops the dependency if you wanted.

[bmeck]

@ljharb I'd personally prefer the major over using the dependency given supply chain concerns and lack of clear reasons to continue support for engines with known major issues.

[ljharb]

I don’t understand “supply chain concerns”; chalk itself is a dependency too, and this PR works by the same mechanism.

Whether to continue support for an engine or not is a separate decision - capriciously dropping support just to avoid adding a dep seems hostile to me, and in particular, antithetical to your goals of getting this PR out into the ecosystem.

[bmeck]

@ljharb My viewpoint is unchanged. I personally would not consider breakage in platforms that are out of the support matrix and their own support matrix as breaking changes. I do not think adding a dependency is a trivial matter but you might. I do not see any hostility, but if you wish to view my opinions as such you are free to frame them as you please. The goal of this PR is to prepare for situations where __proto__ is potentially unavailable, which it does.

[ljharb]

It also makes chalk brittle if someone deletes Object.setPrototypeOf, but that’s easily fixable, at least.

[bmeck]

Adding a dependency also adds some form of brittle-ness (registry going down / removal from registry / removal from disk / etc.). I don't see any major point to add to this discussion really.

[jridgewell]

node 0.10 and below don't have Object.setPrototypeOf, which still makes this PR a breaking change

Chalk supports node 8 and above:

chalk/package.json

Lines 9 to 11 in 797461e

	"engines": {
	"node": ">=8"
	},

[bmeck]

It does look like a variety of other things being used are not available in node 0.10.x like

https://github.com/chalk/chalk/blob/master/source/templates.js#L29

[ljharb]

@jridgewell ah, alright then - fair enough.

[ljharb]

In that case, Object.setPrototypeOf should still be cached at module level rather than freshly looked up off of Object, but I withdraw my other comments.

[bmeck]

I'll leave that to the maintainers, but existing usage in

chalk/source/index.js

Lines 41 to 42 in 797461e

	Object.setPrototypeOf(chalk, Chalk.prototype);
	Object.setPrototypeOf(chalk.template, chalk);

does not cache the value, nor do any of the other primordials/methods get cached. I think such a change for robustness could be discussed in a different PR.

[ljharb]

Fair enough as well.

[bmeck]

node has landed the CLI flag to disable __proto__ nodejs/node#32279 and deno now always disables it denoland/deno#4341

[devsnek]

I don't understand how Object.xyzPrototypeOf is more of a deletion hazard than Object.prototype.__proto__ (in fact i think __proto__ is technically more of a deletion hazard because people keep deleting it, which is why this pr exists in the first place).

[Qix-]

Just chiming in that there are a number of setPrototypeOf-based attacks floating around out there. Caching the function may not be the worst idea.

[ExE-Boss]

Don’t forget that Object.setPrototypeOf can be polyfilled in ES5 (and some ES3) environments that implement __proto__ as an accessor property on Object.prototype, in which case, one can do:

Object.defineProperty(Object, "setPrototypeOf", {
	configurable: true,
	writable: true,
	value: Function.prototype.call.bind(Object.getOwnPropertyDescriptor("__proto__").set)
});

[bmeck]

Node 13.12.0 now ships with the flag to remove Object.prototype.__proto__

[MatthiasKunnen]

If anyone wants to use --disable-proto but a dependency with an old version of chalk is holding you back, with Yarn's selective dependency resolutions you can force chalk to version 4.

Only recommended for forcing version 3 to 4. Lower versions might be a problem due to breaking changes.