albertogasparin / react-magnetic-di

Dependency injection and replacement for Javascript and React components/hooks
MIT License
131 stars 7 forks source link

Add support for `react-test-renderer` #62

Closed TimeRaider closed 1 year ago

TimeRaider commented 1 year ago

Dependency injection doesn't work properly when a component is being rendered via react-test-renderer:

import React, { useState } from 'react';
import { create } from 'react-test-renderer';
import { DiProvider, injectable } from 'react-magnetic-di';

import { Input, useTheme } from '../input';

const useThemeDi = injectable(useTheme, () => {
  return useState({ color: '#B00' });
});

describe('Input', () => {
    it('should render with theme', () => {
      const tree = create(
        <DiProvider use={[useThemeDi]}>
          <Input />
        </DiProvider>
      ).toJSON();
      expect(tree).toMatchInlineSnapshot(`
        <input
          placeholder="Type..."
          style={
            {
              "border": "1px solid #777", // <-- Wrong, should be #B00
            }
          }
        />
      `);
    });
  });
});

Supposedly due to how react-test-renderer's create works, the di consumer does not get a value from a Provider in https://github.com/albertogasparin/react-magnetic-di/blob/41949762bd5ba3682c4e12f5f35bf37ae72bcb50/src/react/consumer.js#L10 and ends up using a default implementation from https://github.com/albertogasparin/react-magnetic-di/blob/41949762bd5ba3682c4e12f5f35bf37ae72bcb50/src/react/context.js#L4-L6

Replacing const { getDependencies = (v) => v } = Context._currentValue || {} with const { getDependencies = (v) => v } = useContext(Context) works, but breaks the ability to use di in both functional and class components.

Would you consider a major version release to remove the hack in di function and providing di and useDi separately?

albertogasparin commented 1 year ago

Yeah, this is a known limitation with shallow rendering. Using useContext directly is a no go, because it internally checks if it’s running during the correct phase and if not it blows up. Also, removing support for class components is a bit of a shame as sometimes we are still required to use them (eg error boundaries). I’d much rather prefer finding a way to detect test renderer and do something special for it

TimeRaider commented 1 year ago

How about providing both di and useDi for explicit (but optional) use in class and functional components?

albertogasparin commented 1 year ago

I think I've found a solution to make it work. Can you please test it out (haven't published a beta but it's a 1 line change) and if working I'll make a new release 😊