Fix TSDoc comments #3560
Fix TSDoc comments #3560
Conversation
src/createStore.ts
Outdated
| @@ -257,11 +257,7 @@ export default function createStore<S, A extends Action, Ext, StateExt>( | |||
| throw new Error('Expected the nextReducer to be a function.') | |||
| } | |||
|
|
|||
| // TODO: do this more elegantly | |||
There was a problem hiding this comment.
See line 84 for the "more elegant" approach, which allows any reducer to be assigned to currentReducer. It's all that can be done, really.
test/applyMiddleware.spec.ts
Outdated
| @@ -51,16 +51,11 @@ describe('applyMiddleware', () => { | |||
| const spy = jest.fn() | |||
| const store = applyMiddleware(test(spy), thunk)(createStore)(reducers.todos) | |||
|
|
|||
| // the typing for redux-thunk is super complex, so we will use an as unknown hack | |||
There was a problem hiding this comment.
This is why I installed redux-thunk as a dev dependency. I think it makes sense to ensure its compatibility here, seeing as the tests are using thunks.
test/applyMiddleware.spec.ts
Outdated
| @@ -51,16 +51,11 @@ describe('applyMiddleware', () => { | |||
| const spy = jest.fn() | |||
| const store = applyMiddleware(test(spy), thunk)(createStore)(reducers.todos) | |||
There was a problem hiding this comment.
This store was not being typed properly, because the thunk wasn't typed properly. That's now fixed in this PR.
test/applyMiddleware.spec.ts
Outdated
| return dispatchedValue.then(() => { | ||
| expect(spy.mock.calls.length).toEqual(2) | ||
| }) | ||
| await store.dispatch(addTodoAsync('Use Redux')) |
There was a problem hiding this comment.
Now that the store is typed properly, this dispatch call has no issues. The root of the problem has been fixed.
test/createStore.spec.ts
Outdated
| @@ -505,6 +505,7 @@ describe('createStore', () => { | |||
|
|
|||
| it('throws if action type is missing', () => { | |||
| const store = createStore(reducers.todos) | |||
| // @ts-ignore | |||
| expect(() => store.dispatch({})).toThrow( | |||
There was a problem hiding this comment.
TypeScript won't let you call store.dispatch with a missing action type, now that the store is typed properly; thus, we need to // @ts-ignore this line to allow the test to run w/o compilation errors.
test/createStore.spec.ts
Outdated
| @@ -519,10 +520,10 @@ describe('createStore', () => { | |||
|
|
|||
| it('does not throw if action type is falsy', () => { | |||
| const store = createStore(reducers.todos) | |||
| expect(() => store.dispatch({ type: false })).not.toThrow() | |||
| expect(() => store.dispatch({ type: 0 })).not.toThrow() | |||
| expect(() => store.dispatch({ type: false } as any)).not.toThrow() | |||
There was a problem hiding this comment.
Again, because the store is properly typed, TS won't compile this, so I'm just casting as any to allow it to compile. Pretty common to see as any usage in tests. In fact, I usually change my .eslintrc to allow any in test files.
test/helpers/middleware.ts
Outdated
|
|
||
| export function thunk({ dispatch, getState }: MiddlewareAPI) { | ||
| return (next: Dispatch) => <T>(action: ThunkAction) => | ||
| export const thunk: Middleware< |
There was a problem hiding this comment.
Unfortunately, using ThunkMiddleware kept returning a type of any for some reason, so I resorted to this redux Middleware type instead, which gives us a properly-typed thunk. Worth looking into in the redux-thunk library why this is happening, but outside the scope of this PR.
|
We're still in the middle of the conversion. We want to:
So, this PR is going to conflict for a bit while we get finished with step 2. I'd hold off until #3536 closes, at which point we'll be ready for improvements. |
jednano
force-pushed
the
tsdoc-types
branch
4 times, most recently
from
b684953
to
180e93e
Compare
|
@timdorr I've limited this PR in scope to just TSDoc comments. I'll hold off for the other stuff. |
No description provided.