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

Create slice changes #197

Merged
merged 1 commit into from Sep 23, 2019

Conversation

@phryneas
Copy link
Contributor

phryneas commented Sep 22, 2019

These are the changes you requested over in #82

@netlify

This comment has been minimized.

Copy link

netlify bot commented Sep 22, 2019

Deploy preview for redux-starter-kit-docs ready!

Built with commit c58aa2c

https://deploy-preview-197--redux-starter-kit-docs.netlify.com

@phryneas phryneas changed the base branch from master to v0.8-integration Sep 22, 2019
@markerikson

This comment has been minimized.

Copy link
Collaborator

markerikson commented Sep 23, 2019

Great, thanks!

Ironically, I'm actually having second thoughts about the "return the reducer" thing:

https://twitter.com/acemarke/status/1175881949514219521

It's an interesting idea, and yet at the same time I'm not sure it buys much actual improvement.

Thoughts?

@phryneas

This comment has been minimized.

Copy link
Contributor Author

phryneas commented Sep 23, 2019

It's definitely nicer to use (took my a while to figure out that I could not use the slice directly in combineReducers), but the console.log in chrome is really bad :/
(Firefox works though: https://imgur.com/mIipct5 )

What about turning the idea around? Export a modified combineReducers from RSK that takes either functions or slices?

@markerikson

This comment has been minimized.

Copy link
Collaborator

markerikson commented Sep 23, 2019

No, one of the main points of RSK is that it's "just Redux". I don't want to have a special version of combineReducers.

Y'know, I think I've talked myself out of returning the reducer directly. Returning the object gives us a bit more flexibility in case we ever do add something else to createSlice, and I don't think there's all that much benefit in returning the reducer by itself. Let's drop that and just do the slice? -> name change.

Sorry for the whiplash :)

@markerikson

This comment has been minimized.

Copy link
Collaborator

markerikson commented Sep 23, 2019

@phryneas : just re-read your comment in the 1.0 thread.

Yeah, sure, let's go ahead and include the individual reducers in the slice object (which is also another argument for keeping it as an object). Main question is what to call it? Maybe caseReducers?

Semi-recapping:

const sliceObj = createSlice({
    name: "todos",
    initialState,
    reducers: {
        addTodo() {},
        toggleTodo() {}
    }
});

console.log(sliceObj);
/*
{
    actions: {addTodo, toggleTodo},
    reducer : fn(),
    caseReducers: {addTodo, toggleTodo}
}
*/
@phryneas

This comment has been minimized.

Copy link
Contributor Author

phryneas commented Sep 23, 2019

@markerikson okay. I'll revert the "reducer function" commit so that this can get merged and open a separate PR for the caseReducers (I think that name is fine) in the coming days.

@phryneas phryneas force-pushed the phryneas:createSliceChanges branch from c5dabec to c58aa2c Sep 23, 2019
@markerikson

This comment has been minimized.

Copy link
Collaborator

markerikson commented Sep 23, 2019

One additional request: can we throw an error if name is falsy? That would force the user to add a non-empty name.

DOH! I should actually read the code, shouldn't I :)

@markerikson

This comment has been minimized.

Copy link
Collaborator

markerikson commented Sep 23, 2019

Awright, looks good to me!

@markerikson markerikson merged commit a685bce into reduxjs:v0.8-integration Sep 23, 2019
6 checks passed
6 checks passed
Header rules - redux-starter-kit-docs No header rules processed
Details
Pages changed - redux-starter-kit-docs 2 new files uploaded
Details
Redirect rules - redux-starter-kit-docs No redirect rules processed
Details
Mixed content - redux-starter-kit-docs No mixed content detected
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
netlify/redux-starter-kit-docs/deploy-preview Deploy preview ready!
Details
markerikson added a commit that referenced this pull request Oct 15, 2019
* Create slice changes (#197)

* Include case reducers in createSlice result (#209)

* Fix missing name field in type test

* Include provided case reducers in createSlice output

- Added `caseReducers` field to the createSlice return object
- Added test and type test for returned case reducer
- Removed "Enhanced" terminology from types, and replaced with
  "CaseReducerWithPrepare" and "CaseReducerDefinition"
- Restructured logic to only loop over reducer names once, and
  check case reducer definition type once per entry

* Add type tests for correct case reducer export inference

* Rewrite caseReducers types for correct inference of state + action

* Add type test for reducers with prepare callback

* Fix type inference for actions from reducers w/ prepare callbacks

* Clean up type names and usages

* Use a generic State type in ActionForReducer

* Re-switch Slice generics order

* Use `void` instead of `any` for undefined payloads (#174)

* Change default createAction payload type to void to make it required

* Fix createAction type tests per review

* Update tutorials for v0.8 (#213)

* Update basic tutorial with createSlice changes

* Update intermediate tutorial with createSlice changes

* Update advanced tutorial with createSlice changes

* Change existing commit links to point to reduxjs org repos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.