microsoft / tslib

Runtime library for TypeScript helpers.
BSD Zero Clause License
1.26k stars 128 forks source link

tslib@1.12.0 breaks typeorm #107

Closed thynson closed 4 years ago

thynson commented 4 years ago

After upgrading tslib to 1.12, I get the following error message when requiring 'typeorm'(which requiring tslib@^1.9.0)

> require('typeorm')
Uncaught:
TypeError: Cannot set property EntityManager of #<Object> which has only a getter
    at Object.<anonymous> (/path/to/typeorm-test/node_modules/typeorm/index.js:120:23)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
    at Module.load (internal/modules/cjs/loader.js:1002:32)
    at Function.Module._load (internal/modules/cjs/loader.js:901:14)
    at Module.require (internal/modules/cjs/loader.js:1044:19)
    at require (internal/modules/cjs/helpers.js:77:18)

It seems like there's breaking changes in tslib.__exportStar() that defining getter instead of defining properties on target object. Resulting that once a symbol has already been exported by export * from './other-files' indirectly, the above error will be thrown when it is explicitly exported again in same file.

georgekrax commented 4 years ago

I have the same error. I tried to install tslib@1.11.2 with yarn & npm, and checked the package.json file, but nothing really happened. I was still getting the same error. So was to see this.

thynson commented 4 years ago

I have the same error. I tried to install tslib@1.11.2 with yarn & npm, and checked the package.json file, but nothing really happened. I was still getting the same error. So was to see this.

Have you tried removing the node_modules and lockfile before downgrading tslib to ~1.11.2?

georgekrax commented 4 years ago

I solved it afterwards. Here is my full answer.

P.S. If you have solved the problem. please close the issue that you opened here.

thynson commented 4 years ago

P.S. If you have solved the problem. please close the issue that you opened here.

I've solved it before submit this issue, but I think it's an accidential breaking change of tslib and they should fix it.

georgekrax commented 4 years ago

Fixed #107

dyatko commented 4 years ago

Is tslib even regression tested with the most popular TypeScript libraries?

DanielRosenwasser commented 4 years ago

1.13.0 is out and should fix things, 1.12.0 has been deprecated. 2.0.0 can be opted into for TypeScript 3.9 users.

dyatko commented 4 years ago

@DanielRosenwasser tslib@2.0.0 still fails https://github.com/typeorm/typeorm/issues/6054#issuecomment-643776357

DanielRosenwasser commented 4 years ago

@weswigham @rbuckton any clues?

rbuckton commented 4 years ago

This is the main reason: https://github.com/typeorm/typeorm/issues/6054#issuecomment-627568058

They have two lines in their index.ts file causing the issue:

https://github.com/typeorm/typeorm/blob/defa9bced0b5d1258e4f50d5c590978e6d3324d3/src/index.ts#L105

export * from "./entity-manager/EntityManager";

https://github.com/typeorm/typeorm/blob/defa9bced0b5d1258e4f50d5c590978e6d3324d3/src/index.ts#L141

export {EntityManager} from "./entity-manager/EntityManager";

We aren't tracking the re-export in the same way we do a local export:

https://www.typescriptlang.org/play/index.html?module=1#code/KYDwDg9gTgLgBAKjgMyhAtnARAOgPQQwAWwUWA3AFCiSxwDecAYhBHAL4pqa4HGkVq4aPADGAGwCGAZ2lwAQpKgN2QA

I think this is an issue for the compiler/emitter, not tslib. We really should also have something like an exports.Foo = void 0; assignment at the top of the file like we do for Bar, just so we don't attempt to re-export it later.

input

// @target: esnext
// @module: commonjs
// @importHelpers: true
export * from "./other";
export { Foo } from "./other";
export class Bar {}

actual output

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Bar = void 0;
const tslib_1 = require("tslib");
tslib_1.__exportStar(require("./other"), exports);
var other_1 = require("./other");
Object.defineProperty(exports, "Foo", { enumerable: true, get: function () { return other_1.Foo; } });
class Bar {
}
exports.Bar = Bar;

expected output

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Bar = exports.Foo = void 0;
//            ^^^^^^^^^^^^^
const tslib_1 = require("tslib");
tslib_1.__exportStar(require("./other"), exports);
var other_1 = require("./other");
Object.defineProperty(exports, "Foo", { enumerable: true, get: function () { return other_1.Foo; } });
class Bar {
}
exports.Bar = Bar;
weswigham commented 4 years ago

cough https://github.com/microsoft/TypeScript/pull/38809 cough

Maybe wanna review that and backport to 3.9