Skip to content

Conversation

tony-go
Copy link
Contributor

@tony-go tony-go commented Jul 7, 2021

Hi 👋

This pull request follow the typescript migration initiative #1737

Not 100% confident about type accuracy, but it's good start.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 7, 2021

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.

Latest deployment of this branch, based on commit eb87735:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

@netlify
Copy link

netlify bot commented Jul 7, 2021

✔️ Deploy Preview for react-redux-docs ready!

🔨 Explore the source changes: eb87735

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-redux-docs/deploys/60e6a1e131049a0008824880

😎 Browse the preview: https://deploy-preview-1760--react-redux-docs.netlify.app


export function whenMapDispatchToPropsIsFunction(mapDispatchToProps) {
export function whenMapDispatchToPropsIsFunction(
mapDispatchToProps: ActionCreatorsMapObject | ActionCreator<any>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✋ It won't ever be a single action creator. It's either an object with action creators inside, a function that accepts dispatch and returns the bound action creators object, or nothing.

You might be able to use the types I pasted into selectorFactory.ts in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put a FixTypeLater, I'll try to crack it today.


export function whenMapDispatchToPropsIsMissing(mapDispatchToProps) {
export function whenMapDispatchToPropsIsMissing(
mapDispatchToProps: ActionCreatorsMapObject | ActionCreator<any>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✋ Here, it's probably just undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shoul I let the !mapDispatchToProps check ?

// and a state object in some others (ex: whenMapStateToPropsIsMissing)
// eslint-disable-next-line no-unused-vars
getConstant: (dispatch: Dispatch) => { dispatch?: Dispatch }
getConstant: (dispatch: Dispatch) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✋ This should probably be reworked to be more generic and flexible - maybe like:

getConstant: <T>(value: T) => ( () => T)

In other words, whatever you pass in, it returns a function that returns that thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't match with the usage of wrapMapToPropsConstant made by whenMapDispatchToPropsIsMissing:

 wrapMapToPropsConstant((dispatch: Dispatch) => ({
      dispatch,
 }))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, it's not just a passthrough

@markerikson
Copy link
Contributor

This is probably good enough for the moment. Thanks!

@markerikson markerikson merged commit 147f178 into reduxjs:typescript-port Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants