[react] New useReducer signature #7420
Conversation
threepointone
changed the title
|
(updating tests, will update PR soon) EDIT: done. |
tests/react/useReducer_hook.js
Outdated
|
|
||
| const [state, dispatch] = React.useReducer(reducer, initialState, initialAction); | ||
| const [state, dispatch] = React.useReducer(reducer, { type: "reset", payload: 123 }, init); |
There was a problem hiding this comment.
Not super important, but why does the init arg in this example have a type and payload? Implies that it's the same type as the action, but that's not necessarily true.
There was a problem hiding this comment.
No reason, just translated what was already there. I’ll change it. I specifically wanted it to not have the signature of the state, but I should have gone with something totally different. Thx!
|
I've added (as a separate commit) a bunch of .diff files that got generated. I"m honestly not sure whether that's expected. |
nmote
added
the
Library definitions
lib/react.js
Outdated
| init: void, | ||
| ): [S, Dispatch<A>]; | ||
|
|
||
| declare export function useReducer<S, I, A>( |
There was a problem hiding this comment.
Is this the best ordering of the generics for when you specify them explicitly or would <S, A, I> be better so that the action type line up in the same place? Is there any practical difference?
There was a problem hiding this comment.
I don't think there's a practical difference, happy to align it. If I squint, I could make the argument that an initialArg would always 'occur' in source before any dispatch calls, so it's chronologically sound?
There was a problem hiding this comment.
It seems like the TypeScript defs need S and A to line up in a separate type for some obscure reason and therefore adds I at the end. So switching it would line up with TS.
There was a problem hiding this comment.
@threepointone has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| declare export function useReducer<S, A>( | ||
| reducer: (S, A) => S, | ||
| initialState: S, | ||
| init: void, |
There was a problem hiding this comment.
How is this type overload different from the previous one?
There was a problem hiding this comment.
Otherwise Flow will error on useReducer(reducer, state, undefined); because the argument count is different than what’s expected.
|
cc @jbrown215 This PR, #7430, and #7440 are all minor adjustments to things in the upcoming 16.8 release. Should we merge them into one PR to reduce the number of snapshot test conflicts? |
|
@bvaughn: That or stacking them will work! In either case, they'll need to be re-recorded. Will review these tomorrow |
React is updating the signature for the
useReducerhook facebook/react#14723 This PR reflects those changes. We'll also update the official react docs soon.I'm doing a build and tests right now locally (installing everything from scratch atm), but figured I'd start the PR as well.