Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Action creators from createSlice are not inferred as PayloadActionCreator #157

Closed
markerikson opened this issue Jul 15, 2019 · 6 comments
Closed

Comments

@markerikson
Copy link
Collaborator

I'm finally getting a chance to use RSK myself in a real project, and I just ran into an interesting issue.

In createAction(), we specifically attach the generated type to the generated action creator so that we can use the action creator itself as the type value, by both overriding toString() and adding it as actionCreator.type = type.

I've got some code I just wrote that looks like:

const someSlice = createSlice({
  /* omit */
});

const {doSomething} = someSlice.actions;

// later, in a middleware
switch(action.type) {
    case doSomething.type: { // error: Property 'type' does not exist on type '(payload: SomePayload) => PayloadAction<SomePayload, string>'

    }
}

So, even though we're using createAction() inside of createSlice, there's a disconnect in the types.

In our TS code, we specifically have a PayloadActionCreator type that appears to define itself as "a function with this payload, plus the type field".

However, I don't see that PayloadActionCreator type being used in createSlice anywhere. Instead, there's a SliceActionCreator type that reuses the PayloadAction definition.

Seems like SliceActionCreator ought to be referring to PayloadActionCreator somehow?

@markerikson
Copy link
Collaborator Author

/cc @denisw, @Jessidhia, @Dudeonyx , @phryneas :

any ideas?

@markerikson
Copy link
Collaborator Author

I just tried specifically hand-editing the typings for SliceActionCreator in my local app codebase node_modules to be:

export declare type SliceActionCreator<P> = P extends void ? PayloadActionCreator<void> : PayloadActionCreator<P>;

And that seems to compile okay.

@phryneas
Copy link
Member

Those two ActionCreators seem to have diverged on the zero-argument-case - even though they actually are exactly the same internally.
Drives me nuts since forever, actually :) (See my comment here: #149 (comment))

I guess I'll try to make those behave similarly when I touch them for #149 anyways :)

@markerikson
Copy link
Collaborator Author

Meh. I tried to poke around at this some myself right now, but don't have time since I'm at work.

What I'm seeing is that my suggested change breaks the expected test failure in createSlice.types.ts:

  // typings:expect-error
  counter.actions.multiply()

Which I think is verifying that if the reducer is defined as not taking an action arg, that the action creator rejects attempts to call it without an argument.

I also added this test, which fails without with the change but passes with it:

  const incrementType = counter.actions.increment.type

@phryneas, could you do me a favor and try to resolve just this one issue for now and pick up with the payload customization thing later?

@phryneas
Copy link
Member

phryneas commented Jul 15, 2019

Which I think is verifying that if the reducer is defined as not taking an action arg, that the action creator rejects attempts to call it without an argument.

I guess adding this behaviour to the PayloadActionCreator would be okay?
It would break some tests but I guess it's what was intended in the first place anyways.

@phryneas, could you do me a favor and try to resolve just this one issue for now and pick up with the payload customization thing later?

Sure thing :)

@markerikson
Copy link
Collaborator Author

Fixed via #158 .

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

No branches or pull requests

2 participants