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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override helpers #14

Open
jods4 opened this issue Dec 17, 2016 · 8 comments
Open

Override helpers #14

jods4 opened this issue Dec 17, 2016 · 8 comments

Comments

@jods4
Copy link

jods4 commented Dec 17, 2016

Introducing support for tslib in TS 2.1 is great as we can now easily use up-to-date TS helpers without maintaining our own copy of the source files from github. 馃槃

But it also introduced a regression: the option to override TS helpers with our own seems to have been lost?

Specifically, I want to provide my own __extends because the one provided by TS is (deliberately) buggy.
Before I could do that by having a global __extends that would take precedence. With the new --importHelpers it seems that I lost this option.

What is the recommended way to go about that?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 19, 2016

I would say this should be done on the TS side of things. the compiler could allow an option for --importHelpers specifying the other library name, and it would import helpers from it, allowing you to have a custom import helper library.

@jods4
Copy link
Author

jods4 commented Dec 19, 2016

Not sure you need to do anything like that. What you describe is basically what we've done until now: --noEmitHelpers +having my own file copied from github. The only difference is whether we use globals or a module but it doesn't matter much here.

The problem with this approach is that we have to copy the whole thing to modify just __extends. Updating our copy when you tweak the helpers is not good because it's hard to know when something changes.

BTW: maybe you could reconsider your position on __extends... As IE 10- is less and less relevant, I think the benefits of having standard compliant code-gen in widespread browsers is more important than being consistently incorrect on some edge cases.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 19, 2016

The problem with this approach is that we have to copy the whole thing to modify just __extends. Updating our copy when you tweak the helpers is not good because it's hard to know when something changes.

you can still export * from tslib if you want.

BTW: maybe you could reconsider your position on __extends... As IE 10- is less and less relevant, I think the benefits of having standard compliant code-gen in widespread browsers is more important than being consistently incorrect on some edge cases.

I am sure you will have different feelings if you are supporting users on IE8, IE9 and IE10. We have heard feedback from multiple users who care about this scenario.

@jods4
Copy link
Author

jods4 commented Dec 19, 2016

you can still export * from tslib if you want.

Good point!

I am sure you will have different feelings if you are supporting users on IE8, IE9 and IE10. We have heard feedback from multiple users who care about this scenario.

If I may:
(a) then maybe it would be nice to have a choice and being able to opt-in standard-compliant emit (e.g. with a flag on the command line).
(b) when I had to support IE8-10 (heck, I started with IE6), I was always sure to develop and test on the lowest version supported. We are talking about an edge-case that can't ever work in those browsers (so you better not depend on it even with the current emit). If your users support IE8-10 but never test with any of those three, then I wouldn't say they actually support IE10.

In the meantime... people who try to write standard-compliant code, or who try to migrate from Babel now that your downlevel emit is complete, have a really fun time debugging this stuff out. It takes a lot of time to understand why your Reflect.metadata polyfill doesn't seem to work, trust me I did.. several times 馃槅

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

We have a proposal for __extends in microsoft/TypeScript#12488 that should work for IE10 as well as newer browsers.

@jods4
Copy link
Author

jods4 commented Dec 21, 2016

Well, that proposal is great because it will be standard-compliant in IE11+ and it will fix issues in frameworks that use metadata when code is compiled to ES5. 馃帀

But I should admit that I don't see how that would work for IE10-??
If I'm looking at the right code, you would be using setPrototypeOf or __proto__ when available. Neither of those are in IE 10-, so we'll keep on using the current strategy, which is copying the properties.

This is exactly what I've been asking for but I need to point out that you would introduce a different behavior between IE10- and IE11+, which I thought was exactly what you wanted to avoid.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

that is the best we could come up with :) trying to keep existing code working as well.

@jods4
Copy link
Author

jods4 commented Dec 21, 2016

that is the best we could come up with :)

No pressure! It's impossible to emulate inheritance in IE10- with 100% fidelity.

I am glad you changed your mind about having a different ES-compliant behavior in capable browsers! 馃槂

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

2 participants