Skip to content

Commit 4ded71a

Browse files
Zaimwa9Zaimwa9
committed
fix: surface-init-error-in-react-wrappers
1 parent 545390f commit 4ded71a

File tree

2 files changed

+136
-116
lines changed

2 files changed

+136
-116
lines changed

‎react.tsx‎

Lines changed: 97 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -1,145 +1,129 @@
1-
import React, {
2-
createContext,
3-
FC,
4-
useCallback,
5-
useContext,
6-
useEffect,
7-
useMemo,
8-
useRef,
9-
useState,
10-
} from 'react';
1+
import React, { createContext, FC, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react';
112
import Emitter from './utils/emitter';
123
const events = new Emitter();
134

14-
import { IFlagsmith, IFlagsmithTrait, IFlagsmithFeature, IState } from './types'
5+
import { IFlagsmith, IFlagsmithTrait, IFlagsmithFeature, IState } from './types';
156

16-
export const FlagsmithContext = createContext<IFlagsmith<string,string> | null>(null)
7+
export const FlagsmithContext = createContext<IFlagsmith<string, string> | null>(null);
178
export type FlagsmithContextType = {
18-
flagsmith: IFlagsmith // The flagsmith instance
19-
options?: Parameters<IFlagsmith['init']>[0] // Initialisation options, if you do not provide this you will have to call init manually
20-
serverState?: IState
9+
flagsmith: IFlagsmith; // The flagsmith instance
10+
options?: Parameters<IFlagsmith['init']>[0]; // Initialisation options, if you do not provide this you will have to call init manually
11+
serverState?: IState;
2112
children: React.ReactNode;
22-
}
13+
};
2314

24-
export const FlagsmithProvider: FC<FlagsmithContextType> = ({
25-
flagsmith, options, serverState, children,
26-
}) => {
27-
const firstRenderRef = useRef(true)
15+
export const FlagsmithProvider: FC<FlagsmithContextType> = ({ flagsmith, options, serverState, children }) => {
16+
const firstRenderRef = useRef(true);
2817
if (flagsmith && !flagsmith?._trigger) {
2918
flagsmith._trigger = () => {
3019
// @ts-expect-error using internal function, consumers would never call this
31-
flagsmith.log("React - trigger event received")
20+
flagsmith?.log('React - trigger event received');
3221
events.emit('event');
33-
}
22+
};
3423
}
3524

3625
if (flagsmith && !flagsmith?._triggerLoadingState) {
3726
flagsmith._triggerLoadingState = () => {
3827
events.emit('loading_event');
39-
}
28+
};
4029
}
4130

4231
if (serverState && !flagsmith.initialised) {
43-
flagsmith.setState(serverState)
32+
flagsmith.setState(serverState);
4433
}
4534

4635
if (firstRenderRef.current) {
47-
firstRenderRef.current = false
36+
firstRenderRef.current = false;
4837
if (options) {
49-
flagsmith.init({
50-
...options,
51-
state: options.state || serverState,
52-
onChange: (...args) => {
53-
if (options.onChange) {
54-
options.onChange(...args)
55-
}
56-
},
57-
})
38+
flagsmith
39+
.init({
40+
...options,
41+
state: options.state || serverState,
42+
onChange: (...args) => {
43+
if (options.onChange) {
44+
options.onChange(...args);
45+
}
46+
},
47+
})
48+
.catch((error) => {
49+
// @ts-expect-error using internal function, consumers would never call this
50+
flagsmith?.log('React - Failed to initialize flagsmith', error)
51+
events.emit('event');
52+
});
5853
}
5954
}
60-
return (
61-
<FlagsmithContext.Provider value={flagsmith}>
62-
{children}
63-
</FlagsmithContext.Provider>
64-
)
65-
}
55+
return <FlagsmithContext.Provider value={flagsmith}>{children}</FlagsmithContext.Provider>;
56+
};
6657

6758
const useConstant = function <T>(value: T): T {
68-
const ref = useRef(value)
59+
const ref = useRef(value);
6960
if (!ref.current) {
70-
ref.current = value
61+
ref.current = value;
7162
}
72-
return ref.current
73-
}
74-
63+
return ref.current;
64+
};
7565

