chore(build): Inject tslib functions

[imcotton]

To avoid prepend tslib globally.

[imcotton]

Odd, unit test seems failed on the master HEAD already (see #2122), shouldn't caused by this PR I think.

[kwonoj]

one another unexpected behavior of yarn, it picks up 2.1.1 of TypeScript compiler.

#2120 will resolve build failure by taking out yarn.

[coveralls]

Coverage remained the same at 97.685% when pulling 9648b42 on imcotton:inject-tslib into b876a3f on ReactiveX:master.

[kwonoj]

Change.. looks fine to me, but it'd be appreciate to elaborate few details if possible

• how to test regressions for this changes?
• how much gain after applying this changes?

[imcotton]

This PR is the first step toward to its ultimate goal, which depends on two another up coming developments:

1. Set --importHelpers when targeting to TS v2.1
2. tslib module provides full ES6 outputs (Add __generator helper to es6 source and populate jsnext:main microsoft/tslib#5)

Where No.1 lands then below changes will no longer required:

rollupInject({
  exclude: 'node_modules/**',
  modules: _.mapValues(tslib, function (value, key) {
    return ['tslib', key];
  }),
})

Where No.2 lands then below changes will no longer required:

rollupCommonJS({
  namedExports: {
    tslib: Object.keys(tslib),
  },
})

The final result will let Rollup being able to tree-shakes every helper functions in tslib module, but right now this is as near as it gets.

To the test matter that I couldn't figure out yet, would be great to have any suggestion.

[kwonoj]

Please allow me some dumb question, as 2.1 is nearby and seems tslib change's also suggested as PR, is there specific reason to not to wait those release and make desired change at once? Is this change makes functional changes need to be adapted as soon as possible?

[imcotton]

One reason to me is by simply prepending tslib for a library bundle seems too rough and aggressive, since it touches global env of user end, another concern is adopting TS v2.1 might not going to happen soon (at least before 5.0 final release).

[kwonoj]

So this seems depends on when codebase moves to TS@2.1. I think once it becomes public release (and if there is no regression) it can be upgraded fairly easily as codebase won't have 2.1 specific breaking features. Question would be when is planned for v5 public release.

[imcotton]

So there are two taking of this PR:

1. Merge right away, and propose new PR which only minus two plugin calls when requirements satisfied.
2. Modify current PR to its final shape with labeled WIP and waits for the time ready to hit the merge button.

[coveralls]

Coverage remained the same at 97.686% when pulling 63916a8 on imcotton:inject-tslib into 128fb9c on ReactiveX:master.

[Flounn]

Can you take a decision with this pr? We can't use the Rx.min.js file in an Angular 2 project, we need to rebuild the rxjs library.
Thx

[jayphelps]

Can someone explain this to me in laymens? Seems like it's basically don't bundle tslib in global builds, instead rely on it being globally available? If that's the case, this is a breaking change btw.

[kwonoj]

@jayphelps no afaik it's how to inject tslib instead of reading tslib directly as file and concat what we do currently. @imcotton can explain in detail.

[jayphelps]

We have some thoughts and concerns, we're going to discuss this Monday at our team meeting.

[imcotton]

This change is utilizing rollupInject plugin to gather global function calls depend on tslib during its tree-shaking process, which can be achieved by --importHelpers flag from TS v2.1 (see support for external helpers library).

[coveralls]

Coverage remained the same at 97.698% when pulling bf58dcb on imcotton:inject-tslib into cba7413 on ReactiveX:master.

[imcotton]

I've had rebase against master branch.

[jayphelps]

Thanks @imcotton !

[lock]

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.