Copy code from cra-template-redux and convert to TS by BenLorantfy · Pull Request #3 · reduxjs/redux-templates

BenLorantfy · 2020-02-21T04:56:14Z

Copies over the code from cra-template-redux and converts it to TS.

A few notes:

Not sure if I did the typescript correctly for the selector
I didn't bother copying over the jest/babel configuration. Figure we can set that up if/when we want to setup CI.

markerikson · 2020-02-21T05:00:51Z

      Learn how to configure a non-root public URL by running `npm run build`.
    -->
-    <title>React App</title>
+    <title>React Redux App</title>


Did we change the title of the JS one too?

yeah: https://github.com/reduxjs/cra-template-redux/blob/master/template/public/index.html#L27

nickserv · 2020-02-22T13:32:41Z

Sidenote: I think it would be easier to maintain this as a fork of cra-template-redux so we can pull updates from upstream, otherwise we'd have to duplicate them manually

markerikson · 2020-02-22T16:07:10Z

I'm not expecting the TS template to have that many changes over time, so I don't think we need to treat this as a true repo fork. If that proves wrong, we can rethink it.

markerikson · 2020-02-22T16:30:17Z

Just left some thoughts on improvements I want made to the JS template.

Pasting them here:

After trying this out, a few thoughts for tweaks:

It would be good to either show use of an inline selector with useSelector, or add a comment right above the useSelector saying you can define them inline

I'd like to tweak the slice comment slightly and say:
      // Redux Toolkit allows us write "mutating" logic in reducers. It doesn't 
      // actually mutate the state because it uses the immer library, which detects
      // changes to a "draft state" and produces a brand new immutable state
      // based off those changes
We should have an example of a thunk somewhere. Simplest example would be an "Add Async" button that sleeps for 1 second and then dispatches increment().

I want to move store.js into an app subfolder.

These changes should also be made to the TS template as well.

nickserv · 2020-02-22T13:47:59Z

