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: draft implementation of useStore in #733 (not production ready) #734

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/fluxible-addons-react/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export { default as createElementWithContext } from './createElementWithContext'
export { default as provideContext } from './provideContext';
export { default as useFluxible } from './useFluxible';
export { default as withFluxible } from './withFluxible';
export { default as useStore } from './useStore';
export { default as useExecuteAction } from './useExecuteAction';
22 changes: 22 additions & 0 deletions packages/fluxible-addons-react/src/useExecuteAction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import useFluxible from './useFluxible'

/**
* React hook that returns an executeAction handler
* TODO: this is a draft for an ongoing discussion in #733
*
* Example:
*
* const FooComponent = () => {
* const executeAction = useExecuteAction();
* return <p id={foo} onClick={() => excuteAction(...)} />;
* };
*
* @function useExecuteAction
* @returns {Function} - executeAction handler
*/
const useExecuteAction = () => {
const { executeAction } = useFluxible();
return executeAction;
}

export default useExecuteAction;
38 changes: 38 additions & 0 deletions packages/fluxible-addons-react/src/useStore.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import {useEffect, useState, useLayoutEffect} from 'react';
import useFluxible from './useFluxible'

/**
* React hook that returns a state from Fluxible store.
* TODO: this is a draft for an ongoing discussion in #733
*
* Example:
*
* const FooComponent = () => {
* const foo = useStore('FooStore', store => store.getFoo());
* return <p id={foo} />;
* };
*
* @function useStore
* @returns {object} - a state from Fluxible store
*/
const useStore = (storeName, getStateFromStore) => {
const { getStore } = useFluxible();
const store = getStore(storeName);
const [state, setState] = useState(getStateFromStore(store));

function updateState() {
setState(getStateFromStore(store));
}

// useLayoutEffect is the closest to componentDidMount
// (we want to block render until store is subscribed)
// TODO: NOTE useLayoutEffect is called on server-side during SSR
useLayoutEffect(() => {
Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to use useLayoutEffect and not just useEffect?

Copy link
Author

Choose a reason for hiding this comment

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

The original test would fail for useEffect. I think that the following would take place with useEffect:

  1. render and return
  2. Test runs assertions on the return
  3. Actions in useEffect is triggered
    This causes the assertions in 2 to fail.

With useLayoutEffect:

  1. Render
  2. Actions in useLayoutEffect is executed and waited to be finished
  3. Test runs assertions on the return

Basically, it is just to ensure the same behavior as connectToStore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Could you please take a look how react-redux is handling this issue: https://blog.isquaredsoftware.com/2018/11/react-redux-history-implementation/#connect-is-implemented-using-hooks
They are also using useLayoutEffect but on SSR they use useEffect to prevent the warning message.

Copy link
Contributor

Choose a reason for hiding this comment

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

store.on('change', updateState);
return () => store.removeListener('change', updateState);
}, []);

return state;
}

export default useStore;
89 changes: 89 additions & 0 deletions packages/fluxible-addons-react/tests/unit/lib/useStore.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import React, { useState } from 'react';
import { expect } from 'chai';
import TestRenderer from 'react-test-renderer';
import createMockComponentContext from 'fluxible/utils/createMockComponentContext';
import { useStore, FluxibleProvider, useExecuteAction, FluxibleComponent } from '../../../';
import FooStore from '../../fixtures/stores/FooStore';
import BarStore from '../../fixtures/stores/BarStore';

const DumbComponent = () => {

const foo = useStore(FooStore, store => store.getFoo());
const bar = useStore(BarStore, store => store.getBar());
const executeAction = useExecuteAction();
const onClick = () => executeAction((context) => context.dispatch('DOUBLE_UP'))

return (
<div>
<span id="foo">{foo}</span>
<span id="bar">{bar}</span>
<button id="button" onClick={onClick} />
</div>
)
};

DumbComponent.displayName = 'DumbComponent';
DumbComponent.initAction = () => {};

const stores = [FooStore, BarStore];

const renderComponent = (Component) => {
const context = createMockComponentContext({ stores });

const app = TestRenderer.create(
<FluxibleComponent context={context}>
<Component ref={undefined} />
</FluxibleComponent>
);

return { app, context };
};

describe('fluxible-addons-react', () => {
describe('useStore', () => {
it('returns fluxible store', () => {
const FooComponent = () => {
const foo = useStore('FooStore', store => store.getFoo())
return <p id={foo} />;
};

const context = createMockComponentContext({ stores: [FooStore] });

const testRenderer = TestRenderer.create(
<FluxibleProvider context={context}>
<FooComponent />
</FluxibleProvider>
);

const component = testRenderer.root.findByType('p');

expect(component.props.id).to.deep.equal('bar');
});
it('should register/unregister from stores on mount/unmount', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make it simple to navigate between code blocks, I would recommend you to add one blank line between each test.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, I can do that. The IDE handles most of the reformatting though, so I might approach it by adding a lint rule.

const { app, context } = renderComponent(DumbComponent);

const barStore = context.getStore(BarStore);
const fooStore = context.getStore(FooStore);

expect(barStore.listeners('change').length).to.equal(1);
expect(fooStore.listeners('change').length).to.equal(1);

app.unmount();

expect(barStore.listeners('change').length).to.equal(0);
expect(fooStore.listeners('change').length).to.equal(0);
});
it('should listen to store changes', () => {
const { app } = renderComponent(DumbComponent);
const button = app.root.findByProps({ id: 'button' })
const foo = app.root.findByProps({ id: 'foo' })
const bar = app.root.findByProps({ id: 'bar' })

button.props.onClick();
console.error(foo)

expect(foo.props.children).to.equal('barbar');
expect(bar.props.children).to.equal('bazbaz');
});
});
});