search

ngUpgraders / ng-forward

The default solution for those that want to write Angular 2.x style code in Angular 1.x
411 stars 36 forks source link

Update rxjs dependency #122

Open timkindberg opened 8 years ago

timkindberg commented 8 years ago

Apparently our lib doesn't work with latest rxjs release.

https://www.npmjs.com/~reactivex

SergDi commented 8 years ago

event-emitter.js:15Uncaught TypeError: Super expression must either be null or a function, not object

timkindberg commented 8 years ago

I've confirmed this bug. I updated the package.json to point to 'rxjs' instead of '@reactivex'. It's still 5.0.0-beta.0 though. I'm pretty sure it's the call to super() happening in EventEmitter.ts. @MikeRyan52 any chance you could take a quick look? Here's where I'm at: https://github.com/ngUpgraders/ng-forward/commit/d5001580c5c71f4d348eff24504db5d9f6770936 on a new branch https://github.com/ngUpgraders/ng-forward/tree/issue-122

MikeRyanDev commented 8 years ago

Maybe we should just copy Angular 2's EventEmitter (code again)[https://github.com/angular/angular/blob/master/modules/angular2/src/facade/async.ts#L109]? It's been updated for the latest release of RxJS (now beta 1, I think)

timkindberg commented 8 years ago

Except for some line breaks in that update I did copy the latest version of EventEmitter at the time. It's not liking it. See: https://github.com/ngUpgraders/ng-forward/commit/d5001580c5c71f4d348eff24504db5d9f6770936

MikeRyanDev commented 8 years ago

Ah, sorry. Did not realize that's where you started. I'll take a closer look.

timkindberg commented 8 years ago

That fixed it locally (the {Subject} advice)! I've pushed to the issue-122 branch, we'll see if it goes through CI.

timkindberg commented 8 years ago

Hmm... nope... failed. It runs the tests locally... I'm using npm 2, not sure if that matters.

timkindberg commented 8 years ago

Tried installing 'rxjs-es' too... and that's failing too. @MikeRyan52 I'm not sure.

laurelnaiad commented 8 years ago

I'm not sure what went wrong with the tests, but the change that maps EventEmitter::emit to Subject::next re-enables that function for me, and is one of two reasons, since it's not in npm, yet, that I'm running from a fork (the other being addressed in https://github.com/ngUpgraders/ng-forward/pull/141).

What criteria must be met to get to a next npm release? Anything I can do to help it along?

Edit: forgot to mention that holding down to reactivex/rxjs@5.0.0-alpha.7 instead of using 5.0.0-beta.1 also seems to be be necessary-ish, as under beta.1 I get: "src/browser/node_modules/ng-forward/dist/es6/events/event-emitter.d.ts(1,8): error TS1192: Module '"src/browser/node_modules/@reactivex/rxjs/dist/es6/Subject"' has no default export."

In beta.1, rxjs dropped default from the class export.