Skip to content

Feature implementation from commits a4310a8..0c2afc9 #2

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

Open
wants to merge 25 commits into
base: feature-base-2
Choose a base branch
from

Conversation

yashuatla
Copy link
Owner

This PR contains changes from a range of commits from the original repository.

Commit Range: a4310a8..0c2afc9
Files Changed: 72 (30 programming files)
Programming Ratio: 41.7%

Commits included:

piotrwitek and others added 25 commits February 3, 2019 19:18
* Update dependencies and react types and simplified use reducer example

* Updated playground code to be up to date with updated librariesFixed piotrwitek#138

* Updated readme

* Updated contributors
Fixes piotrwitek#140 

Let me know if it looks good or I need to make any changes. Thanks!
* Added reference example for testing of epics

* updated hoc typings
On my Linux system, the playground example did not compile right away, because `Home` was imported, but the file was called `home.tsx`.
@@ -1,13 +1,11 @@
import * as countersConstants from './constants';
import * as countersActions from './actions';
import countersReducer, { CountersState, CountersAction } from './reducer';
import countersReducer from './reducer';
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Removed type exports may break dependent code.

Removing CountersState and CountersAction exports could cause compilation errors in any code that imports these types from this barrel file.

Current Code (Diff):

- import countersReducer from './reducer';
+ import countersReducer, { CountersState, CountersAction } from './reducer';
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
import countersReducer from './reducer';
import countersReducer, { CountersState, CountersAction } from './reducer';

import * as todosSelectors from './selectors';
export { todosActions, todosSelectors, todosReducer, TodosState, TodosAction };
export { todosActions, todosSelectors, todosReducer };
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Breaking API Change: Removed Type Exports.

Removing TodosState and TodosAction types from public exports will break any code that imports these types from this module.

Current Code (Diff):

- export { todosActions, todosSelectors, todosReducer };
+ export { todosActions, todosSelectors, todosReducer, TodosState, TodosAction };
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
export { todosActions, todosSelectors, todosReducer };
export { todosActions, todosSelectors, todosReducer, TodosState, TodosAction };

@@ -1,5 +1,5 @@
// public API
import * as todosActions from './actions';
import todosReducer, { TodosState, TodosAction } from './reducer';
import todosReducer from './reducer';
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Missing Type Imports for Public API.

The TodosState and TodosAction types need to be imported if they are to be exported in the public API.

Current Code (Diff):

- import todosReducer from './reducer';
+ import todosReducer, { TodosState, TodosAction } from './reducer';
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
import todosReducer from './reducer';
import todosReducer, { TodosState, TodosAction } from './reducer';

@@ -20,5 +20,3 @@ const logAddAction: Epic<TodosAction, Types.RootState, Types.Services> = (
}),
ignoreElements()
);
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Critical Export Removed.

The export statement for the logAddAction epic has been removed, which will break any code that imports this epic and cause runtime errors in the Redux middleware.

Current Code (Diff):

+ export default combineEpics(logAddAction);
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
);
export default combineEpics(logAddAction);

@@ -1,13 +1,6 @@
// public API
import * as todosConstants from './constants';
import * as todosActions from './actions';
import todosReducer, { TodosState, TodosAction } from './reducer';
import todosReducer from './reducer';
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Breaking API Change: Removed Type Exports.

Removing TodosState and TodosAction types from the public API will break any code that imports these types from this module.

Current Code (Diff):

- import todosReducer from './reducer';
+ import todosReducer, { TodosState, TodosAction } from './reducer';
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
import todosReducer from './reducer';
import todosReducer, { TodosState, TodosAction } from './reducer';

TodosState,
TodosAction,
};
export { todosConstants, todosActions, todosSelectors, todosReducer };
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Breaking API Change: Removed Type Exports.

Removing TodosState and TodosAction types from the exports will break any code that imports these types from this module.

Current Code (Diff):

- export { todosConstants, todosActions, todosSelectors, todosReducer };
+ export { todosConstants, todosActions, todosSelectors, todosReducer, TodosState, TodosAction };
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
export { todosConstants, todosActions, todosSelectors, todosReducer };
export { todosConstants, todosActions, todosSelectors, todosReducer, TodosState, TodosAction };

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.

6 participants