reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.93k stars 15.27k forks source link

fix(types): getState is not bound to MiddlewareAPI #4737

Closed janeklb closed 2 months ago

janeklb commented 2 months ago

PR Type

Does this PR add a new feature, or fix a bug?

Fixes a type bug

Why should this PR be included?

The getState function provided by MiddlewareAPI is not bound to any object, and as such it can be used freely as in the examples.

The MiddlewareAPI type does not make this explicit, and typescript linters can report this as problematic usage.

Checklist

Bug Fixes

What is the current behavior, and the steps to reproduce the issue?

What is the expected behavior?

getState can be used with the linting rule enabled and not produce errors

How does this PR fix the problem?

Fixes the signature of getState

codesandbox-ci[bot] commented 2 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

timdorr commented 2 months ago

This should have some sort of test to go along with it. That will prevent a regression in the future.

janeklb commented 2 months ago

This should have some sort of test to go along with it. That will prevent a regression in the future.

I'm able to create a test if we revert to

export interface MiddlewareAPI<D extends Dispatch = Dispatch, S = any> {
  dispatch: D
  getState(this: void): S
}

however, with the current signature (getState: () => S), I can't find a way to do it. @EskiMojo14 can you advise?

janeklb commented 2 months ago

This should have some sort of test to go along with it. That will prevent a regression in the future.

I'm able to create a test if we revert to

export interface MiddlewareAPI<D extends Dispatch = Dispatch, S = any> {
  dispatch: D
  getState(this: void): S
}

however, with the current signature (getState: () => S), I can't find a way to do it. @EskiMojo14 can you advise?

Never mind -- found something that covers the bases, I think.

timdorr commented 2 months ago

Thanks!