search

angular-redux / ng-redux

Angular bindings for Redux
MIT License
1.16k stars 177 forks source link

Changing subscribe's return type in Reducer #173

Closed dawidzq closed 6 years ago

dawidzq commented 6 years ago

Changed the return type of subscribe(listener: Function) since redux defines it as "Unsubscribe". Although using Function works, it may result in TypeScript errors in certain use cases.

http://redux.js.org/docs/api/Store.html#subscribe

dawidzq commented 6 years ago

Just to elaborate,

I am proposing this PR because I am getting typescript issues when trying to use another package, specifically redux-persist. 

  Types of property 'subscribe' are incompatible.
    Type '(listener: Function) => Function' is not assignable to type '(listener: () => void) => Unsubscribe'.
      Type 'Function' is not assignable to type 'Unsubscribe'.
        Type 'Function' provides no match for the signature '(): void'.

I originally thought it was an issue with the redux-persist code base, but after doing some digging, I noticed that redux itself has an Unsubscribe data type that is declared as:

export interface Unsubscribe {
  (): void;
}

This is obviously a Function-like definition and seems to fall in line with the behavior of the subscribe function (being that it returns an unsubscribe function when called). I went into the ng-redux code and made the same change inside my project that I am proposing in this PR, and it didn't seem to break anything while simultaneously resolving the above Typescript issue. I can't imagine importing Unsubscribe from redux being a problem either, since redux is a dependency for ng-redux.

I'm fairly new to the industry, so I would greatly appreciate any feedback if I'm not understanding something, but I'm fairly confident that this is a harmless fix to something that could be affecting other packages.