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

fix: type inference of type #833

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

wSedlacek
Copy link

@wSedlacek wSedlacek commented Dec 1, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

When using ofType with more than one type it would try to collapse those down into a single type, ie A | B would become A if B was essentially the same as A but with a few extra properties. And if the types couldn't be collapsed because they were incompatible then you would just get an error requiring the generics to be manually filled int

Issue Number: N/A

What is the new behavior?

The ofType() can now property infer types of unions wether they are similar or disparate

Does this PR introduce a breaking change?

[x] Yes
[ ] No

Technically the generics of ofType() are changing so that the second parameter is an array rather than a union.
I think this is relatively low risk as most people would be inferring the type and if someone is providing the generic it is probably because it couldn't be inferred which this should fix

Other information

@@ -32,7 +39,7 @@ describe('operators/ofType', () => {
expectedResults.push(new A());

stream.next(new B());
stream.next(...expectedResults);
expectedResults.forEach((event) => stream.next(event));
Copy link
Author

Choose a reason for hiding this comment

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

This was failing tsc before because of

A spread argument must either have a tuple type or be passed to a rest parameter.

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

1 participant