7666
const flagsAsArray = (_flags: any): string[] => {
7767
if (typeof _flags === 'string') {
78-
return [_flags]
68+
return [_flags];
7969
} else if (typeof _flags === 'object') {
8070
// eslint-disable-next-line no-prototype-builtins
8171
if (_flags.hasOwnProperty('length')) {
82-
return _flags
72+
return _flags;
8373
}
8474
}
85-
throw new Error(
86-
'Flagsmith: please supply an array of strings or a single string of flag keys to useFlags',
87-
)
88-
}
75+
throw new Error('Flagsmith: please supply an array of strings or a single string of flag keys to useFlags');
76+
};
8977

9078
const getRenderKey = (flagsmith: IFlagsmith, flags: string[], traits: string[] = []) => {
9179
return flags
9280
.map((k) => {
93-
return `${flagsmith.getValue(k)}${flagsmith.hasFeature(k)}`
94-
}).concat(traits.map((t) => (
95-
`${flagsmith.getTrait(t)}`
96-
)))
97-
.join(',')
98-
}
81+
return `${flagsmith.getValue(k)}${flagsmith.hasFeature(k)}`;
82+
})
83+
.concat(traits.map((t) => `${flagsmith.getTrait(t)}`))
84+
.join(',');
85+
};
9986

10087
export function useFlagsmithLoading() {
10188
const flagsmith = useContext(FlagsmithContext);
10289
const [loadingState, setLoadingState] = useState(flagsmith?.loadingState);
10390
const [subscribed, setSubscribed] = useState(false);
104-
const refSubscribed = useRef(subscribed)
91+
const refSubscribed = useRef(subscribed);
10592

10693
const eventListener = useCallback(() => {
10794
setLoadingState(flagsmith?.loadingState);
108-
}, [flagsmith])
95+
}, [flagsmith]);
10996
if (!refSubscribed.current) {
110-
events.on('loading_event', eventListener)
111-
refSubscribed.current = true
97+
events.on('loading_event', eventListener);
98+
refSubscribed.current = true;
11299
}
113100

114101
useEffect(() => {
115102
if (!subscribed && flagsmith?.initialised) {
116-
events.on('loading_event', eventListener)
117-
setSubscribed(true)
103+
events.on('loading_event', eventListener);
104+
setSubscribed(true);
118105
}
119106
return () => {
120107
if (subscribed) {
121-
events.off('loading_event', eventListener)
108+
events.off('loading_event', eventListener);
122109
}
123110
};
124-
}, [flagsmith, subscribed, eventListener])
111+
}, [flagsmith, subscribed, eventListener]);
125112

126-
return loadingState
113+
return loadingState;
127114
}
128115

129-
type UseFlagsReturn<
130-
F extends string | Record<string, any>,
131-
T extends string
132-
> = [F] extends [string]
116+
type UseFlagsReturn<F extends string | Record<string, any>, T extends string> = F extends string
133117
? {
134-
[K in F]: IFlagsmithFeature;
135-
} & {
136-
[K in T]: IFlagsmithTrait;
137-
}
118+
[K in F]: IFlagsmithFeature;
119+
} & {
120+
[K in T]: IFlagsmithTrait;
121+
}
138122
: {
139-
[K in keyof F]: IFlagsmithFeature<F[K]>;
140-
} & {
141-
[K in T]: IFlagsmithTrait;
142-
};
123+
[K in keyof F]: IFlagsmithFeature<F[K]>;
124+
} & {
125+
[K in T]: IFlagsmithTrait;
126+
};
143127

144128
/**
145129
* Example usage:
@@ -154,66 +138,63 @@ type UseFlagsReturn<
154138
* }
155139
* useFlags<MyFeatureInterface>(["featureOne", "featureTwo"]);
156140
*/
157-
export function useFlags<
158-
F extends string | Record<string, any>,
159-
T extends string = string
160-
>(
161-
_flags: readonly (F | keyof F)[], _traits: readonly T[] = []
162-
){
163-
const firstRender = useRef(true)
164-
const flags = useConstant<string[]>(flagsAsArray(_flags))
165-
const traits = useConstant<string[]>(flagsAsArray(_traits))
166-
const flagsmith = useContext(FlagsmithContext)
141+
export function useFlags<F extends string | Record<string, any>, T extends string = string>(
142+
_flags: readonly (F | keyof F)[],
143+
_traits: readonly T[] = [],
144+
) {
145+
const firstRender = useRef(true);
146+
const flags = useConstant<string[]>(flagsAsArray(_flags));
147+
const traits = useConstant<string[]>(flagsAsArray(_traits));
148+
const flagsmith = useContext(FlagsmithContext);
167149
const [renderRef, setRenderRef] = useState(getRenderKey(flagsmith as IFlagsmith, flags, traits));
168150
const eventListener = useCallback(() => {
169-
const newRenderKey = getRenderKey(flagsmith as IFlagsmith, flags, traits)
151+
const newRenderKey = getRenderKey(flagsmith as IFlagsmith, flags, traits);
170152
if (newRenderKey !== renderRef) {
171153
// @ts-expect-error using internal function, consumers would never call this
172-
flagsmith?.log("React - useFlags flags and traits have changed")
173-
setRenderRef(newRenderKey)
154+
flagsmith?.log('React - useFlags flags and traits have changed');
155+
setRenderRef(newRenderKey);
174156
}
175-
}, [renderRef])
157+
}, [renderRef]);
176158
const emitterRef = useRef(events.once('event', eventListener));
177159

178-
179-
180160
if (firstRender.current) {
181161
firstRender.current = false;
182162
// @ts-expect-error using internal function, consumers would never call this
183-
flagsmith?.log("React - Initialising event listeners")
163+
flagsmith?.log('React - Initialising event listeners');
184164
}
185165

186-
useEffect(()=>{
166+
useEffect(() => {
187167
return () => {
188-
emitterRef.current?.()
189-
}
190-
}, [])
168+
emitterRef.current?.();
169+
};
170+
}, []);
191171

192172
const res = useMemo(() => {
193-
const res: any = {}
194-
flags.map((k) => {
173+
const res: any = {};
174+
flags
175+
.map((k) => {
195176
res[k] = {
196177
enabled: flagsmith!.hasFeature(k),
197178
value: flagsmith!.getValue(k),
198-
}
199-
}).concat(traits?.map((v) => {
200-
res[v] = flagsmith!.getTrait(v)
201-
}))
202-
return res
203-
}, [renderRef])
204-
205-
return res as UseFlagsReturn<F, T>
179+
};
180+
})
181+
.concat(
182+
traits?.map((v) => {
183+
res[v] = flagsmith!.getTrait(v);
184+
}),
185+
);
186+
return res;
187+
}, [renderRef]);
188+
189+
return res as UseFlagsReturn<F, T>;
206190
}
207191

208-
export function useFlagsmith<
209-
F extends string | Record<string, any>,
210-
T extends string = string
211-
>() {
212-
const context = useContext(FlagsmithContext)
192+
export function useFlagsmith<F extends string | Record<string, any>, T extends string = string>() {
193+
const context = useContext(FlagsmithContext);
213194

214195
if (!context) {
215-
throw new Error('useFlagsmith must be used with in a FlagsmithProvider')
196+
throw new Error('useFlagsmith must be used with in a FlagsmithProvider');
216197
}
217198

218-
return context as unknown as IFlagsmith<F, T>
199+
return context as unknown as IFlagsmith<F, T>;
219200
}

‎test/react.test.tsx‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,42 @@ describe('FlagsmithProvider', () => {
192192
expect(JSON.parse(screen.getByTestId("flags").innerHTML).hero.enabled).toBe(true)
193193
});
194194
});
195+
196+
it('should not crash when server returns 500 error', async () => {
197+
const onChange = jest.fn();
198+
const onError = jest.fn();
199+
200+
const { flagsmith, initConfig, mockFetch } = getFlagsmith({
201+
onChange,
202+
onError,
203+
});
204+
205+
mockFetch.mockImplementationOnce(() =>
206+
Promise.resolve({
207+
status: 500,
208+
headers: { get: () => null },
209+
text: () => Promise.resolve('API Response: 500')
210+
})
211+
);
212+
213+
expect(() => {
214+
render(
215+
<FlagsmithProvider flagsmith={flagsmith} options={initConfig}>
216+
<FlagsmithPage/>
217+
</FlagsmithProvider>
218+
);
219+
}).not.toThrow();
220+
221+
expect(mockFetch).toHaveBeenCalledTimes(1);
222+
223+
await waitFor(() => {
224+
// Loading should complete with error
225+
const loadingState = JSON.parse(screen.getByTestId("loading-state").innerHTML);
226+
expect(loadingState.isLoading).toBe(false);
227+
expect(loadingState.isFetching).toBe(false);
228+
expect(loadingState.error).toBeTruthy();
229+
});
230+
231+
// onError callback should have been called
232+
expect(onError).toHaveBeenCalledTimes(1);
233+
});

0 commit comments

Comments
 (0)