Skip to content

Commit 1d1cdcc

Browse files
committed
fix(hooks): dynamic update scope issue
dynamic update useAyanami’s Scope didn’t update ayanami instance
1 parent 03de486 commit 1d1cdcc

File tree

4 files changed

+57
-24
lines changed

4 files changed

+57
-24
lines changed

src/core/scope/__test__/scope.spec.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,24 @@ describe('Scope spec:', () => {
3333

3434
scopes.forEach((scope) => {
3535
class Test {}
36-
expect(getInstanceWithScope(Test, { scope })).toBeInstanceOf(Test)
36+
expect(getInstanceWithScope(Test, scope)).toBeInstanceOf(Test)
3737
})
3838
})
3939

4040
it('should always return same instance if same scope', () => {
4141
const scope = 'scope'
4242
class Test {}
4343

44-
expect(getInstanceWithScope(Test, { scope })).toBe(getInstanceWithScope(Test, { scope }))
44+
expect(getInstanceWithScope(Test, scope)).toBe(getInstanceWithScope(Test, scope))
4545
})
4646

4747
it('should always return new instance if scope is TransientScope', () => {
4848
class Test {}
4949

50-
expect(getInstanceWithScope(Test, { scope: TransientScope })).toBeInstanceOf(Test)
50+
expect(getInstanceWithScope(Test, TransientScope)).toBeInstanceOf(Test)
5151

5252
expect(
53-
getInstanceWithScope(Test, { scope: TransientScope }) ===
54-
getInstanceWithScope(Test, { scope: TransientScope }),
53+
getInstanceWithScope(Test, TransientScope) === getInstanceWithScope(Test, TransientScope),
5554
).toBeFalsy()
5655
})
5756

