-
Notifications
You must be signed in to change notification settings - Fork 53
feat(cmf): add common expressions #1069
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
Conversation
…nd/ui into jmfrancois/feat/cmf/add-expressions
| ); | ||
| } | ||
|
|
||
| export default curry(getInState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This super selectors seems odd and a little bit complex.
I have imagined selectors to be more precise (collectionSelectors, componentSelectors etc).
Not a big fan of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first version was written this way and we add exactly the same code with just 'collections' and 'components' difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you think immutable.js is and odd name for a file ?
It can be misleading and confusing with the library immutable.
| ); | ||
| } | ||
|
|
||
| export default curry(getInState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you think immutable.js is and odd name for a file ?
It can be misleading and confusing with the library immutable.
…nd/ui into jmfrancois/feat/cmf/add-expressions
What is the problem this PR is trying to solve?
We have no common expressions related to cmf so we must write code
What is the chosen solution to this problem?
provide default expressions:
TODO: Wait for #1055 to register in api.registerInternals();
Please check if the PR fulfills these requirements
[ ] This PR introduces a breaking change