Skip to content

Commit 36dfb70

Browse files
jansavIku-turso
authored andcommitted
fix: Fix re-renders of component with di context provider causing unnecessary rerenders for children
Signed-off-by: Janne Savolainen <[email protected]>
1 parent 68ca5bb commit 36dfb70

File tree

9 files changed

+70
-74
lines changed

9 files changed

+70
-74
lines changed

packages/injectable/react/index.d.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,7 @@ export declare function getInjectableComponent<
4545
},
4646
): InjectableComponent<Component>;
4747

48-
interface DiContainerProviderProps {
49-
di: DiContainer | DiContainerForInjection;
50-
}
51-
52-
export const DiContextProvider: React.Provider<DiContainerProviderProps>;
48+
export const DiContextProvider: React.Provider<DiContainer | DiContainerForInjection>;
5349

5450
export interface WithInjectablesSyncOptions<
5551
Dependencies extends object,
@@ -96,5 +92,3 @@ export interface WithInjectables {
9692
}
9793

9894
export const withInjectables: WithInjectables;
99-
100-
export function registerInjectableReact(di: DiContainer): void;

packages/injectable/react/index.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ import withInjectables, {
22
DiContextProvider,
33
} from './src/withInjectables/withInjectables';
44

5-
import registerInjectableReact from './src/registerInjectableReact/registerInjectableReact';
6-
7-
export { withInjectables, DiContextProvider, registerInjectableReact };
5+
export { withInjectables, DiContextProvider };
86

97
export { getInjectableComponent } from './src/getInjectableComponent/getInjectableComponent';
108
export { useInject, useInjectDeferred } from './src/useInject/useInject';

packages/injectable/react/src/getInjectableComponent/getInjectableComponent.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { forwardRef, Suspense, useContext } from 'react';
1+
import React, { forwardRef, Suspense, useContext, useMemo } from 'react';
22

