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.

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

Type definition for extended operator is not published #642

Closed
kwonoj opened this issue Nov 3, 2015 · 17 comments
Closed

Type definition for extended operator is not published #642

kwonoj opened this issue Nov 3, 2015 · 17 comments
Milestone

Comments

@kwonoj
Copy link
Member

kwonoj commented Nov 3, 2015

Code snippet below works in JS,

var Rx = require('....Rx.KitchenSink');
Rx.Observable.of(42).isEmpty().subscribe(console.log);

when it comes to TS

import * as Rx from '....Rx.KitchenSink';
Rx.Observable.of(42).isEmpty().subscribe(console.log);

compiler will complain due to Observable.of does not returns extended type of operator instead of core.
error TS2339: Property 'isEmpty' does not exist on type 'Observable<number>'.

@kwonoj
Copy link
Member Author

kwonoj commented Nov 3, 2015

This issue is related with modularity strategies. Tried some approaches ends up with vain, exploring few other approaches including mixins.

@benlesh
Copy link
Member

benlesh commented Nov 3, 2015

cc/ @robwormald

@benlesh
Copy link
Member

benlesh commented Nov 3, 2015

cc/ @jeffbcross

@kwonoj
Copy link
Member Author

kwonoj commented Nov 4, 2015

Created PR #643 for initial proposal based on mixin, and possibly including ambient declaration merging if it's necessarily required.

@robwormald
Copy link
Contributor

Nice work OJ! I'll try this out later today but at first glance it looks
like it'll solve a bunch of issues for us.

On Wednesday, November 4, 2015, OJ Kwon notifications@github.com wrote:

Created PR #643 #643 for initial
proposal based on mixin, and possibly including ambient declaration merging
if it's necessarily required.


Reply to this email directly or view it on GitHub
#642 (comment).

@kwonoj
Copy link
Member Author

kwonoj commented Nov 20, 2015

Related with #750 .

@benlesh
Copy link
Member

benlesh commented Jan 15, 2016

@kwonoj @robwormald is this issue resolved? Can we close this?

@benlesh benlesh modified the milestones: 5.0.0 release, Initial Beta Jan 15, 2016
@kwonoj
Copy link
Member Author

kwonoj commented Jan 15, 2016

No, it's still there and we don't have decent solution yet.

@benlesh
Copy link
Member

benlesh commented Jan 15, 2016

@kwonoj @jeffbcross @robwormald ... is it reasonable to say this is a problem with TypeScript and not necessarily with how RxJS is structured?

@kwonoj
Copy link
Member Author

kwonoj commented Jan 15, 2016

I'm considering half and half. TypeScript can possibly bring better feature resolves this issue, but I would expect it'll eventually requires change structures of RxJS codebase, just matter of how much. Short example would be interfaces are tied to return Observable only as well as implementation of lift does, etcs.

As an issue tracking perspective, I don't think this can be updated in relatively short time frame. I'm seeing couple of discussions are in TypeScript perspective would require some time to something's actually happening. I can suggest close this for now and bring it back once circumstances allowing further discussion.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 2, 2016

Thinking about this issue, I came to think this also could be resolved via module augmentation as same as #1193, with introducing some organization changes. It might be get rid of interface KitchenSinkOperators and let CoreOperators contains all signatures (maybe renaming required for interface? ) then publish type of each extended operator via module augumentation. By this way, importing Rx.Kitchensink includes extended operator patches as well as type definition belongs Observable, removes type inference issues currently occurring.

Still this is vague idea, might need to come up with solid changes once infrastructure (primarily min version of typescript) is ready.

@david-driscoll
Copy link
Member

In theory CoreOperators and KitchenSinkOperators can go away once we have augmentation for all operators in place.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 3, 2016

I'm bit on fence to get rid of interface itself, but yes - augmentation can make it happen. Let me explore further to come up with proof of concept.

@benlesh
Copy link
Member

benlesh commented Mar 21, 2016

ping @kwonoj ... can this be closed now?

@kwonoj
Copy link
Member Author

kwonoj commented Mar 21, 2016

Yes, I thought it's already closed.. :/ Thanks for finding, closing.

@kwonoj kwonoj closed this as completed Mar 21, 2016
@kwonoj
Copy link
Member Author

kwonoj commented Mar 21, 2016

Note: this issue is closed by #1388.

@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants