Skip to content
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

__createBinding assumes exports are readonly #103

Closed
imhoffd opened this issue May 12, 2020 · 12 comments
Closed

__createBinding assumes exports are readonly #103

imhoffd opened this issue May 12, 2020 · 12 comments

Comments

@imhoffd
Copy link

imhoffd commented May 12, 2020

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

Same issue as #102

@imhoffd
Copy link
Author

imhoffd commented May 12, 2020

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

weswigham commented May 13, 2020

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

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

@linxiaowu66
Copy link

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

@sramam
Copy link

sramam commented May 13, 2020

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

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

This issue, together with #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
Copy link

dasa commented May 13, 2020

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

Our current plan is outlined over at #109. Can we get some feedback from users on this thread?

@DanielRosenwasser
Copy link
Member

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

sramam commented May 14, 2020

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

@imhoffd
Copy link
Author

imhoffd commented May 14, 2020

Yes, thank you! Much appreciated. ❤️

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

8 participants