33
import { getInjectable, lifecycleEnum } from '@lensapp/injectable';
44
import { useInjectDeferred } from '../useInject/useInject';
@@ -34,11 +34,11 @@ export const getInjectableComponent = ({
3434

3535
const InjectableComponent = Object.assign(
3636
forwardRef((props, ref) => {
37-
const { di: failSafeDi } = useContext(diContext);
37+
const failSafeDi = useContext(diContext);
3838
const InjectedComponent = useInjectDeferred(InjectableComponent);
3939

4040
return (
41-
<DiContextProvider value={{ di: diForComponentContext || failSafeDi }}>
41+
<DiContextProvider value={diForComponentContext || failSafeDi}>
4242
{PlaceholderComponent ? (
4343
<Suspense fallback={<PlaceholderComponent {...props} />}>
4444
<InjectedComponent {...props} ref={ref} />

packages/injectable/react/src/getInjectableComponent/getInjectableComponent.test.jsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -539,15 +539,15 @@ describe('getInjectableComponent', () => {
539539

540540
const rendered = render(
541541
<div>
542-
<DiContextProvider value={{ di: di1 }}>
542+
<DiContextProvider value={di1}>
543543
<SomeComponent data-component-in-di-1-test />
544544
</DiContextProvider>
545545

546-
<DiContextProvider value={{ di: di2 }}>
546+
<DiContextProvider value={di2}>
547547
<SomeComponent data-component-in-di-2-test />
548548
</DiContextProvider>
549549

550-
<DiContextProvider value={{ di: di1 }}>
550+
<DiContextProvider value={di1}>
551551
<SomeComponent data-component-in-di-1-test />
552552
</DiContextProvider>
553553
</div>,
@@ -642,7 +642,7 @@ describe('getInjectableComponent', () => {
642642

643643
const rendered = render(
644644
<div>
645-
<DiContextProvider value={{ di }}>
645+
<DiContextProvider value={di}>
646646
<SomeComponent name="some-name" />
647647
</DiContextProvider>
648648
</div>,
@@ -660,7 +660,7 @@ describe('getInjectableComponent', () => {
660660
const mountFor = (di, onRenderingError) => node =>
661661
render(
662662
<ErrorBoundary onError={onRenderingError}>
663-
<DiContextProvider value={{ di }}>{node}</DiContextProvider>
663+
<DiContextProvider value={di}>{node}</DiContextProvider>
664664
</ErrorBoundary>,
665665
);
666666

packages/injectable/react/src/registerInjectableReact/registerInjectableReact.js

Lines changed: 0 additions & 5 deletions
This file was deleted.

packages/injectable/react/src/useInject/useInject.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const useNonDeferredValue = x => x;
99
const _useInject = (injectable, instantiationParameter, config) => {
1010
checkForUnsupportedAsyncTransient(injectable);
1111

12-
const { di } = useContext(diContext);
12+
const di = useContext(diContext);
1313

1414
const maybePromise = useMemo(
1515
() => di.inject(injectable, instantiationParameter),

packages/injectable/react/src/useInject/useInject.test.jsx

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { noop } from 'lodash/fp';
22
import { observable, runInAction } from 'mobx';
3-
import { Observer } from 'mobx-react';
3+
import { Observer, observer } from 'mobx-react';
44
import React, { Suspense, useEffect } from 'react';
55
import { act, render } from '@testing-library/react';
66

@@ -12,7 +12,6 @@ import {
1212
} from '@lensapp/injectable';
1313

1414
import asyncFn from '@async-fn/jest';
15-
import registerInjectableReact from '../registerInjectableReact/registerInjectableReact';
1615
import { DiContextProvider } from '../withInjectables/withInjectables';
1716
import { useInjectDeferred, useInject } from './useInject';
1817
import { flushPromises } from '@lensapp/ogre-test-utils';
@@ -29,11 +28,50 @@ describe('useInject', () => {
2928

3029
jest.spyOn(di, 'inject');
3130

32-
registerInjectableReact(di);
33-
3431
mount = mountFor(di, onErrorWhileRenderingMock);
3532
});
3633

34+
it('given parent component with context provider, when parent rerenders, does not cause unnecessary rerenders for childs', async () => {
35+
const someInjectable = getInjectable({
36+
id: 'some',
37+
instantiate: di => 'irrelevant',
38+
});
39+
40+
di.register(someInjectable);
41+
42+
const childComponentRenderMock = jest.fn();
43+
44+
const SomeChildComponentWithUseInject = () => {
45+
childComponentRenderMock();
46+
47+
useInject(someInjectable);
48+
49+
return <div>irrelevant</div>;
50+
};
51+
52+
const someObservable = observable.box(0);
53+
54+
const SomeParentComponentWithContextProvider = observer(({ children }) => {
55+
someObservable.get();
56+
57+
return <DiContextProvider value={di}>>{children}</DiContextProvider>;
58+
});
59+
60+
render(
61+
<SomeParentComponentWithContextProvider>
62+
<SomeChildComponentWithUseInject />
63+
</SomeParentComponentWithContextProvider>,
64+
);
65+
66+
await act(async () => {
67+
runInAction(() => {
68+
someObservable.set(1);
69+
});
70+
});
71+
72+
expect(childComponentRenderMock).toHaveBeenCalledTimes(1);
73+
});
74+
3775
describe('given a sync injectable, when rendered', () => {
3876
let rendered;
3977
let someRelatedPropState;
@@ -1682,7 +1720,7 @@ describe('useInject', () => {
16821720
const mountFor = (di, onRenderingError) => node =>
16831721
render(
16841722
<ErrorBoundary onError={onRenderingError}>
1685-
<DiContextProvider value={{ di }}>{node}</DiContextProvider>
1723+
<DiContextProvider value={di}>{node}</DiContextProvider>
16861724
</ErrorBoundary>,
16871725
);
16881726

packages/injectable/react/src/withInjectables/withInjectables.js

Lines changed: 15 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,13 @@
1-
import React, { useState } from 'react';
1+
import React, { useMemo, useState } from 'react';
22
import { constant } from 'lodash/fp';
33

44
import { isPromise } from '@lensapp/fp';
5-
import { getInjectable, lifecycleEnum } from '@lensapp/injectable';
6-
75
export const diContext = React.createContext();
86

97
const { Provider: DiContextProvider, Consumer: DiContextConsumer } = diContext;
108

119
export { DiContextProvider };
1210

13-
export const componentNameMapInjectable = getInjectable({
14-
id: 'component-name-map',
15-
instantiate: () => new Map(),
16-
});
17-
1811
const ComponentOrPlaceholder = ({
1912
di,
2013
nonDependencyProps,
@@ -23,10 +16,21 @@ const ComponentOrPlaceholder = ({
2316
Component,
2417
refProps,
2518
}) => {
19+
const diForComponentContext = useMemo(() => {
20+
return {
21+
...di,
22+
23+
inject: (alias, parameter) => di.inject(alias, parameter, {}),
24+
25+
injectMany: (injectionToken, parameter) =>
26+
di.injectMany(injectionToken, parameter, {}),
27+
};
28+
}, [di]);
29+
2630
const [propsState, setPropsState] = useState();
2731

2832
React.useLayoutEffect(() => {
29-
const maybeAsyncProps = getProps(di, nonDependencyProps);
33+
const maybeAsyncProps = getProps(diForComponentContext, nonDependencyProps);
3034

3135
if (isPromise(maybeAsyncProps)) {
3236
maybeAsyncProps.then(setPropsState);
@@ -45,40 +49,12 @@ const ComponentOrPlaceholder = ({
4549
export default (Component, { getPlaceholder = constant(null), getProps }) =>
4650
React.forwardRef((nonDependencyProps, ref) => (
4751
<DiContextConsumer>
48-
{({ di }) => {
49-
const componentNameMap = di.inject(componentNameMapInjectable);
50-
51-
if (!componentNameMap.has(Component)) {
52-
componentNameMap.set(
53-
Component,
54-
Component.displayName ||
55-
Component.name ||
56-
`anonymous-component-${componentNameMap.size}`,
57-
);
58-
}
59-
60-
const componentContext = {
61-
injectable: {
62-
id: componentNameMap.get(Component),
63-
lifecycle: lifecycleEnum.transient,
64-
},
65-
};
66-
67-
const diForComponentContext = {
68-
...di,
69-
70-
inject: (alias, parameter) =>
71-
di.inject(alias, parameter, componentContext),
72-
73-
injectMany: (injectionToken, parameter) =>
74-
di.injectMany(injectionToken, parameter, componentContext),
75-
};
76-
52+
{di => {
7753
const refProps = ref ? { ref } : {};
7854

7955
return (
8056
<ComponentOrPlaceholder
81-
di={diForComponentContext}
57+
di={di}
8258
nonDependencyProps={nonDependencyProps}
8359
getPlaceholder={getPlaceholder}
8460
getProps={getProps}

packages/injectable/react/src/withInjectables/withInjectables.test.jsx

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,12 @@ import withInjectables, { DiContextProvider } from './withInjectables';
1313
import asyncFn from '@async-fn/jest';
1414
import { observable, runInAction } from 'mobx';
1515
import { observer } from 'mobx-react';
16-
import registerInjectableReact from '../registerInjectableReact/registerInjectableReact';
1716
import { keys } from 'lodash/fp';
1817

19-
const flushPromises = () => new Promise(flushMicroTasks);
20-
2118
const mountFor =
2219
di =>
2320
(node, ...rest) =>
24-
render(<DiContextProvider value={{ di }}>{node}</DiContextProvider>, {
21+
render(<DiContextProvider value={di}>{node}</DiContextProvider>, {
2522
...rest,
2623
});
2724

@@ -32,8 +29,6 @@ describe('withInjectables', () => {
3229
beforeEach(() => {
3330
di = createContainer('some-container');
3431

35-
registerInjectableReact(di);
36-
3732
mount = mountFor(di);
3833
});
3934

0 commit comments

Comments
 (0)