Skip to content

should pass --noEmitHelpers to tsc when compiling #1078

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
prabirshrestha opened this issue Dec 15, 2015 · 6 comments
Closed

should pass --noEmitHelpers to tsc when compiling #1078

prabirshrestha opened this issue Dec 15, 2015 · 6 comments

Comments

@prabirshrestha
Copy link

Currently the generated js code contains lot of duplicate __extends which can be seen here https://npmcdn.com/@reactivex/[email protected]/dist/global/Rx.umd.js

var __extends = (this && this.__extends) || function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};

This should be avoided by using tsc --noEmitHelpers.

@kwonoj
Copy link
Member

kwonoj commented Dec 16, 2015

In case of babel, it also has similar option and requires to import common helper module on runtime. While typescript compiler option does not specifies detail (https://github.com/Microsoft/TypeScript/wiki/Compiler%20Options) it seems like works similar way - is that correct? At least by local tryout, giving noEmitHelper option makes cjs build throws

    __extends(Subscriber, _super);
    ^

ReferenceError: __extends is not defined
    at D:\github\RxJS\dist\cjs\Subscriber.js:7:5

that helper function is not imported.

@prabirshrestha
Copy link
Author

Supporting external helpers library is part of TS v2.0 roadmap. https://github.com/Microsoft/TypeScript/wiki/Roadmap

Here are some of the issues discussing about it microsoft/TypeScript#2901, systemjs/builder#178, microsoft/TypeScript#3364

If using webpack you can use the imports-loader to fix the __extends issue. https://webpack.github.io/docs/shimming-modules.html#importing

Not sure about browserify. There could be something here https://github.com/thlorenz/browserify-shim.
Don't think it would be good to tell the user to define __extends as global var.

@kwonoj
Copy link
Member

kwonoj commented Dec 16, 2015

First, I don't think bundler can be considered as option since library should work out of box. For example, as noted above CJS module should work without any additional import, or bundling before any consumer start to use it.

Given aspect typescript compiler has plan to support it, I'd like to suggest to close issue this for now until those logistics are landed in compiler support. RxJS5 is living on leading-edge version of typescript (latest dev mostly) so once typescript dev branch adopt it as experimental, can try out quickly enough. What do you think @prabirshrestha ?

Also cc @Blesh before make decision to close out to have second opinion if there is.

disclaimer : personally I strongly prefer to remove duplication as original issue states, and appreciate to find out this is occurring on current codebase. My opinion is let's wait bit more to have environment / logistics to be more ready.

@prabirshrestha
Copy link
Author

I'm ok for closing this issue and revisiting the issue when typescript official supports runtime libs.

I havent tried it with rx but when I tested with one of my internal libs, I noticed that when gzipping the size is almost the same whether there are multiple __extends or a single __extends.

@kwonoj
Copy link
Member

kwonoj commented Dec 16, 2015

Thanks for confirmation. I'll close this for now, and reopen once typescript is ready to track further details of how to deliver this into RxJS. Appreciate once again, @prabirshrestha

@kwonoj kwonoj closed this as completed Dec 16, 2015
@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants