Emitter helper function could be override by user code

[Kingwl]

Bug Report

🔎 Search Terms

emit, helper

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________

⏯ Playground Link

https://www.typescriptlang.org/play?target=1#code/MYewdgzgLgBA+nApgDyosATCMC8MAUAlLgHwwDeMUAFgE4gDuMYiTAorfbUTAL4CwAKCHAANgEMI2AEIUY43DACMfERKkwAIjBRpMMigOGCAbuNowMilk01ER4CCFGIAdKJABzfBlfjCQA

💻 Code

const __extends = () => { throw new Error() }

class B { a = 1 }
class D extends B {}

var d = new D()
console.log(d.a)

🙁 Actual behavior

Exception

🙂 Expected behavior

console.log(1)

[MartinJohns]

Duplicate of #17485.

[Kingwl]

Okay. Thanks!

[hardfist]

It's very common that combine bundler like webpack | rollup | esbuild and transformer together, which will transform third party code using typescript compile api (which to make sure the bundled code doesn't have any es6+ code) ,but the third party code may already be transformed by typescript already(https://unpkg.com/browse/@pixi/core@6.0.0/dist/esm/core.js for example), so that means we have two helpers method __extends at the same time, what makes things worse is that if the source code both have transpiled code and untranspiled code and do scope hoist, which will make the inner helper methods shadows the outer helpers methods, which will likely break the code.

• transpiled third party code

// @pixi/core
***************************************************************************** */
/* global Reflect, Promise */

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 (b.hasOwnProperty(p)) { d[p] = b[p]; } } };
    return extendStatics(d, b);
};

function __extends(d, b) {
    extendStatics(d, b);
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
}

• untranspiled user's code

// src/lib.ts
class C {
}
class D extends C {
}

// src/index.ts
import * as lib from './src/lib.ts'
import * pixi from '@pixi/utils'

when bundled these code together and transpiled by typescript and do scope hoisting will generate the following code

"use strict";
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);
    };
    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 __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
})();
(function () {
    var C = /** @class */ (function () {
        function C() {
        }
        C.c = 1;
        return C;
    }());
    var D = /** @class */ (function (_super) {
        __extends(D, _super); **// __extends use the inner __extends because of shadowing, and extendStatics is undefined here**
        function D() {
            return _super !== null && _super.apply(this, arguments) || this;
        }
        return D;
    }(C));
    // @pixi/core
    /* global Reflect, Promise */
    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 (b.hasOwnProperty(p)) {
                    d[p] = b[p];
                }
            } };
        return extendStatics(d, b);
    };
    function __extends(d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    }
});

which will break the code

[MartinJohns]

That sounds like an issue with the bundler. It shouldn't bundle the code in a way that causes it to break.

[hardfist]

No, I bundle the code using esbuild firstly, which do scope hoist& bundler(esbuild only target to es6, so class not transpiled here), which generate the following code

(function () {
   // src/lib.ts // just bundle not transpiled yet
   class C {
   }
   class D extends C {
   }

    // @pixi/core
    /* global Reflect, Promise */
    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 (b.hasOwnProperty(p)) {
                    d[p] = b[p];
                }
            } };
        return extendStatics(d, b);
    };
    function __extends(d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    }
});

which is safe here, actually I don't whether the scope hoist here is reasonable @evanw,
but then I transpile the bundled code through typescript compiler api, it breaks

"use strict";
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);
    };
    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 __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
})();
(function () {
    var C = /** @class */ (function () {
        function C() {
        }
        C.c = 1;
        return C;
    }());
    var D = /** @class */ (function (_super) {
        __extends(D, _super); **// __extends use the inner __extends because of shadowing, and extendStatics is undefined here**
        function D() {
            return _super !== null && _super.apply(this, arguments) || this;
        }
        return D;
    }(C));
    // @pixi/core
    /* global Reflect, Promise */
    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 (b.hasOwnProperty(p)) {
                    d[p] = b[p];
                }
            } };
        return extendStatics(d, b);
    };
    function __extends(d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    }
});

So it's because typescript doesn't mangle helper method , which break the code. @RyanCavanaugh

[RyanCavanaugh]

The non-mangling is intentional and has been the way we've emitted code since forever; given multiple years of webpack + TS co-existing with no reports of this, it seems like you must have a misconfiguration somewhere.

The rules are in place so that you can put in your own __extends to fit different criteria. Even if we did mangle, we can't mangle in a way that predicts every possible other identifier that you might hoist in through a bundler rewrite. It's always going to be possible to actively break this on purpose, which is what that example is indistinguishable from.

[hardfist]

I didnt use weback but use esbuild +ts，because esbuild only tranform code to es6，I need to transform code to es5 by ts，which i think is very reasonable.

[hardfist]

https://github.com/microsoft/tslib/blob/master/tslib.js#L64-L74 https://github.com/microsoft/TypeScript/blob/master/src/compiler/factory/emitHelpers.ts#L561-L583 why these two helpers are different，and tslib implemention seems breaks for this case，since it put extendstatic outside，which hoist not working for previous code

[hardfist]

By the way, it seems that babel mangle the helpers method, if it's conflict
babel mangle helper

[Kingwl]

We may add a flag what if the flag turns on, tsc will rename the emitted helper name if there are conflicts in a file?

[RyanCavanaugh]

@rbuckton thoughts?

[rbuckton]

The two helpers are different intentionally. When emitting helpers without tslib, we don't want extendStatics to pollute global scope, but in tslib there is no concern for that to happen so extendStatics is moved out of the IIFE into the "module".

[rbuckton]

We've always allowed user override of built-in helpers as an "undocumented" feature. I'm not certain I understand what @hardfist means by the tslib implementation breaking for their case.

[rbuckton]

Ah, I see the issue. @pixi/core is pulling in tslib.es6.js and inlining the definition. The problem is that if we changed tslib.es6.js to use the same emit as the helpers we emit directly, then __extends would be undefined.

We could modify extends in tslib.es6.js to replace itself on is first call without using var:

export function __extends(d, b) {
    var extendStatics = Object.setPrototypeOf ||
        ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
        function (d, b) { for (var p in b) {
            if (b.hasOwnProperty(p)) {
                d[p] = b[p];
            }
        }
    };
    __extends = function(d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    }
    __extends(d, b);
}

I'm still not certain it would help in this case.

[hardfist]

it will helps, because function really hoist in the scope hoist scenario, but I do think helper mangle is better

[hardfist]

We've always allowed user override of built-in helpers as an "undocumented" feature. I'm not certain I understand what @hardfist means by the tslib implementation breaking for their case.

why allow user override built-in helpers?

[RyanCavanaugh]

It really seems like this is the bundler's fault; there are other scenarios where code taking a dependency on a global could get "blocked" by a hoisted declaration later in the same lexical block. Even mangling is not going to really work here since we would have no way of knowing what other names are in scope since those come from later bundles outside the knowledge of the current compilation.

[hardfist]

It really seems like this is the bundler's fault; there are other scenarios where code taking a dependency on a global could get "blocked" by a hoisted declaration later in the same lexical block. Even mangling is not going to really work here since we would have no way of knowing what other names are in scope since those come from later bundles outside the knowledge of the current compilation.

the bundler comes before compilation , so ts compilation should know all the info about scope name conflict