-
Notifications
You must be signed in to change notification settings - Fork 3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
build: use tsc importHelpers flag to remove duplicate helpers from es…
…m5 and esm2015 builds (#3416) I intentionally didn't use the flag for cjs builds because that would make cjs builds incompatible with SystemJS without further configuration on the client-side - this would be highly undesirable.
- Loading branch information
Showing
3 changed files
with
7 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
616710a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reposting comment from #2436 (comment) for higher visibility:
I implemented this feature in #3416. I did the following three things very intentionally (but I'm open to discussion if someone finds them problematic):
importHelpers
is enabled only foresm5
andesm2015
builds because these are the builds that are further optimized by other build tools (rollup and webpack).importHelpers
are disabled forcjs
builds - we still emit the helpers inline in each file - this was done to preserve the current SystemJS support for folks that still require SystemJS - we can revisit this in the future versions if we decide that we no longer want to support SystemJS. In the meantime , I'm not aware of any downside for node users (primary users of cjs builds) when it comes to inlining the helpers incjs
builds.tslib
to be a dependency rather than peerDependency of rxjs - this was done to not require that every application that depends on rxjs has to provide tslib in it's package.json. A change like this would make adoption of RxJS v6 much harder for developers that don't already use tslib in their project (e.g. most non-Angular projects would be affected, while Angular users would be unaffected).tslib
as dependency is that there is a potential for having to deal with duplicate versions oftslib
in a single project which could increase the size very slightly but shouldn't have any other impact.tslib
is a stateless library, so AFAIK it's ok to have multiple versions of it in a single runtime dependency graph. For users that do care about size, they can just ensure to provide a compatible tslib in the root package.json of the app and let npm/yarn dedupe the dependencies - in most cases this should happen without users having to worry about it.