Skip to content

Commit

Permalink
build: use tsc importHelpers flag to remove duplicate helpers from es…
Browse files Browse the repository at this point in the history 
…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
@IgorMinar @benlesh
IgorMinar authored and benlesh committed Mar 8, 2018
1 parent bd28eaa commit 616710a
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 1 deletion.
4 changes: 3 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@
"url": "https://github.com/ReactiveX/RxJS/issues"
},
"homepage": "https://github.com/ReactiveX/RxJS",
"dependencies": {
"tslib": "^1.9.0"
},
"devDependencies": {
"@angular-devkit/build-optimizer": "0.0.24",
"@types/chai": "4.1.2",
Expand Down Expand Up @@ -226,7 +229,6 @@
"symbol-observable": "1.0.1",
"systemjs": "^0.21.0",
"ts-node": "4.1.0",
"tslib": "1.5.0",
"tslint": "5.9.1",
"tslint-no-unused-expression-chai": "0.0.3",
"typescript": "latest",
Expand Down
2 changes: 2 additions & 0 deletions tsconfig/tsconfig.esm2015.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"extends": "./tsconfig.base.json",
"compilerOptions": {
"module": "es2015",
"importHelpers": true,
"moduleResolution": "node",
"target": "es2015",
"outDir": "../dist/esm2015"
}
Expand Down
2 changes: 2 additions & 0 deletions tsconfig/tsconfig.esm5.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"extends": "./tsconfig.base.json",
"compilerOptions": {
"module": "es2015",
"importHelpers": true,
"moduleResolution": "node",
"target": "es5",
"outDir": "../dist/esm5"
}
Expand Down

1 comment on commit 616710a

@IgorMinar
Copy link
Contributor Author

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):

  1. importHelpers is enabled only for esm5 and esm2015 builds because these are the builds that are further optimized by other build tools (rollup and webpack).
  2. importHelpers are disabled for cjs 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 in cjs builds.
  3. I set 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).
  • the downside of declaring tslib as dependency is that there is a potential for having to deal with duplicate versions of tslib 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.

Please sign in to comment.