Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update rxjs dependency #122

Open
timkindberg opened this issue Dec 21, 2015 · 9 comments
Open

Update rxjs dependency #122

timkindberg opened this issue Dec 21, 2015 · 9 comments
Labels
bug
Milestone
Milestone 2

Comments

@timkindberg
Copy link
Contributor

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

https://www.npmjs.com/~reactivex
@timkindberg timkindberg added this to the Milestone 2 milestone Dec 21, 2015
@SergDi
Copy link

SergDi commented Dec 22, 2015

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

@MikeRyanDev MikeRyanDev mentioned this issue Jan 4, 2016
Closed
@timkindberg timkindberg added bug and removed chore labels Jan 6, 2016
timkindberg added a commit that referenced this issue Jan 12, 2016
@timkindberg
fix(EventEmitter): Update rxjs dependency. Add 'emit' method.
163df75 
fixes #122

BREAKING CHANGE:

Before:
```js
@output() foo = new EventEmitter();

foo.next();
```

Before:
```js
@output() foo = new EventEmitter();

foo.emit(); // <--
```
timkindberg added a commit that referenced this issue Jan 12, 2016
@timkindberg
fix(EventEmitter): Update rxjs dependency. Add 'emit' method.
d500158 
fixes #122

BREAKING CHANGE:

Before:
```js
@output() foo = new EventEmitter();

foo.next();
```

Before:
```js
@output() foo = new EventEmitter();

foo.emit(); // <--
```
@timkindberg
Copy link
Contributor Author

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: d500158 on a new branch https://github.com/ngUpgraders/ng-forward/tree/issue-122

@timkindberg timkindberg reopened this Jan 14, 2016
@kwonoj kwonoj mentioned this issue Jan 15, 2016
Closed
@MikeRyanDev
Copy link
Member

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
Copy link
Contributor Author

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

@MikeRyanDev
Copy link
Member

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

timkindberg added a commit that referenced this issue Jan 19, 2016
@timkindberg
fix(EventEmitter): Update rxjs dependency. Update EventEmitter. Add '…
38ad789 
…emit' method and deprecate 'next' method.

fixes #122

BREAKING CHANGE:

Before:
```js
@output() foo = new EventEmitter();

foo.next();
```

Before:
```js
@output() foo = new EventEmitter();

foo.emit(); // <--
```
@timkindberg
Copy link
Contributor Author

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

@timkindberg
Copy link
Contributor Author

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

@timkindberg
Copy link
Contributor Author

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

@laurelnaiad
Copy link
Contributor

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 #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/[email protected] 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
Milestone 2
Development

No branches or pull requests
4 participants
@timkindberg @MikeRyanDev @laurelnaiad @SergDi