@@ -76,10 +75,10 @@ describe('Scope spec:', () => {
7675
}
7776

7877
it('should return same instance whether scope is same or not', () => {
79-
const b = getInstanceWithScope(B, { scope: 'b' })
80-
const c1 = getInstanceWithScope(C, { scope: 'c1' })
81-
const c2 = getInstanceWithScope(C, { scope: 'c2' })
82-
const d = getInstanceWithScope(D, { scope: 'c2' })
78+
const b = getInstanceWithScope(B, 'b')
79+
const c1 = getInstanceWithScope(C, 'c1')
80+
const c2 = getInstanceWithScope(C, 'c2')
81+
const d = getInstanceWithScope(D, 'c2')
8382

8483
expect(b.a).toBeInstanceOf(A)
8584
expect(b.a).toBe(c1.a)
@@ -104,17 +103,17 @@ describe('Scope spec:', () => {
104103

105104
it('should return same instance if is same scope', () => {
106105
const scope = 'scope'
107-
const b = getInstanceWithScope(B, { scope })
108-
const c = getInstanceWithScope(C, { scope })
106+
const b = getInstanceWithScope(B, scope)
107+
const c = getInstanceWithScope(C, scope)
109108

110109
expect(b.a).toBeInstanceOf(A)
111110
expect(b.a).toBe(c.a)
112111
})
113112

114113
it('should return different instance if is different scope', () => {
115-
const b = getInstanceWithScope(B, { scope: 'b' })
116-
const c1 = getInstanceWithScope(C, { scope: 'c1' })
117-
const c2 = getInstanceWithScope(C, { scope: 'c2' })
114+
const b = getInstanceWithScope(B, 'b')
115+
const c1 = getInstanceWithScope(C, 'c1')
116+
const c2 = getInstanceWithScope(C, 'c2')
118117

119118
expect(b.a).toBeInstanceOf(A)
120119
expect(c1.a).toBeInstanceOf(A)

src/core/scope/index.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { InjectableFactory, ValueProvider } from '@asuka/di'
2-
import { get } from 'lodash'
32

43
import { ConstructorOf } from '../types'
54
import { ScopeConfig } from './type'
@@ -12,13 +11,14 @@ export const TransientScope = Symbol('scope:transient')
1211

1312
export const SingletonScope = Symbol('scope:singleton')
1413

15-
export function getInstanceWithScope<T>(constructor: ConstructorOf<T>, config?: ScopeConfig): T {
16-
const scope = get(config, 'scope', SingletonScope)
17-
14+
export function getInstanceWithScope<T>(
15+
constructor: ConstructorOf<T>,
16+
scope: ScopeConfig['scope'] = SingletonScope,
17+
): T {
1818
const providers = getSameScopeInjectionParams(constructor).map(
1919
(sameScopeInjectionParam): ValueProvider => ({
2020
provide: sameScopeInjectionParam,
21-
useValue: getInstanceWithScope(sameScopeInjectionParam, { scope }),
21+
useValue: getInstanceWithScope(sameScopeInjectionParam, scope),
2222
}),
2323
)
2424

src/hooks.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,19 @@ export function useAyanamiInstance<M extends Ayanami<S>, S>(
2121
ayanami: M,
2222
config?: UseAyanamiInstanceConfig,
2323
): HooksResult<M, S> {
24+
const ayanamiRef = React.useRef(ayanami)
2425
const ikari = React.useMemo(() => combineWithIkari(ayanami), [ayanami])
2526
const [state, setState] = React.useState<S>(() => ayanami.getState())
2627

28+
if (ayanamiRef.current !== ayanami) {
29+
ayanamiRef.current = ayanami
30+
setState(ayanami.getState())
31+
}
32+
2733
React.useEffect(() => {
2834
const subscription = ayanami.getState$().subscribe(setState)
2935
return () => subscription.unsubscribe()
30-
}, [])
36+
}, [ayanami])
3137

3238
React.useEffect(
3339
() => () => {
@@ -37,7 +43,7 @@ export function useAyanamiInstance<M extends Ayanami<S>, S>(
3743
ayanami.destroy()
3844
}
3945
},
40-
[],
46+
[ayanami, config],
4147
)
4248

4349
return [state, ikari.triggerActions] as HooksResult<M, S>
@@ -47,12 +53,12 @@ export function useAyanami<M extends Ayanami<S>, S>(
4753
A: ConstructorOf<M>,
4854
config?: ScopeConfig,
4955
): M extends Ayanami<infer SS> ? HooksResult<M, SS> : HooksResult<M, S> {
50-
const ayanami = React.useMemo(() => getInstanceWithScope(A, config), [A])
56+
const scope = get(config, 'scope')
57+
const ayanami = React.useMemo(() => getInstanceWithScope(A, scope), [scope])
5158

5259
const useAyanamiInstanceConfig = React.useMemo((): UseAyanamiInstanceConfig => {
53-
const scope = get(config, 'scope', false)
5460
return { destroyWhenUnmount: scope === TransientScope }
55-
}, [])
61+
}, [scope])
5662

5763
return useAyanamiInstance<M, S>(ayanami, useAyanamiInstanceConfig) as any
5864
}

test/specs/hooks.spec.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,5 +185,33 @@ describe('Hooks spec:', () => {
185185
expect(spy.mock.calls.length).toBe(1)
186186
})
187187
})
188+
189+
describe('Dynamic update scope', () => {
190+
const testRenderer = create(<CountComponent scope={1} />)
191+
const count = () => testRenderer.root.findByType('span').children[0]
192+
const click = (action: CountAction) =>
193+
act(() => testRenderer.root.findByProps({ id: action }).props.onClick())
194+
195+
it(`should use same Ayanami at each update if scope didn't change`, () => {
196+
testRenderer.update(<CountComponent scope={1} />)
197+
click(CountAction.ADD)
198+
expect(count()).toBe('1')
199+
})
200+
201+
it(`should use new scope's Ayanami if scope changed`, () => {
202+
testRenderer.update(<CountComponent scope={2} />)
203+
click(CountAction.MINUS)
204+
expect(count()).toBe('-1')
205+
})
206+
207+
it(`should update state to corresponding one`, () => {
208+
testRenderer.update(<CountComponent scope={1} />)
209+
expect(count()).toBe('1')
210+
testRenderer.update(<CountComponent scope={2} />)
211+
expect(count()).toBe('-1')
212+
testRenderer.update(<CountComponent scope={3} />)
213+
expect(count()).toBe('0')
214+
})
215+
})
188216
})
189217
})

0 commit comments

Comments
 (0)