Skip to content

Commit

Permalink
fix(build): use individual RxJS imports for webpack treeshacking
Browse files Browse the repository at this point in the history 
Fixes #731
  • Loading branch information
@dhardtke @ocombe
dhardtke authored and ocombe committed Nov 24, 2017
1 parent a8f23a4 commit 141e23e
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/src/translate.service.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import {EventEmitter, Inject, Injectable, InjectionToken} from "@angular/core";
import {Observable} from "rxjs/Observable";
import {Observer} from "rxjs/Observer";
import {concat, share, map, merge, switchMap, toArray, take} from "rxjs/operators"
import {concat} from "rxjs/operators/concat";
import {share} from "rxjs/operators/share";
import {map} from "rxjs/operators/map";
import {merge} from "rxjs/operators/merge";
import {switchMap} from "rxjs/operators/switchMap";
import {toArray} from "rxjs/operators/toArray";
import {take} from "rxjs/operators/take";
import {of} from "rxjs/observable/of";
import {
MissingTranslationHandler,
Expand Down

9 comments on commit 141e23e

@vecernik
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't you forget mergeAll? Aot builds with angular cli works, but there's error in call a12434.mergeAll() in uglified get() and stream() code.

@Mesamo
Copy link

@Mesamo Mesamo commented on 141e23e Dec 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vecernik I have the same error, How did you solve it?

@vecernik
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, I had to lock ngx-translate back to 9.0.0, which works ok.

@Mesamo
Copy link

@Mesamo Mesamo commented on 141e23e Dec 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vecernik thanks

@vecernik
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's interesting that the error of a12434.mergeAll() appears after a successful aot build in runtime.

@MattiJarvinen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This project doesn't even seem to contain calls to mergeAll anywhere ( at least search doesn't produce any results in code ). You could try to check your project source first.

@ocombe
Copy link
Member

@ocombe ocombe commented on 141e23e Dec 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this project doesn't use mergeAll, and the generated built files don't contain mergeAll anywhere

@MattiJarvinen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vecernik you probably are using Observer.mergeAll currently the shift in using RxJS is to use lettable operators which do not monkeypatch Observer prototype.

You should read https://github.com/ReactiveX/rxjs/blob/master/doc/lettable-operators.md and either move to using pipe and lettable operators yourself or import 'rxjs/add/operator/mergeAll'; somewhere in your code.

@vecernik
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bug I mentioned during a release of 9.0.1 could be related to another bug in RxJs. With 9.0.2 and latest RxJs it looks ok, thank you guys.

Please sign in to comment.