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

Each helper in separate module #33

Closed
streamich opened this issue Jul 15, 2017 · 10 comments
Closed

Each helper in separate module #33

streamich opened this issue Jul 15, 2017 · 10 comments

Comments

@streamich
Copy link

Why not put each helper in a separate module and let TypeScript require only those that are actually needed?

@lmk123
Copy link

lmk123 commented Aug 17, 2017

First, install tslib:

npm install tslib

Then add importHelpers: true in your tsconfig.json:

{
  "compilerOptions": {
    "moduleResolution": "node",
    "module": "commonjs",
    "target": "es5",
    // ...
    "importHelpers": true // <-------
  }
}

Now when you compiler *.ts file, it may looks like this:

var tslib_1 = require("tslib")

tslib_1.__awaiter(...)

tslib_1.__extends(...)

@streamich
Copy link
Author

So you are importing the whole tslib there.

Again, why not put each helper in its own module and do something like this:

var _extends = require("tslib/lib/extends");

@aluanhaddad
Copy link
Contributor

Honestly, there is not a lot of code in this library. While the output does not lend itself to tree-shaking, your loader will only include it once so it doesn't seem like a code size issue.

Also, some of the functions reference each other, meaning that if the package were split, they would need cross referencing imports which would result in a more complex multi-module format export strategy than what is already in place.

@streamich
Copy link
Author

streamich commented Aug 20, 2017

Honestly, there is not a lot of code in this library. While the output does not lend itself to tree-shaking, your loader will only include it once so it doesn't seem like a code size issue.

@aluanhaddad "a lot of code" is relative, if I write a 10 line script for the web that uses only _extends, why would I incur a penalty of 200-line tslib.

@frankwallis
Copy link
Contributor

@streamich if you don't use importHelpers then typescript will inline only the methods which you actually use. importHelpers is to reduce helper methods being inlined across multiple modules.

@streamich
Copy link
Author

@frankwallis I know that, but it will inline those in each file though.

@DylanPiercey
Copy link

DylanPiercey commented Oct 22, 2017

I am also finding this to be a problem when publishing many small typescript modules. Ideally each utility would have it's own namespace require('tslib/$NAME').

Inlining the code directly means (when publishing a bunch of small modules) a bunch of code is duplicated, however when you use importHelpers it means you are incurring at least 2kb gzipped, which is a lot to ask for a module that is say 1kb gzipped.

I would personally advocate the approach taken by just however everything doesn't necessarily need to be in it's own published package.


It would be even cooler if these utilities could be used (without risk and with official support) in non typescript files to even further decrease duplication. For example in many of modules I require something like just-extend when I could easily use tslib.__extends if it were supported.

@DanielRosenwasser
Copy link
Member

Is there a reason that existing bundlers can't just do a good job here with tree-shaking? We provide both ES2015 and UMD outputs. Both Webpack and Rollup should be able to do a decent job here.

@DylanPiercey
Copy link

Just because they do doesn’t mean that this wouldn’t be a good change. This does nothing for browserify users or people using older versions of webpack.

This solution is both relatively easy to switch to and the support is guaranteed.

@orta
Copy link
Contributor

orta commented Oct 6, 2020

Yeah, I'm against the idea of splitting this into sub-modules too. Module bundlers can handle tree shaking tslib just fine nowadays (we have tests for in in #126).

I'm going to close 👍🏻

@orta orta closed this as completed Oct 6, 2020
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

7 participants