-
-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CallableInstance throws error in older ECMA 5 targets #245
Comments
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 Are you opposed to the guarding condition for some reason? |
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 |
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: |
I guess unified is quite popular so that might be why it pops up here, now.
👍 |
Initial checklist
Affected packages and versions
11.0.4
Link to runnable example
No response
Steps to reproduce
The CallableInstance class is using
Object.defineProperty
without checkingdescriptor.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 areconfigurable: false
, so callingObject.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 checkingconfigurable
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 thesuper('copy')
within the Processor's constructor.Affected runtime and version
node@20.11.1
Affected package manager and version
npm@11.0.4
Affected OS and version
No response
Build and bundle tools
No response
The text was updated successfully, but these errors were encountered: