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

Update rxjs peer dependency to include rxjs 7.x #103

Merged
merged 5 commits into from
Oct 23, 2023

Conversation

Igorbek
Copy link
Contributor

@Igorbek Igorbek commented Oct 23, 2023

Now both focal and focal-atom have peer dependency to rxjs < 7. However the library totally works fine with [email protected]. this PR updates peer dependency to now include [email protected].

As part of the change, dev dependencies have also been updated to rx@7 to test with the latest rxjs.

In order to check the changes are compatible with [email protected] here's PR #104 which has the same changes but doesn't update rxjs.

@@ -371,7 +380,13 @@ class LensedAtom<TSource, TDest> extends AbstractAtom<TDest> {
private _refCount = 0

// Rx method overrides
_subscribe(subscriber: Subscriber<TDest>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rxjs@7 https://rxjs.dev/6-to-7-change-summary#observable

_subscribe method is no longer public and is now marked @internal.

@@ -383,7 +398,7 @@ class LensedAtom<TSource, TDest> extends AbstractAtom<TDest> {
this._subscription = null
}
})
sub.add(super._subscribe(subscriber))
sub.add(super.subscribe(...(args as [Partial<Observer<TDest>>])))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to make TS happy

@@ -744,7 +744,7 @@ describe('atom', () => {
describe('fromObservable', () => {
test('emits atom', async () => {
const a = await Atom.fromObservable(from([1])).pipe(take(1)).toPromise()
expect(a.get()).toEqual(1)
expect(a?.get()).toEqual(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rxjs@7 https://rxjs.dev/6-to-7-change-summary#observable

toPromise method now correctly returns Promise<T | undefined> instead of Promise<T>. This is a correction without a runtime change, because if the observable does not emit a value before completion, the promise will resolve with undefined.

@Igorbek Igorbek merged commit b6f927e into grammarly:master Oct 23, 2023
1 check passed
@Igorbek Igorbek deleted the f-up-rx branch October 23, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants