Skip to content

docs: demonstrate selector usage in playground - #352

Open
libracapitalinvestments-rgb wants to merge 1 commit into
piotrwitek:masterfrom
libracapitalinvestments-rgb:casharmy/fix-issue-109
Open

docs: demonstrate selector usage in playground#352
libracapitalinvestments-rgb wants to merge 1 commit into
piotrwitek:masterfrom
libracapitalinvestments-rgb:casharmy/fix-issue-109

Conversation

@libracapitalinvestments-rgb

Copy link
Copy Markdown

Add selector usage in playground connected components to demonstrate how to adapt global Redux state to feature-specific state shapes using exported selectors from feature modules. Closes #109

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces connected components for the counter and todo features, and refactors their respective selectors to use reselect for deriving state from the global RootState. Feedback highlights several critical TypeScript and runtime issues: FCCounter should be imported as a named export, and onDecrement should be removed from its mapDispatchToProps as it is not supported by the component's props. Additionally, the selectors need corrections to match the actual state shapes, specifically using reduxCounter instead of counter, todosFilter instead of filter, and completed instead of done for todo items.

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.

import { RootState } from '../store/root-reducer';
import { countersActions } from '../features/counters';
import { getCounter } from '../features/counters/selectors';
import FCCounter from '../components/fc-counter';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The component FCCounter is exported as a named export (export const FCCounter) in playground/src/components/fc-counter.tsx, not as a default export. You should use a named import here to avoid import errors.

Suggested change
import FCCounter from '../components/fc-counter';
import { FCCounter } from '../components/fc-counter';

Comment on lines +15 to +18
const mapDispatchToProps = {
onIncrement: countersActions.increment,
onDecrement: countersActions.decrement,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The FCCounter component's props do not include onDecrement. Passing onDecrement in mapDispatchToProps will cause a TypeScript compilation error when connecting the component.

const mapDispatchToProps = {
  onIncrement: countersActions.increment,
};

Comment on lines +21 to +24
export const getCounter = createSelector(
getCounters,
counters => counters.counter
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The state shape defined in CountersState has the property reduxCounter, not counter. Accessing counters.counter will result in a compilation error or undefined at runtime.

Suggested change
export const getCounter = createSelector(
getCounters,
counters => counters.counter
);
export const getCounter = createSelector(
getCounters,
counters => counters.reduxCounter
);

Comment on lines +26 to +29
export const getTodosFilter = createSelector(
getTodos,
todos => todos.filter
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The state shape defined in TodosState has the property todosFilter, not filter. Accessing todos.filter will return undefined instead of the filter value.

Suggested change
export const getTodosFilter = createSelector(
getTodos,
todos => todos.filter
);
export const getTodosFilter = createSelector(
getTodos,
todos => todos.todosFilter
);

Comment on lines +34 to +43
(todos, filter) => todos.filter(todo => {
switch (filter) {
case 'active':
return !todo.done;
case 'completed':
return todo.done;
default:
return true;
}
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The Todo items in the state use the property completed to track completion status, not done. Accessing todo.done will result in incorrect filtering behavior because it will be undefined.

Suggested change
(todos, filter) => todos.filter(todo => {
switch (filter) {
case 'active':
return !todo.done;
case 'completed':
return todo.done;
default:
return true;
}
})
(todos, filter) => todos.filter(todo => {
switch (filter) {
case 'active':
return !todo.completed;
case 'completed':
return todo.completed;
default:
return true;
}
})

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.

Selectors in the playground?

1 participant