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

feat(cmf): add two state static accessors #1371

Merged
merged 18 commits into from
May 28, 2018
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions output/cmf.eslint.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@
The react/require-extension rule is deprecated. Please use the import/extensions rule from eslint-plugin-import instead.

/home/travis/build/Talend/ui/packages/cmf/src/cmfConnect.js
237:11 error 'displayName' is not defined no-undef
238:11 error 'propTypes' is not defined no-undef
242:11 error 'contextTypes' is not defined no-undef
40:8 error 'actionCreator' is defined but never used no-unused-vars
41:8 error 'component' is defined but never used no-unused-vars
42:8 error 'CONST' is defined but never used no-unused-vars
43:8 error 'expression' is defined but never used no-unused-vars
242:11 error 'displayName' is not defined no-undef
243:11 error 'propTypes' is not defined no-undef
247:11 error 'contextTypes' is not defined no-undef
253:11 error 'getState' is not defined no-undef
256:11 error 'setStateAction' is not defined no-undef

/home/travis/build/Talend/ui/packages/cmf/src/componentState.js
87:3 warning Unexpected console statement no-console
Expand All @@ -26,5 +32,5 @@ The react/require-extension rule is deprecated. Please use the import/extensions
/home/travis/build/Talend/ui/packages/cmf/src/sagas/collection.js
10:1 error Prefer default export import/prefer-default-export

12 problems (9 errors, 3 warnings)
18 problems (15 errors, 3 warnings)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm some cleaning maybe ;)

37 changes: 37 additions & 0 deletions packages/cmf/__tests__/cmfConnect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,11 @@ describe('cmfConnect', () => {

describe('Higher Order Component', () => {
const Button = ({ onClick, label }) => <button onClick={onClick}>{label}</button>;
Button.propTypes = {
onClick: PropTypes.func,
label: PropTypes.string,
};
Button.displayName = 'Button';
const CMFConnectedButton = cmfConnect({})(Button);
it('should create a connected component', () => {
const TestComponent = jest.fn();
Expand All @@ -208,6 +213,38 @@ describe('cmfConnect', () => {
expect(wrapper.props()).toMatchSnapshot();
});

it('should expose getState static function to get the state', () => {
expect(typeof CMFConnectedButton.getState).toBe('function');
const state = mock.state();
state.cmf.components = fromJS({
Button: {
default: { foo: 'bar' },
other: { foo: 'baz' },
},
});
expect(CMFConnectedButton.getState(state).get('foo')).toBe('bar');
expect(CMFConnectedButton.getState(state, 'other').get('foo')).toBe('baz');
});
it('should expose setStateAction static function to get the redux action to setState', () => {
expect(typeof CMFConnectedButton.setStateAction).toBe('function');
const state = new Map({ foo: 'bar' });
let action = CMFConnectedButton.setStateAction(state);
expect(action).toEqual({
type: 'Button.setState',
cmf: {
componentState: {
componentName: 'Button',
componentState: state,
key: 'default',
type: 'REACT_CMF.COMPONENT_MERGE_STATE',
},
},
});
action = CMFConnectedButton.setStateAction(state, 'foo', 'MY_ACTION');
expect(action.type).toBe('MY_ACTION');
expect(action.cmf.componentState.key).toBe('foo');
});

it('should support no context in dispatchActionCreator', () => {
const TestComponent = props => <div className="test-component" {...props} />;
TestComponent.displayName = 'TestComponent';
Expand Down
16 changes: 16 additions & 0 deletions packages/cmf/src/cmfConnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes';
import { connect } from 'react-redux';
import omit from 'lodash/omit';
import api from './api';
import actions from './actions';
import deprecated from './deprecated';
import onEvent from './onEvent';
import { initState, getStateAccessors, getStateProps } from './componentState';
Expand Down Expand Up @@ -245,6 +246,21 @@ export default function cmfConnect({
router: PropTypes.object,
};
static WrappedComponent = WrappedComponent;
static getState = function getState(state, id = 'default') {
return state.cmf.components.getIn([getComponentName(WrappedComponent), id], defaultState);
};
static setStateAction = function setStateAction(state, id = 'default', type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

state parameter to be renamed newState i think
also shouldn't those state update operation also support synchronous state update function ? for safe state update operation when relying on previous state ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

newState already exists in the upper declaration
I have added the asyn state update support

return {
type: type || `${getComponentName(WrappedComponent)}.setState`,
cmf: {
componentState: actions.components.mergeState(
getComponentName(WrappedComponent),
id,
state,
),
},
};
};

constructor(props, context) {
super(props, context);
Expand Down
26 changes: 26 additions & 0 deletions packages/cmf/src/cmfConnect.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,32 @@ If you want to render some component conditionally, just pass "renderIf" prop (t
You can also use Expression for this and customize this prop like "renderIfExpression" in
CMF json configuration files

## How to access state and mutate from the outside
Copy link
Contributor

Choose a reason for hiding this comment

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

title may be misleading, the state is not mutated in sense of pointer value mutation, but it is updated
to avoid confusion i would rename this title to
## Read and update component state from outside


Every cmfConnected component expose two static function:
* getState
* setStateAction

So if we take back the `Clock` example from below and we try to write a saga:

```javascript
import Clock from './Clock.connect';

export default function* myDeLorean() {
const clockState = yield select(Clock.getState);
const action = Clock.setStateAction(clockState.set('date', new Date('2025/12/25')));
yield put(action);
}
```

If you have multiple instance of the same component those api support `id` as a second argument.

```javascript
const componentState = Clock.getState(state, 'a-component-id');
// mutation
Clock.setStateAction(componentState, 'a-component-id');
```

## How to test


Expand Down
2 changes: 1 addition & 1 deletion packages/cmf/src/componentState.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class MyComponent extends React.Component {
export default cmfConnect({})(MyComponent);
*/

export function getStateProps(state, name, id) {
export function getStateProps(state, name, id = 'default') {
return {
state: state.cmf.components.getIn([name, id]),
};
Expand Down