Skip to content

ES5 class rewriting incompatible with frozen intrinsics #43450

Closed as not planned

Description

@kriskowal

Bug Report

When TSC targets ES5, classes that extend other classes generate JS that throws a Cannot assign to read only property 'constructor' of object TypeError when the intrinsics, particularly the Function.prototype, are frozen. This affects programs that are freezing their intrinsics to protect against supply chain attacks, particularly using SES.

🔎 Search Terms

  • constructor
  • frozen

🕗 Version & Regression Information

I identified this issue in the published code of external-editor 3.1.0, which is using TypeScript 3.5.2 https://github.com/mrkmg/node-external-editor/blob/e1070a8295b8ebbafa7b29802ae0e2da0fad47c0/tsconfig.json

In the playground, the output is consistent for the very oldest and very newest versions of TypeScript when targeting ES5.

⏯ Playground Link

Playground link with relevant code

💻 Code

Object.freeze(Function);
Object.freeze(Function.prototype);
class Foo {
    constructor() {
    }
}
class Bar extends Foo {
    constructor() {
    }
}

🙁 Actual behavior

This code throws an exception like:

TypeError: Cannot assign to read only property 'constructor' of object ''

🙂 Expected behavior

This should not throw an exception. This can be achieved by using Object.defineProperty(prototype, 'constructor', {value: constructor}) instead of prototype.constructor = constructor. This is a symptom of the so-called property override mistake, in the design of ES5.

Activity

  1. RyanCavanaugh commented on Mar 30, 2021

    @RyanCavanaugh
    Member

    This can be achieved by using Object.defineProperty(prototype, 'constructor', {value: constructor})

    Our helper emit is designed to work in ES3 as well, where defineProperty is not available. I'm not sure that other alternatives are available; you can provide your own __extends helper that uses that approach if you're not running in ES3 environments.

  2. kriskowal commented on Mar 30, 2021

    @kriskowal
    Author

    There’s no way to discriminate the output based on ES5 vs ES3?

  3. kriskowal commented on Mar 30, 2021

    @kriskowal
    Author

    Would it be an option to feature-test Object.defineProperty to discriminate at runtime?

  4. RyanCavanaugh commented on Mar 30, 2021

    @RyanCavanaugh
    Member

    There’s no way to discriminate the output based on ES5 vs ES3?

    This just confuses the situation, because this is a global helper with "first in wins" semantics (to enable you to provide your own, as mentioned) - so if an ES3-targeted library loads before an ES5-targeted library, which is a totally legal situation, then the ES3 version of the helper is going to win and you're back where you started.

  5. kriskowal commented on Mar 31, 2021

    @kriskowal
    Author

    Would it work if the ES3 and ES5 helpers were identical:

    function __extends(x, y) {
      if (Object.defineProperty) {
        Object.defineProperty(x, 'constructor', {value: y});
      } else {
        x.constructor = y;
      }
    }
    
  6. DanielRosenwasser commented on Mar 31, 2021

    @DanielRosenwasser
    Member

    We do that in __makeTemplateObject, __createBinding, and __setModuleDefault for what it's worth.

  7. rbuckton commented on Apr 1, 2021

    @rbuckton
    Contributor

    We do that in __makeTemplateObject, __createBinding, and __setModuleDefault for what it's worth.

    I took a look and we do that wrong in __makeTemplateObject, unfortunately. For __createBinding and __setModuleDefault we check for Object.create (which is good), but we check for Object.defineProperty in __makeTemplateObject. The problem with testing for Object.defineProperty is that IE8's Object.defineProperty could only be used on DOM nodes, however IE8 did not support Object.create.

    So, we need to fix __makeTemplateObject, and the __extends helper should probably be:

    var __extends = (this && this.__extends) || (function () {
        var extendStatics = function (d, b) {
            extendStatics = Object.setPrototypeOf ||
                ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
                function (d, b) { for (var p in b) if (Object.prototype.hasOwnProperty.call(b, p)) d[p] = b[p]; };
            return extendStatics(d, b);
        };
        var setConstructor = function (f, d) {
            setConstructor = Object.create ?
                function (f, d) { Object.defineProperty(f, "constructor", { value: d, writable: true, configurable: true }); } :
                function (f, d) { f.constructor = d; };
            setConstructor(f, d);
        };
        return function (d, b) {
            if (typeof b !== "function" && b !== null)
                throw new TypeError("Class extends value " + String(b) + " is not a constructor or null");
            extendStatics(d, b);
            function __() { setConstructor(this, d); }
            d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
        };
    })();
  8. kumavis commented on Apr 12, 2021

    @kumavis

    Ron Buckton (@rbuckton) should this

    setConstructor = Object.create ?

    be

    setConstructor = Object.defineProperty ?
  9. kumavis commented on Apr 12, 2021

    @kumavis

    oh i see you explained exactly why you did this 🤦 I suppose adding a comment would help clarify its intentional. 😸

  10. erights commented on Jun 9, 2021

    @erights
  11. added a commit that references this issue on Sep 4, 2025
    83f7970
  12. gibson042 commented on Sep 9, 2025

    @gibson042

    I offered a fix at #62397, but it was closed because this issue is not open for PRs, with the implication that accepting it could impose friction for anyone relying on the incorrect behavior (which, for the record, is failure to evaluate the definition of a class that uses extends) and the explanation that user-side workarounds are "fairly straightforward" ("inject your own __extends or use a different downleveler").

    In my response, I question that anyone could be depending upon the incorrect behavior, and further challenge availability of the workarounds when this issue manifests inside a dependency, as in the case of both the initial report here (external-editor version 3.1.0) and the encounter that prompted me to open the PR (json-rpc-2.0 version 1.7.1)—nor would it be reasonable IMO to expect every JS-importable library written in TypeScript to customize downleveling of extends.

    What would it take to move this issue along such that #62397 could be reopened and accepted?

  13. RyanCavanaugh commented on Sep 9, 2025

    @RyanCavanaugh
    Member

    ES5 is being deprecated, so no further changes are warranted here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

In DiscussionNot yet reached consensusNeeds InvestigationThis issue needs a team member to investigate its status.SuggestionAn idea for TypeScript

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions