Skip to content

[email protected] breaks typeorm #107

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

Closed
thynson opened this issue May 13, 2020 · 11 comments
Closed

[email protected] breaks typeorm #107

thynson opened this issue May 13, 2020 · 11 comments

Comments

@thynson
Copy link

thynson commented May 13, 2020

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
Copy link

I have the same error. I tried to install [email protected] 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
Copy link
Author

thynson commented May 13, 2020

I have the same error. I tried to install [email protected] 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
Copy link

georgekrax commented May 13, 2020

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
Copy link
Author

thynson commented May 13, 2020

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
Copy link

Fixed #107

@dyatko
Copy link

dyatko commented May 13, 2020

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

@DanielRosenwasser
Copy link
Member

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
Copy link

dyatko commented Jun 14, 2020

@DanielRosenwasser
Copy link
Member

@weswigham @rbuckton any clues?

@rbuckton
Copy link
Contributor

This is the main reason: typeorm/typeorm#6054 (comment)

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
Copy link
Member

cough microsoft/TypeScript#38809 cough

Maybe wanna review that and backport to 3.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants