Fix: [Section] Scalable Application Structure - #328
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive documentation guide for building a scalable React, Redux, and TypeScript application structure using feature-based grouping. The guide includes code examples for Redux slices, public API entry points, dynamic store configurations, and feature flags. The review feedback highlights a few discrepancies and improvements in the code examples: defining missing selectors in the slice example, correcting the import style of reducers in the store configuration to match their named exports, and typing the dynamically enabled reducers as Partial<typeof allReducers> instead of Record<string, any> to preserve TypeScript type safety.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export const { loginStart, loginSuccess, loginFailure, logout } = authSlice.actions; | ||
| export default authSlice.reducer; |
There was a problem hiding this comment.
The public API example (index.ts) in the next section attempts to export selectUser and selectIsAuthenticated from ./slices/authSlice. However, these selectors are not defined in authSlice.ts. Defining them here makes the example complete and functional.
| export const { loginStart, loginSuccess, loginFailure, logout } = authSlice.actions; | |
| export default authSlice.reducer; | |
| export const { loginStart, loginSuccess, loginFailure, logout } = authSlice.actions; | |
| export const selectUser = (state: { auth: AuthState }) => state.auth.user; | |
| export const selectIsAuthenticated = (state: { auth: AuthState }) => state.auth.isAuthenticated; | |
| export default authSlice.reducer; |
| import authReducer from '../features/auth'; | ||
| import productsReducer from '../features/products'; | ||
| import cartReducer from '../features/cart'; |
There was a problem hiding this comment.
In the public API pattern (index.ts), the reducers are exported as named exports (e.g., export { default as authReducer } from './slices/authSlice'). Importing them as default imports here will result in undefined at runtime. They should be imported as named imports instead.
| import authReducer from '../features/auth'; | |
| import productsReducer from '../features/products'; | |
| import cartReducer from '../features/cart'; | |
| import { authReducer } from '../features/auth'; | |
| import { productsReducer } from '../features/products'; | |
| import { cartReducer } from '../features/cart'; |
| const enabledReducers = Object.keys(allReducers).reduce((acc, key) => { | ||
| const featureKey = key as keyof typeof allReducers; | ||
| if (featureFlags[featureKey]) { | ||
| acc[featureKey] = allReducers[featureKey]; | ||
| } | ||
| return acc; | ||
| }, {} as Record<string, any>); |
There was a problem hiding this comment.
Typing enabledReducers as Record<string, any> causes Redux Toolkit's configureStore to lose type safety, resulting in RootState being inferred as Record<string, any>. Typing it as Partial<typeof allReducers> preserves TypeScript type safety for the store's state.
| const enabledReducers = Object.keys(allReducers).reduce((acc, key) => { | |
| const featureKey = key as keyof typeof allReducers; | |
| if (featureFlags[featureKey]) { | |
| acc[featureKey] = allReducers[featureKey]; | |
| } | |
| return acc; | |
| }, {} as Record<string, any>); | |
| const enabledReducers = Object.keys(allReducers).reduce((acc, key) => { | |
| const featureKey = key as keyof typeof allReducers; | |
| if (featureFlags[featureKey]) { | |
| acc[featureKey] = allReducers[featureKey]; | |
| } | |
| return acc; | |
| }, {} as Partial<typeof allReducers>); |
|
Hi @piotrwitek, this PR is ready for review and addresses an IssueHunt bounty. Please review and merge when you have a moment. Thank you! |
|
Closing as duplicate. The latest fix is in PR #338. Apologies for the noise — improving the automation. |
Fixes #42
This PR addresses the IssueHunt-funded issue #42.