microsoft / tslib

Runtime library for TypeScript helpers.
BSD Zero Clause License
1.25k stars 126 forks source link

__createBinding assumes exports are readonly #103

Closed imhoffd closed 4 years ago

imhoffd commented 4 years ago

Using tslib 1.12.0, we are seeing the following error being thrown:

TypeError: Cannot set property foo of #<Object> which has only a getter

This is a result of this commit, which changes __exportStar to use __createBinding, which now uses Object.defineProperty to create a readonly property on exports.

The implication is that it's no longer possible to do this in TypeScript:

# module1
export function foo() { /* do something */ }

# module2
export * from 'module1';
export function foo() { /* do another thing */ }

This differs from CommonJS behavior, where it is possible to overwrite previous exports.

mgabeler-lee-6rs commented 4 years ago

Same issue as #102

imhoffd commented 4 years ago

To maintain the "live binding", which seems to be the reasoning behind this change, can it also have a setter? e.g.:

Object.defineProperty(o, k2, {
  enumerable: true,
  get: function() { return m[k]; },
  set: function(v) { m[k] = v; }
});
weswigham commented 4 years ago

This is only an issue when using tslib with old emit, right? The emit TS 3.9 has hoisted exported variable assignments such that the __exportStar helper won't override local export names, and such that they can be reconfigured later on once actually assigned.

weswigham commented 4 years ago

@DanielRosenwasser We might need to remark that the new version of tslib should probably not be used without output from ts < 3.9.

linxiaowu66 commented 4 years ago

can fix this issue as soon as possible ? typeorm package can not use any more

sramam commented 4 years ago

We might need to remark that the new version of tslib should probably not be used without output from ts < 3.9.

Please consider making this a major release.

tslib is usually a (deeply) nested dependency and npm install of the parent auto-bumps to v1.12.0. Which escalates the issue from maintainers to users of the library.

Narrowing down to the root cause turns out to being really painful.

@linxiaowu66, npm install tslib@1.11.0 should solve the problem. Will likely need to be repeated after every npm install.

argshook commented 4 years ago

Agree with @sramam this should be treated as a major version.

This issue, together with https://github.com/microsoft/tslib/issues/105 is painful to track down.

I also saw cases where users don't even use typescript, but because of layers of dependencies, the issue rises up.

dasa commented 4 years ago

We might need to remark that the new version of tslib should probably not be used without output from ts < 3.9.

Is it only a problem when older versions use __exportStar?

DanielRosenwasser commented 4 years ago

Our current plan is outlined over at https://github.com/microsoft/tslib/pull/109. Can we get some feedback from users on this thread?

DanielRosenwasser commented 4 years ago

1.13.0 should be available for those impacted by the change. Users on TypeScript 3.9 should look into using tslib 2.0.0.

sramam commented 4 years ago

@DanielRosenwasser & @weswigham, thank you for the quick turn around!

imhoffd commented 4 years ago

Yes, thank you! Much appreciated. ❤️