"constructor" is enumerable when targetting es5

Open
#104 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
48/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
typescript
Domain
tooling

Research direction

Start at the __extends helper and reproduce the example in the TypeScript playground with an ES5 target, then compare it with ES2015 output. The work is done when the inherited constructor is not enumerable during iteration, while the desired behavioral change is confirmed for the helper.

Written by the indexing model from the issue text.

Description

Hi, I've noticed that the behavior of "__extends" with regards to defining the "constructor" property is unexpected when you target "es5".

Try the following example in the TypeScript playground:

class TestSuper { }
class TestSub extends TestSuper { }

const properties = [];
for (const p in new TestSub()) {
    properties.push(p);
}
console.assert(properties.length === 0, "Should not have any enumerable properties");
console.assert(properties.indexOf("constructor") === -1, "Should not have constructor as an enumerable property");

If you compile this with ES2015, then classes and inheritance are natively supported, and running the code doesn't print anything to the console (as "constructor" isn't enumerable).
If you change the target to "es5", then you'll see stuff in the console, since "constructor" is now enumerable.

The fix should be easy, it's a matter of redefining __extends as follows:

    __extends = function (d, b) {
        extendStatics(d, b);
        function __() { Object.defineProperty(this, "constructor", { configurable: true, value: d, writable: true }); }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };

Notice that the current implementation doesn't use defineProperty and is instead assigning the this.constructor member directly.

Is this a behavioral change that we'd like? I can submit a PR for it. But since it would be a breaking change, I don't know what would be the decision here.
As additional input, we've been using an updated version of __extends with this fix for several years in the Azure Portal.

Dominant language
TypeScript
Stars
1.3k
Forks
145
No merged PRs in 30d

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from microsoft/tslib

All issues in microsoft/tslib

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.