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

Added Provider.FromValue, to enable mocking in Storybook and Unit Tests #127

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

scottrippey
Copy link

I have started using constate to separate my data-fetching logic from my UI, and it's perfect for the job! Thank you.

However, in Storybook and in Unit tests, I'd like to be able to manually provide (mock) values to the context provider.
So, in this PR, I've exposed a property on the Provider called Provider.FromValue, which is just a Provider that ignores the hook data and uses the provided data instead.

Storybook example:

const [CounterProvider, useCounterContext] = constate(useCounter);
export const withMockedValues = <>
  <CounterProvider.FromValue value={{ count: 0, increment: action('increment') }}>
    <Increment />
    <Count />
  </CounterProvider.FromValue>
</>;

React Testing Library example:

test("increment gets called", () => {
  const mockCounter = { count: 0, increment: jest.fn() };
  const MockProvider: React.FC = ({ children }) => (
    <CounterProvider.FromValue value={mockCounter}>
      {children}
    </CounterProvider.FromValue>
  );
  const { getByText } = render(
    <div>
      <Increment />
      <Count />
    </div>,
    { wrapper: MockProvider }
  );
  fireEvent.click(getByText("Increment"));
  expect(getByText("10")).toBeDefined();
  expect(mockCounter.increment).toHaveBeenCalledTimes(1);
});

I'm open to feedback, especially regarding naming things, so please let me know!

TODO:

  • Add FromValue feature
  • Add unit tests
  • Add documentation

@scottrippey scottrippey changed the title feat: Added Provider.FromValue, to enable easy dependency mocking Added Provider.FromValue, to enable mocking in Storybook and Unit Tests Jan 6, 2021
@narinluangrath
Copy link

@diegohaz Thoughts on this PR?

@luiznasciment0
Copy link

I’m facing exactly this same issue. Seems like a great solution!

@luiznasciment0
Copy link

@scottrippey hey, how are you solving this problem until this PR is not approved? Can you share a solution? :)

@scottrippey
Copy link
Author

@luiznasciment0 this whole lib is just 1 file, so I just copied my fork into my project.

@luiznasciment0
Copy link

lol hahaha awesome! actually that's a pretty good solution!

liuyunjs pushed a commit to liuyunjs/constate that referenced this pull request May 14, 2021
@diegohaz
Copy link
Owner

Thanks for working on this.

There are several ways to deal with this problem. For example, a more explicit approach would be accepting the overrides through the hook/Provider props:

function useCounter(props) {
  const [_count, _setCount] = useState(props.initialCount);
  const count = props.count ?? _count;
  const setCount = props.setCount || _setCount;

  const _increment = useCallback(() => setCount((prevCount) => prevCount + 1), [setCount]);
  const increment = props.increment || _increment;

  return { count, increment };
}

const [CounterProvider, useCounterContext] = constate(useCounter);

<CounterProvider count={0} increment={action("increment")} />

I haven't tried it, but I guess you can create a wrapper around constate to abstract this logic.

Anyway, I don't think this library should encourage mocking the state like that, which usually leads to implementation detail tests, which is the case of the increment gets called test example in the PR.

@codecov-commenter
Copy link

codecov-commenter commented May 17, 2021

Codecov Report

Merging #127 (87894df) into master (48ac889) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #127   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           31        34    +3     
  Branches         5         5           
=========================================
+ Hits            31        34    +3     
Impacted Files Coverage Δ
src/index.tsx 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48ac889...87894df. Read the comment docs.

@scottrippey
Copy link
Author

My intent with this PR is just to expose the Provider externally, so that it could be used for whatever purposes a user might need. I think there are many practical use-cases; in my case, it's extremely convenient for mocking and testing.

I don't think this makes constate any more complex, and apart from type improvements, this only required about 3 lines of code and has no runtime overhead. Please reconsider supporting this feature! Thanks :)

@scottrippey
Copy link
Author

FWIW, there is an alternative approach, similar to what you described. I've authored react-overridable-hooks which wraps hooks, making them overridable via an override Provider.

// Normal custom hook:
const useCounterHook = () => ...;

// Wrap it:
export const [ useCounter, CounterProvider ] = overridableHook(useCounterHook);

// Use it like normal:
const Counter = () => {  const { count, increment } = useCounter();  return <span>{count}</span> };

// Mock it in tests/storybook/etc:
const Test = <CounterProvider value={{ count: 5 }}>  <Counter /> </CounterProvider>

This plays nicely with constate, but ultimately, I could just be using constate if the Provider was exposed!

@janfabian
Copy link

Just came here looking for the exact solution @scottrippey is proposing in the PR.

As someone who inherited this library as dependency, I can't believe there is no other way to mock or inject the value into provider. The solution proposed by @diegohaz is just ... ugly.

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