Don't bundle external dependencies

[wessberg]

This pull request brings down the bundle size from many thousands of lines of code to just ~765 lines by not bundling along all external dependencies but instead relying on requiring/importing them from node_modules.

Why?

• Reuse of shared modules between multiple libraries brings the overall size of any library that depend on rollup-plugin-typescript2 down.
• Previously, since all dependent libraries were bundled along and marked as devDependencies, consumers couldn't override any of the dependencies by instead depending on a different version than the one rollup-plugin-typescript2 depends on.
• Smaller size and faster bundling. Since we don't need to resolve and flatten the dependency tree of external libraries, rollup-plugin-typescript2 doesn't require depending on rollup-plugin-commonjs and rollup-plugin-node-resolve any more.

[ezolenko]

Thanks for PR, but I intentionally bundled all the dependencies I could to minimize leftpad effect and to make sure as little as possible can change under me. Packages can go missing from the repo or can have breaking changes in patch releases or get taken over.

Static builds reduce problem area, but clients are paying for that in partial duplication of the code they would be including anyway, like lodash (rollup reduces overhead by shaking the tree somewhat).

[wessberg]

Thanks for your comments 😊👍

Regarding the leftpad issue, NPM did handle it by disallowing authors from unpublishing packages. And, you can depend on specific versions of packages if unintended breaking changes happen in libraries that you don’t control. So I would say we learned from the leftpad issue and fixed this problem in the ecosystem.

Thing is, it’s not as much the added cost of redundancy of code duplication that made me submit this PR as it is the fact that I expected to be able to depend on a different version of a dependency such as object-hash but couldn’t due to the bundling. This is a blocker for me since object-hash doesn’t handle untranspiled Async Functions.

I of course respect your decision should you choose to discard this PR, but please consider these points 😊

[domoritz]

I think adding a yarn.lock or package-lock.json with fixed versions prevents issues with breaking versions. I'm in favor of not copying all dependencies. It makes it harder to upgrade them (e.g. for new functionality or security issues), optimize the code, and also causes licensing issues as it's not clear who the authors are.

[ezolenko]

package.lock is on the building side -- to make sure I get same output on my machine and on a build machine that runs its own npm install for example. Bundled dependencies are on the user side to make sure when user runs npm install (using their own package.lock) nothing changes either.

With dynamic dependencies users generating their package locks at different times will have different versions of my own dependencies and have potentially different behavior.

Since this plugin is a build tool and won't normally be a part of end-user visible code, security issues and code optimizations are less relevant.

This is the same trade-off static builds have outside of js ecosystem.

Unless you are talking about bigger build tool suites that include this plugin as part of themselves (I think there is one or two already).

[domoritz]

Just to avoid confusion. I was talking about putting the yarn.lock file in the repo itself so that it is under version control. This way you know exactly what versions of packages to use and you have reproducible builds (i.e. everyone is going to build exactly the same code). This is what rust and ruby have been doing and it seems to become the standard for JavaScript as well.

[ezolenko]

Yep, that's what I was talking about too. That goes into source control and affects your own builds, but it is not a build artifact you can publish to npm repo with any effect on your package clients. (I think?)

[domoritz]

That goes into source control and affects your own builds, but it is not a build artifact you can publish to npm repo with any effect on your package clients. (I think?)

When I publish to npm, I usually build a package first and then publish the bundle with all dependencies in it. However, there is no need to have the bundle itself in the repo.

For example, https://github.com/vega/vega-lite doesn't have a bundle in the repo but here you can see that I include vega-lite.js with all dependencies compiled into it.

[wessberg]

Bundling all external dependencies does have a use case for libraries that should be executable directly from the browser. However, this plugin still relies on node-exclusive libraries and as such wouldn’t be browser-compatible without further work.

Even so, I’d still argue it would be preferable to generate a “rollup-plugin-typescript2.browser.js” bundle with inlined externals and otherwise not import/require external dependencies for cjs/mjs.

Also, the packages simply cannot be unpublished (npm support will have to get involved, won’t allow it if they are depended upon), but if they could, you would still have the leftpad issue since you list these dependencies in the package.json file and build a distributable bundle with them for each new release. In that case, you would have to take whatever content these packages have and manually place that code within your source folder.

[ezolenko]

@domoritz now you are talking about committing build artifacts into git, a slightly different issue. I do that deliberately to support specifying git url in package.json directly: "rollup-plugin-typescript2": "git+https://github.com/ezolenko/rollup-plugin-typescript2#master",. Easier to ask somebody to test master this way.

[ezolenko]

@wessberg I can fix versions of my direct dependencies and they would stay this way unless shrinkwrapped by the user, true. But I don't think I can fix dependencies of my dependencies this way -- so if I use package A 1.0.0 that depends on minor version of package B ^2.0.0, I can freeze those versions when doing my build with package.lock. But then when a user grabs my package C and the dependency chain C 3.0.0 -> A 1.0.0 -> B ^2.0.0 that last version can become 2.0.1 for that particular user and 2.0.2 for the other user, depending on when they generate their package.locks, unless they shrinkwrapped something explicitly.

This is the variability I want to minimize by bundling all bundlable in. Maybe this is not going to be a problem in practice, but that's the main rationale, minimizing moving parts. And of course it directly disables cases where you do want to update second tier dependencies without rebuilding, like in your case.

[wessberg]

Alright. I hear you. Let’s close this PR then. 😊