Skip to content

Commit 8643381

Browse files
jansavjansav
authored
Merge pull request #16 from lensapp/fix/di-match-confusion
fix: Fix react memoization causing incorrect di-container when nested…
2 parents 0be2389 + 6fbd8f0 commit 8643381

File tree

3 files changed

+24
-61
lines changed

3 files changed

+24
-61
lines changed

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import asyncFn from '@async-fn/jest';
22
import { noop } from 'lodash/fp';
33
import React, { forwardRef, Suspense } from 'react';
44
import { render } from '@testing-library/react';
5-
65
import {
76
createContainer,
87
getInjectable,
@@ -503,8 +502,8 @@ describe('getInjectableComponent', () => {
503502
});
504503

505504
it('given different dis, and same injectable component, when rendered in one di, another one, and first one again, the component remains di-specific', () => {
506-
const di1 = createContainer('some-container-1', { detectCycles: false });
507-
const di2 = createContainer('some-container-2', { detectCycles: false });
505+
const di1 = createContainer('some-container-1');
506+
const di2 = createContainer('some-container-2');
508507

509508
const someInjectionToken = getInjectionToken({
510509
id: 'some-token',
@@ -573,7 +572,7 @@ describe('getInjectableComponent', () => {
573572
});
574573

575574
it('given component which has itself as children, and finally an unregistered component as children, when rendered, throws full path', () => {
576-
const di = createContainer('some-container-1', { detectCycles: false });
575+
const di = createContainer('some-container-1');
577576

578577
const SomeComponent = getInjectableComponent({
579578
id: 'some-injectable-component',
@@ -612,7 +611,7 @@ describe('getInjectableComponent', () => {
612611
});
613612

614613
it('given an injectable component with some placeholder that accepts some props > when the component suspense > shows the placeholder with the original props', () => {
615-
const di = createContainer('some-container-1', { detectCycles: false });
614+
const di = createContainer('some-container-1');
616615
const someFn = asyncFn();
617616

618617
const someAsyncValueInjectable = getInjectable({

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@ const _useInject = (injectable, instantiationParameter, config) => {
1111

1212
const di = useContext(diContext);
1313

14-
const maybePromise = useMemo(
15-
() => di.inject(injectable, instantiationParameter),
16-
[injectable, instantiationParameter],
17-
);
14+
const maybePromise = di.inject(injectable, instantiationParameter);
1815

1916
checkForUnsupportedPromiseLikeTransient(injectable, maybePromise);
2017

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

Lines changed: 19 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,11 @@ describe('useInject', () => {
156156
});
157157
});
158158

159-
it('does not inject', () => {
160-
expect(di.inject).not.toHaveBeenCalled();
159+
it('reinjects', () => {
160+
expect(di.inject).toHaveBeenCalledWith(
161+
someRelatedSyncInjectable,
162+
'some-initial-prop-value',
163+
);
161164
});
162165

163166
it('renders', () => {
@@ -449,14 +452,6 @@ describe('useInject', () => {
449452
expect(actuallySuspended).toBe(false);
450453
});
451454

452-
it('calls to inject the new async injectable', () => {
453-
expect(
454-
di.inject.mock.calls.filter(
455-
([injectable]) => injectable === someAsyncInjectable,
456-
),
457-
).toEqual([[someAsyncInjectable, 'some-fast-prop-value']]);
458-
});
459-
460455
it('still renders as having the old async value', () => {
461456
expect(rendered.baseElement).toMatchInlineSnapshot(`
462457
<body>
@@ -1274,18 +1269,6 @@ describe('useInject', () => {
12741269
expect(actuallySuspended).toBe(false);
12751270
});
12761271

1277-
it('calls to inject the async injectable for each component', () => {
1278-
expect(
1279-
di.inject.mock.calls.filter(
1280-
([injectable]) => injectable === someAsyncInjectable,
1281-
),
1282-
).toEqual([
1283-
[someAsyncInjectable, 'some-new-prop-value'],
1284-
[someAsyncInjectable, 'some-new-prop-value'],
1285-
[someAsyncInjectable, 'some-new-prop-value'],
1286-
]);
1287-
});
1288-
12891272
it('still renders as having the old async values', () => {
12901273
expect(rendered.baseElement).toMatchInlineSnapshot(`
12911274
<body>
@@ -1319,20 +1302,20 @@ describe('useInject', () => {
13191302

13201303
it('renders as having the new async value', () => {
13211304
expect(rendered.baseElement).toMatchInlineSnapshot(`
1322-
<body>
1323-
<div>
1324-
<div
1325-
data-some-related-prop-test="some-new-async-value"
1326-
/>
1327-
<div
1328-
data-some-related-prop-test="some-new-async-value"
1329-
/>
1330-
<div
1331-
data-some-related-prop-test="some-new-async-value"
1332-
/>
1333-
</div>
1334-
</body>
1335-
`);
1305+
<body>
1306+
<div>
1307+
<div
1308+
data-some-related-prop-test="some-new-async-value"
1309+
/>
1310+
<div
1311+
data-some-related-prop-test="some-new-async-value"
1312+
/>
1313+
<div
1314+
data-some-related-prop-test="some-new-async-value"
1315+
/>
1316+
</div>
1317+
</body>
1318+
`);
13361319
});
13371320
});
13381321
});
@@ -1481,22 +1464,6 @@ describe('useInject', () => {
14811464
expect(actuallySuspended).toBe(false);
14821465
});
14831466

1484-
it('calls to inject the async injectable', () => {
1485-
expect(
1486-
di.inject.mock.calls.filter(
1487-
([injectable]) =>
1488-
injectable === someAsyncInjectableWithCompositeKey,
1489-
),
1490-
).toEqual([
1491-
[
1492-
someAsyncInjectableWithCompositeKey,
1493-
{
1494-
somePartOfCompositeKey: 'some-new-part-of-composite-key',
1495-
},
1496-
],
1497-
]);
1498-
});
1499-
15001467
it('still renders as having the old async values', () => {
15011468
expect(rendered.baseElement).toMatchInlineSnapshot(`
15021469
<body>

0 commit comments

Comments
 (0)