+export function Counter() {
+  const count = useSelector(selectCount);
+  const dispatch = useDispatch();
+  const [incrementAmount, setIncrementAmount] = useState('2');


Should be a number, not a string

Suggested change

const [incrementAmount, setIncrementAmount] = useState('2');

const [incrementAmount, setIncrementAmount] = useState(2);

If I keep this a number, I get typescript errors when I use setIncrementAmount in the input below

Right, it needs to be converted from a string to a number somewhere. Either keep it as a string in the component and convert to a number right before you dispatch, or convert it to a number in the change handler and store it as a number.

I think it should be kept as a number in state, since that's what the data type represents logically.

If I keep it as a number in the state, it kind of messes with what the user can type into the textbox. For example, you can't backspace because it converts an empty string into "0".

Currently it gets converted to a number right before the dispatch.

I think we should change the JS template to be consistent. JS doesn't have an issue currently at run-time because it changes it to a string once the user starts typing.

Right, I want both templates consistent in terms of actual code as much as possible.

I wonder if it can be a number? to allow for the empty case

Or keep as string and use it to demonstrate simple validation

I think the simplest thing to do is just keep it as a string. Even number? will have problems, because it will restrict what the user can type, which could be confusing. e.g. I can't type 1. if I want to increment by a fractional number.

nickserv · 2020-02-22T15:41:13Z

 import counterReducer from './features/counter/counterSlice';

-export default configureStore({
+export const store = configureStore({


Suggested change

export const store = configureStore({

export default configureStore({

We should keep this as export default because its default import is used

I'd actually prefer to make it a named export in both templates.

Personally I don't think it makes sense since the file is named store and this is the main export. However, the other file would still need to be updated to fix the import.

Right, the import/export syntax needs to be consistent either way.

Overall, I'm just generally leaning away from default exports in most cases in my own code.

I dislike default exports because of the poor auto-complete experience, but that's just my opinion. We are using default exports in other files though (see App.tsx) so it might be good to use default export here for consistency? Not sure

I fixed the import bug though, good catch

I prefer default exports but I could go either way, I just wanted to point out that bug

@markerikson Do you care about the consistency here? I changed it to a named export but now it's inconsistent with some places where we do default exports

Such as https://github.com/reduxjs/cra-template-redux-typescript/pull/3/files#diff-9830f406f48f2a03610b26f834b0692dR58

Eh, let's leave App as is, and try to go for named exports pretty much everywhere else.

BenLorantfy · 2020-02-22T20:00:18Z

Just left some thoughts on improvements I want made to the JS template.

Should we make these changes in this PR or a separate one? Might be easier to make in a separate PR

BenLorantfy · 2020-02-22T20:04:46Z

@markerikson @nickmccurdy I think I replied/corrected to everything. Feel free to resolve whatever comments you're happy with so it's easier to see what's unresolved 😅

mamal72 · 2020-02-22T20:36:21Z

@BenLorantfy TS 3.8 is released and it looks like this project is depending on 3.7.2. I think it might be a good idea to update dependencies here too if you feel like to do it. 🤔

cc: @markerikson @nickmccurdy

nickserv · 2020-02-23T01:57:02Z

For some reason I can't mark my review comments as resolved in this pull request, but you can resolve what's been fixed so far

markerikson · 2020-02-23T00:02:57Z

+    decrement: state => {
+      state.value -= 1;
+    },
+    incrementByAmount: (state, action: PayloadAction<{ amount: number }>) => {


Question for both templates. Do we want to skip having a wrapper object, and just have the value itself be action.payload ?

I think this is a good example of Immer though

I'm just talking about the action.payload contents - the state.value += part is fine.

Oh yea, might as well just be a number

Might be a good idea but I wonder if we should do that in a separate follow-up PR that we do to both templates

It's nice to have an example of a wrapper object though, even if you wouldn't necessarily use one for this particular case. But you are trying to cut down on boilerplate with the toolkit, so I can see both ways

BenLorantfy · 2020-02-28T17:23:10Z

@markerikson @nickmccurdy I think I've fixed/responded to everything out-standing my-side. Let me know if I missed something.

markerikson · 2020-02-28T17:34:40Z

@BenLorantfy : are you planning to deal with the items in #3 (comment) in a follow-up PR, or this one?

BenLorantfy · 2020-02-28T17:36:19Z

I think it would make sense to do that stuff in a follow-up PR, since it needs to be done to both the TS and JS one. Either I or @lukyth could do it (since he expressed interest in #1)

markerikson · 2020-02-28T17:38:23Z

Okay, just want to make sure it's not being missed.

In that case, I think this is probably good atm.

nickserv · 2020-02-28T21:24:34Z

Sweet, is this ready to be released or are we waiting on the follow-up?

markerikson · 2020-02-28T21:37:30Z

Waiting for a follow-up per the last couple comments. Feel free to toss that one in :)

markerikson · 2020-03-02T15:10:47Z

Can someone go ahead and make the requested changes to both the TS and JS templates?

After trying this out, a few thoughts for tweaks:

It would be good to either show use of an inline selector with useSelector, or add a comment right above the useSelector saying you can define them inline

I'd like to tweak the slice comment slightly and say:
      // Redux Toolkit allows us write "mutating" logic in reducers. It doesn't 
      // actually mutate the state because it uses the immer library, which detects
      // changes to a "draft state" and produces a brand new immutable state
      // based off those changes
We should have an example of a thunk somewhere. Simplest example would be an "Add Async" button that sleeps for 1 second and then dispatches increment().

I want to move store.js into an app subfolder.

BenLorantfy · 2020-03-02T15:31:47Z

I can do it, I was just waiting in case someone else really wanted to

markerikson · 2020-03-02T15:40:56Z

Go for it :)

Copy code from cra-template-redux and convert to TS

BenLorantfy added 2 commits February 20, 2020 23:55

Copy code from cra-template-redux and convert to TS

cf374a0

Update file name in edit message

a49f6df

BenLorantfy requested a review from markerikson February 21, 2020 05:04

markerikson requested changes Feb 21, 2020

View reviewed changes

BenLorantfy added 4 commits February 22, 2020 09:36

Bump toolit version

867e6d8

Move counter state to explicit interface

485f789

Export RootState

c8efaa9

Run prettier

c60bd82

nickserv suggested changes Feb 22, 2020

View reviewed changes

BenLorantfy added 2 commits February 22, 2020 14:50

Add more keywords

d332851

Fix incorrect import

6fdf30a

BenLorantfy requested a review from markerikson February 22, 2020 20:02

BenLorantfy added 3 commits February 22, 2020 15:09

Move store to app/store

cab4f15

Fix imports

f8800c8

Fix feature import

e0c2e85

markerikson reviewed Feb 23, 2020

View reviewed changes

BenLorantfy added 3 commits February 22, 2020 21:19

Remove files that are auto-generated by CRA

128c6dd

Bump versions

1f0fd58

Bump versions in package.json

6c5bf30

nickserv mentioned this pull request Feb 25, 2020

Error: Cannot find module 'cra-template-redux-typescript' #5

Closed

BenLorantfy requested a review from markerikson February 28, 2020 17:23

markerikson merged commit b967476 into reduxjs:master Feb 28, 2020

This was referenced Mar 4, 2020

Add post-launch tweaks #6

Merged

Add post-launch tweaks reduxjs/cra-template-redux#10

Merged

HeatherD2025 referenced this pull request in HeatherD2025/QwRky Jan 23, 2026

Copy code from cra-template-redux and convert to TS (#3)

264fdf2

Copy code from cra-template-redux and convert to TS

	const [incrementAmount, setIncrementAmount] = useState('2');
	const [incrementAmount, setIncrementAmount] = useState(2);

	export const store = configureStore({
	export default configureStore({

Uh oh!

Conversation

BenLorantfy commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nickserv commented Feb 22, 2020

Uh oh!

markerikson commented Feb 22, 2020

Uh oh!

markerikson commented Feb 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenLorantfy Feb 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BenLorantfy commented Feb 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BenLorantfy commented Feb 22, 2020

Uh oh!

mamal72 commented Feb 22, 2020

Uh oh!

nickserv commented Feb 23, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenLorantfy commented Feb 21, 2020 •

edited

Loading

BenLorantfy Feb 22, 2020 •

edited

Loading

BenLorantfy commented Feb 22, 2020 •

edited

Loading

BenLorantfy commented Feb 28, 2020 •

edited

Loading