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

add Redux Starter Kit #961

Merged
merged 6 commits into from Aug 19, 2019
Merged

add Redux Starter Kit #961

merged 6 commits into from Aug 19, 2019

Conversation

TylerShin
Copy link
Collaborator

No description provided.

@TylerShin TylerShin self-assigned this Aug 16, 2019
return state;
}
}
type SetDeviceAction = PayloadAction<{ userDevice: UserDevice }>;
Copy link

Choose a reason for hiding this comment

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

Can you use this type to type Dispatch? How does that work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you can't use it directly inside of Dispatch type definition. you should make the custom wrapper type definition to cover the strict bond between the component and the action creators.

However, I think we won't need to make the Dispatch definition anymore.
The official doc said that use export const { setDeviceType } = layoutSlice.actions; with connect and mapDispatchToProps.
If you follow the doc, you don't need the definition of action types and dispatch.

If you don't want to use connect API and want to handle it with hooks, I think you should make the custom type definition. (I omit it)

Mark(RSK maintainer) said he will update the type definition and use case with hooks API soon.

Copy link

Choose a reason for hiding this comment

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

That's true. Since the creators and reducers are created together by the same library, the library guarantees the correctness. So we don't need to type them.

};
}
const DeviceDetector: React.FC = () => {
const dispatch = useDispatch();
Copy link

Choose a reason for hiding this comment

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

Hooks 👍

case ACTION_TYPES.SET_DEVICE_TO_DESKTOP: {
return { ...state, userDevice: UserDevice.DESKTOP };
}
export enum UserDevice {
Copy link

Choose a reason for hiding this comment

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

FYI, I recently learned how to use enums within ambient types. Let me know if you think it would help you!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure. it would be helpful!

Copy link

Choose a reason for hiding this comment

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

Assuming your webpack is set up correctly, we just need to use the const enum.

Since d.ts files are not included in the bundle, a regular enum in a d.ts, which gets compiled to an object, will disappear with the rest of the d.ts file. But a const enum gets compiled to constant values (like 1) so the bundle will work regardless.

Copy link

Choose a reason for hiding this comment

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

I mention webpack because, in my case, the order of ts and js loaders somehow mattered. But you only use a TS loader, so it should be okay 🤞

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice. thanks to ur comment, I've read https://www.typescriptlang.org/docs/handbook/enums.html#const-enums
I'll apply it soon. thanks for your attention!

# Conflicts:
#	package-lock.json
@TylerShin TylerShin merged commit 8403681 into master Aug 19, 2019
@TylerShin TylerShin deleted the dev/RSK branch August 19, 2019 06:20
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.

None yet

2 participants