Skip to content
This repository has been archived by the owner on Jul 2, 2021. It is now read-only.

Add some check on EmitterSubscription's listener #25

Open
em-yuanbo opened this issue Aug 16, 2016 · 3 comments
Open

Add some check on EmitterSubscription's listener #25

em-yuanbo opened this issue Aug 16, 2016 · 3 comments

Comments

@em-yuanbo
Copy link

https://github.com/facebook/emitter/blob/master/src/EmitterSubscription.js#L32
there is no validation on the param listener

  constructor(subscriber: EventSubscriptionVendor, listener, context: ?Object) {
    super(subscriber);
    this.listener = listener;
    this.context = context;
  }

and no appliable check when apply the listener.
https://github.com/facebook/emitter/blob/master/src/BaseEventEmitter.js#L182

  __emitToSubscription(subscription, eventType) {
    var args = Array.prototype.slice.call(arguments, 2);
    subscription.listener.apply(subscription.context, args);
  }

can we add some checks or make some noises when create a new EmitterSubscription?

@kyldvs
Copy link
Contributor

kyldvs commented Aug 16, 2016

The file has typechecks which will complain if you pass anything other than a function to it. We should also probably add flow types too, but I do not believe we should modify runtime behavior to verify it's a function.

@em-yuanbo
Copy link
Author

The typechecks or the flow types only make sense when you compile this source code. But most time the libs we used are installed from npm and compiled, and all the types or checks are missed. by then the condition is different.

@kyldvs
Copy link
Contributor

kyldvs commented Aug 17, 2016

Yeah that will be true of typechecks probably, but for flow we can provide external library definitions. Doesn't matter if the module is transpiled or not

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

2 participants