CallableInstance throws error in older ECMA 5 targets

[justinbhopper]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

11.0.4

Link to runnable example

No response

Steps to reproduce

The CallableInstance class is using Object.defineProperty without checking descriptor.configurable. When we used unified in a android mobile environment, we saw some differences in how the Processor class was being constructed that caused an error.

The difference we saw was if the javascript is transformed to target ES5 (ECMAScript 5.0), the Processor's copy method had a different number of property names defined than in ES6. Some of these properties are configurable: false, so calling Object.defineProperty on them throws a "Cannot redefine property: arguments" error.

In ES5:

In ES6 (Chrome):

Notice in ES6, the number of property names is simply ['length', 'name'], but in ES5 it includes additional property names that are not configurable. I believe simply checking configurable would solve this issue.

Expected behavior

Should be able to call unified() without error.

Actual behavior

Calling unified() throws an error "Cannot redefine property: arguments" during the super('copy') within the Processor's constructor.

Affected runtime and version

[email protected]

Affected package manager and version

[email protected]

Affected OS and version

No response

Build and bundle tools

No response

[wooorm]

I don't think we treat es5 as a support target -- why should this change happen here, is it not a bug in the tools you use to go from modern code to ancient code?

[justinbhopper]

I don't think we treat es5 as a support target -- why should this change happen here, is it not a bug in the tools you use to go from modern code to ancient code?

Even in ES6, checking configurable is proper to guard against this kind of error. I only mentioned ES5 because it was particular to how we encountered the issue, but it's entirely plausible for this error to happen in other circumstances.

Are you opposed to the guarding condition for some reason?

[wooorm]

The descriptor is passed to that function, shouldn't it handle its options and not fail?

Extra reason, is that's it's external code which was fine for years so wondering why it's a thing now https://github.com/CGamesPlay/node-callable-instance

[justinbhopper]

node-callable-instance would have the same bug as this does.

Object.defineProperty can't defy the rules of the property. The descriptor is passed in because the code is basically saying "define this property and use the same descriptions from this descriptor". However, for properties that aren't configurable, you can't redefine them. They're locked in place.

Since ES5 doesn't support classes, a ES6 class will be transpiled to a traditional function with some helper methods to simulate a object oriented class structure. Traditional functions has a few additional properties that ES6 classes don't have and they cannot be redefined.

Here is a playground that might illustrate this:
https://playcode.io/1896334

[wooorm]

I guess unified is quite popular so that might be why it pops up here, now.

Object.defineProperty

👍