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: fix a typing mismatch in FromEventObservable #3536

Merged

Conversation

joemphilips
Copy link
Contributor

Description:
see #3530

Related issue (if exists):
changes made here is exactly the same with #3530 except the file path.

@rxjs-bot
Copy link

rxjs-bot commented Apr 9, 2018

Messages
📖

CJS: 2278.2KB, global: 749.0KB (gzipped: 120.6KB), min: 145.8KB (gzipped: 31.2KB)

Generated by 🚫 dangerJS

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.401% when pulling 512669d on joemphilips:fix-typemismatch-fromEvent-stable into faa8302 on ReactiveX:stable.

@benlesh
Copy link
Member

benlesh commented Apr 9, 2018

I'm not sure about this one, as stable targets TS 2.0, and we don't want to break people. I'll need to confirm this change before I can merge. :\

@cartant
Copy link
Collaborator

cartant commented Apr 10, 2018

@benlesh To me, it looks like this should be fine, as this code is fine for TypeScript 2.0.10:

type EmitterTypeWithFunction = {
  addListener: (eventName: string, handler: Function) => void;
  removeListener: (eventName: string, handler: Function) => void;
}

type HandlerWithArgs = (...args: any[]) => void;
type EmitterTypeWithArgs = {
  addListener: (eventName: string, handler: HandlerWithArgs) => void;
  removeListener: (eventName: string, handler: HandlerWithArgs) => void;
}

const emitterWithFunction = {
  addListener(eventName: string, handler: Function) {},
  removeListener(eventName: string, handler: Function) {}
}

const emitterWithArgs = {
  addListener(eventName: string, handler: (...args: any[]) => void) {},
  removeListener(eventName: string, handler: (...args: any[]) => void) {}
}

const e1: EmitterTypeWithFunction = emitterWithFunction;
const e2: EmitterTypeWithArgs = emitterWithFunction;
const e3: EmitterTypeWithFunction = emitterWithArgs;
const e4: EmitterTypeWithArgs = emitterWithArgs;

Function will match (...args: any[]) => void and vice versa.

TypeScript 2.6 introduced the strictFunctionTypes compiler option. If that option - or the strict option - is set to true, the assignment to e3 will effect this error:

Type 'Function' provides no match for the signature '(...args: any[]): void'.

So this PR fixes a problem for users of the stable release that are themselves using TypeScript 2.6+ with either strict or strictFunctionTypes enabled.

@benlesh benlesh merged commit 21a59a7 into ReactiveX:stable May 4, 2018
@benlesh
Copy link
Member

benlesh commented May 4, 2018

Thanks, @joemphilips! And thanks for the additional review @cartant!

@lock
Copy link

lock bot commented Jun 5, 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 5, 2018
@joemphilips joemphilips deleted the fix-typemismatch-fromEvent-stable branch June 6, 2018 06:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants