Skip to content

fix: Throw a better error message if context is null #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 6, 2025
Merged

fix: Throw a better error message if context is null #192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7cafd87
fix: Throw a better error if provider is missing
dbasilio Feb 24, 2025
ecb12ee
fix: The context can be null
dbasilio Feb 24, 2025
6ff8dad
fix: Add newlines
dbasilio Feb 24, 2025
68bc7b6
tests: Update tests with new context
dbasilio Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/FlagContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ export interface IFlagContextValue
>;
}

const FlagContext = React.createContext<IFlagContextValue>(null as never);
const FlagContext = React.createContext<IFlagContextValue | null>(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbasilio

This line is a bit concerning, because if anyone is using the FlagContext directly they would receive the same compiler errors as we had in the test. Ideally, I'd love for this change to not require a major update and this is the only part that is changing the public facing API in a way that might break clients.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue that the types were lying to the user before. The flag context is defaulted to null, but the type system is telling users that it will always have a value. This is why the error thrown if you forget to wrap your application in the provider is about the context being null.

If you want we can maybe re-cast the public export so it's not a breaking change? I think for the internal logic of this library it's important that the context is potentially null.

I was thinking something like this in the index.ts file

import FlagContext from './FlagContext';

const FlagContextWithoutNull = FlagContext as React.Context<IFlagContextValue | null>

export {
    FlagContextWithoutNull as FlagContext,
    // other exports
}

and then you could clean that up in a future major update

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you make a good point. And looking at our releases it's been a while since the last major. I will merge this as is and release it as 5.0.0. If we leave it I'm afraid we'll never get around to it.


export default FlagContext;
13 changes: 5 additions & 8 deletions src/FlagProvider.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React, { useContext, useEffect, useState } from 'react';
import React, { useEffect, useState } from 'react';
import { render, screen } from '@testing-library/react';
import { type Mock } from 'vitest';
import { UnleashClient, type IVariant, EVENTS } from 'unleash-proxy-client';
import FlagProvider from './FlagProvider';
import FlagContext from './FlagContext';
import { useFlagContext } from './useFlagContext';
import '@testing-library/jest-dom';

const getVariantMock = vi.fn().mockReturnValue('A');
Expand Down Expand Up @@ -44,8 +44,7 @@ vi.mock('unleash-proxy-client', async (importOriginal) => {
const noop = () => {};

const FlagConsumerAfterClientInit = () => {
const { updateContext, isEnabled, getVariant, client, on } =
useContext(FlagContext);
const { updateContext, isEnabled, getVariant, client, on } = useFlagContext();
const [enabled, setIsEnabled] = useState(false);
const [variant, setVariant] = useState<IVariant | null>(null);
const [context, setContext] = useState<any>('nothing');
Expand All @@ -71,8 +70,7 @@ const FlagConsumerAfterClientInit = () => {
};

const FlagConsumerBeforeClientInit = () => {
const { updateContext, isEnabled, getVariant, client, on } =
useContext(FlagContext);
const { updateContext, isEnabled, getVariant, client, on } = useFlagContext();
const [enabled, setIsEnabled] = useState(false);
const [variant, setVariant] = useState<IVariant | null>(null);
const [context, setContext] = useState<any>('nothing');
Expand Down Expand Up @@ -162,8 +160,7 @@ test('A memoized consumer should not rerender when the context provider values a
const renderCounter = vi.fn();

const MemoizedConsumer = React.memo(() => {
const { updateContext, isEnabled, getVariant, client, on } =
useContext(FlagContext);
const { updateContext, isEnabled, getVariant, client, on } = useFlagContext();

renderCounter();

Expand Down
10 changes: 5 additions & 5 deletions src/integration.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import React, { useContext } from 'react';
import React from 'react';
import { render, screen, waitFor } from '@testing-library/react';
import { EVENTS, UnleashClient } from 'unleash-proxy-client';
import FlagProvider from './FlagProvider';
import useFlagsStatus from './useFlagsStatus';
import { act } from 'react-dom/test-utils';
import useFlag from './useFlag';
import { useFlagContext } from './useFlagContext';
import useVariant from './useVariant';
import FlagContext from './FlagContext';

const fetchMock = vi.fn(async () => {
return Promise.resolve({
Expand Down Expand Up @@ -89,7 +89,7 @@ test('should render toggles', async () => {

test('should be ready from the start if bootstrapped', () => {
const Component = React.memo(() => {
const { flagsReady } = useContext(FlagContext);
const { flagsReady } = useFlagContext();

return <>{flagsReady ? 'ready' : ''}</>;
});
Expand Down Expand Up @@ -183,7 +183,7 @@ test('should render limited times when bootstrapped', async () => {

const Component = () => {
const enabled = useFlag('test-flag');
const { flagsReady } = useContext(FlagContext);
const { flagsReady } = useFlagContext();

renders += 1;

Expand Down Expand Up @@ -229,7 +229,7 @@ test('should resolve values before setting flagsReady', async () => {

const Component = () => {
const enabled = useFlag('test-flag');
const { flagsReady } = useContext(FlagContext);
const { flagsReady } = useFlagContext();

renders += 1;

Expand Down
6 changes: 3 additions & 3 deletions src/useFlag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { useContext, useEffect, useState, useRef } from 'react';
import FlagContext from './FlagContext';
import { useEffect, useState, useRef } from 'react';
import { useFlagContext } from './useFlagContext';

const useFlag = (featureName: string) => {
const { isEnabled, client } = useContext(FlagContext);
const { isEnabled, client } = useFlagContext();
const [flag, setFlag] = useState(!!isEnabled(featureName));
const flagRef = useRef<typeof flag>();
flagRef.current = flag;
Expand Down
17 changes: 17 additions & 0 deletions src/useFlagContext.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { renderHook } from '@testing-library/react-hooks/native';
import FlagProvider from "./FlagProvider";
import { useFlagContext } from "./useFlagContext";

test("throws an error if used outside of a FlagProvider", () => {
const { result } = renderHook(() => useFlagContext());

expect(result.error).toEqual(
Error("This hook must be used within a FlagProvider")
);
});

test("does not throw an error if used inside of a FlagProvider", () => {
const { result } = renderHook(() => useFlagContext(), { wrapper: FlagProvider });

expect(result.error).toBeUndefined();
});
10 changes: 10 additions & 0 deletions src/useFlagContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { useContext } from 'react';
import FlagContext from './FlagContext';

export function useFlagContext() {
const context = useContext(FlagContext);
if (!context) {
throw new Error('This hook must be used within a FlagProvider');
}
return context;
}
6 changes: 3 additions & 3 deletions src/useFlags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { useContext, useEffect, useState } from 'react';
import FlagContext from './FlagContext';
import { useEffect, useState } from 'react';
import { useFlagContext } from './useFlagContext';

const useFlags = () => {
const { client } = useContext(FlagContext);
const { client } = useFlagContext();
const [flags, setFlags] = useState(client.getAllToggles());

useEffect(() => {
Expand Down
6 changes: 2 additions & 4 deletions src/useFlagsStatus.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
/** @format */

import { useContext } from 'react';
import FlagContext from './FlagContext';
import { useFlagContext } from './useFlagContext';

const useFlagsStatus = () => {
const { flagsReady, flagsError } = useContext(FlagContext);
const { flagsReady, flagsError } = useFlagContext();

return { flagsReady, flagsError };
};
Expand Down
5 changes: 2 additions & 3 deletions src/useUnleashClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { useContext } from 'react';
import FlagContext from './FlagContext';
import { useFlagContext } from './useFlagContext';

const useUnleashClient = () => {
const { client } = useContext(FlagContext);
const { client } = useFlagContext();
return client;
};

Expand Down
5 changes: 2 additions & 3 deletions src/useUnleashContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { useContext } from 'react';
import FlagContext from './FlagContext';
import { useFlagContext } from './useFlagContext';

const useUnleashContext = () => {
const { updateContext } = useContext(FlagContext);
const { updateContext } = useFlagContext();

return updateContext;
};
Expand Down
6 changes: 3 additions & 3 deletions src/useVariant.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useContext, useState, useEffect, useRef } from 'react';
import { useState, useEffect, useRef } from 'react';
import { IVariant } from 'unleash-proxy-client';
import FlagContext from './FlagContext';
import { useFlagContext } from './useFlagContext';

export const variantHasChanged = (
oldVariant: IVariant,
Expand All @@ -17,7 +17,7 @@ export const variantHasChanged = (
};

const useVariant = (featureName: string): Partial<IVariant> => {
const { getVariant, client } = useContext(FlagContext);
const { getVariant, client } = useFlagContext();

const [variant, setVariant] = useState(getVariant(featureName));
const variantRef = useRef<typeof variant>({
Expand Down