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

RFC: When to remove Observable#lift? #7201

Closed
benlesh opened this issue Mar 1, 2023 · 0 comments
Closed

RFC: When to remove Observable#lift? #7201

benlesh opened this issue Mar 1, 2023 · 0 comments
Labels
8.x Issues and PRs for version 8.x 9.x AGENDA ITEM Flagged for discussion at core team meetings

Comments

@benlesh
Copy link
Member

benlesh commented Mar 1, 2023

This is an issue to track to the deprecation and removal of lift in v9.

lift was a really interesting idea and a revolutionary thing we added back in v5. The initial promises it held were around performance, the idea that you could add custom "methods" to an inherited custom Observable and keep them after calling a native method, and that enabled things like Subject to still be a Subject after you operated on it.

Why we'd want to remove it

  1. It means we're implementing our operators the same way we'd recommend other people implement their operators.
  2. It means that we're compatible with any "same shaped" observable implementation more directly. )See test in associated PR here: 6403d8a#diff-c8b60d7f1c5a1e59f4a311cdc025d29918fef109c66428feaa6a7aa193803928R283-R305)
  3. It reduces code size and code complexity.
  4. It preps us for a future where a standalone RxJS Observable doesn't have anything "RxJS-specific" on it, (except maybe Observable#pipe, which is just generally useful).
  5. We stopped using "methods" for operators years ago, and we've long recommended against people inheriting from Observable. In fact, it's an industry standard that subclassing types you don't own is a bad practice unless it's documented API (React.Component, or something like that). So lift doesn't have the same purpose it used to.

Remove in v8 or v9?

So there's some debate to be had here with the Core Team:

While we've already had lift deprecated for a very long time. We're making it internal in v8 (per the comments in the codebase). And the "lifted observable" behavior is still supported, even if it's not supported by TypeScript.

Should we completely remove lift in v8? We can get away with this since it's been slated to be made internal (meaning it's not a public API in version 8). In other words, if it's not a public API, no one should have any expectations of behaviors associated with it beyond things that RxJS itself does.

The only issue I see is currently RxJS supports the composition of our subject variants through the operator chain, via lift. This is a little known aspect of RxJS that isn't used much, in no smart part because TypeScript can't support it:

const subject = new Subject<number>()

const lifted = subject.pipe(map(n => n + '?'), map(s => s + '!');

lifted.subscribe(console.log);

(lifted as any).next(42); // logs "42?!"

The workaround if we remove lift entirely is the obvious and correct thing:

const subject = new Subject<number>()

const lifted = subject.pipe(map(n => n + '?'), map(s => s + '!');

lifted.subscribe(console.log);

subject.next(42); // logs "42?!"

I say it's more "correct" because really it's probably not great to pass your data producer around as part of your consumer API. (We even provide asObservable() to cover that)

@benlesh benlesh added AGENDA ITEM Flagged for discussion at core team meetings 8.x Issues and PRs for version 8.x 9.x labels Mar 1, 2023
benlesh added a commit to benlesh/rxjs that referenced this issue Mar 2, 2023
+ Removes `lift`, `source`, and `operator` from `Observable`
+ Updates related code

Resolves ReactiveX#7201
benlesh added a commit to benlesh/rxjs that referenced this issue Mar 2, 2023
+ Removes `lift`, `source`, and `operator` from `Observable`
+ Updates related code

BREAKING CHANGE: `Observable#lift`, `Observable#source`, and `Observable#operator` is no longer a part of the API. These were never meant to be public and have been deprecated for more than 3 years.

Resolves ReactiveX#7201
benlesh added a commit to benlesh/rxjs that referenced this issue Mar 2, 2023
+ Removes `lift`, `source`, and `operator` from `Observable`
+ Updates related code

BREAKING CHANGE: `Observable#lift`, `Observable#source`, and `Observable#operator` is no longer a part of the API. These were never meant to be public and have been deprecated for more than 3 years.

Resolves ReactiveX#7201
@benlesh benlesh closed this as completed in e0bdccf Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.x Issues and PRs for version 8.x 9.x AGENDA ITEM Flagged for discussion at core team meetings
Projects
None yet
Development

No branches or pull requests

1 participant