artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 15:31:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

storm: migrate feature flags to Apollo Client (#48698)

Browse Source 
Based on [the experimental
branch](https://github.com/sourcegraph/sourcegraph/pull/47888). This PR
goes further and removes the `FeatureFlagClient` altogether and replaces
its functionality with the logic in React hook. It's possible because
now the cache is managed by Apollo, and we only add a layer on top of it
that ensures that we refetch values once `cacheTTL` elapses.

This change allows us to skip evaluate feature flag API requests in
Storm routes if feature flag values are preloaded!

## Test plan

1. Enable Storm in experimental features.
2. Load the home page.
3. Notice that the `search-ownership` and
`plg-enable-add-codehost-widget` feature flags are not fetched
separately. They are loaded as a part of the `SearchPageQuery`, and the
`useFeatureFlag` hook reuses cached values.
This commit is contained in:
Valery Bugakov 2023-03-06 21:37:10 -08:00 committed by GitHub
parent 4662476f2a
commit b449de7b71
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 206 additions and 480 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
client/web/src/LegacySourcegraphWebApp.tsx
View File

@ -45,7 +45,7 @@ import { getWebGraphQLClient } from './backend/graphql'
import { isBatchChangesExecutionEnabled } from './batches'
import { ComponentsComposer } from './components/ComponentsComposer'
import { ErrorBoundary } from './components/ErrorBoundary'
import { FeatureFlagsProvider } from './featureFlags/FeatureFlagsProvider'
import { FeatureFlagsLocalOverrideAgent } from './featureFlags/FeatureFlagsProvider'
import { LegacyLayout, LegacyLayoutProps } from './LegacyLayout'
import { LegacyRouteContextProvider } from './LegacyRouteContext'
import { PageError } from './PageError'
@ -254,7 +254,7 @@ export class LegacySourcegraphWebApp extends React.Component<StaticAppConfig, Le
<SettingsProvider settingsCascade={this.state.settingsCascade} />,
<ErrorBoundary location={null} />,
<TraceSpanProvider name={SharedSpanName.AppMount} />,
<FeatureFlagsProvider />,
<FeatureFlagsLocalOverrideAgent />,
<ShortcutProvider />,
<TemporarySettingsProvider temporarySettingsStorage={temporarySettingsStorage} />,
<SearchResultsCacheProvider />,

4
client/web/src/SourcegraphWebApp.tsx
View File

@ -32,7 +32,7 @@ import { setLinkComponent, RouterLink, WildcardThemeContext, WildcardTheme } fro
import { authenticatedUser as authenticatedUserSubject, AuthenticatedUser, authenticatedUserValue } from './auth'
import { ComponentsComposer } from './components/ComponentsComposer'
import { ErrorBoundary, RouteError } from './components/ErrorBoundary'
import { FeatureFlagsProvider } from './featureFlags/FeatureFlagsProvider'
import { FeatureFlagsLocalOverrideAgent } from './featureFlags/FeatureFlagsProvider'
import { LegacyRoute, LegacyRouteContextProvider } from './LegacyRouteContext'
import { PageError } from './PageError'
import { createPlatformContext } from './platform/context'
@ -286,7 +286,7 @@ export const SourcegraphWebApp: FC<SourcegraphWebAppProps> = props => {
<SettingsProvider settingsCascade={settingsCascade} />,
<ErrorBoundary location={null} />,
<TraceSpanProvider name={SharedSpanName.AppMount} />,
<FeatureFlagsProvider />,
<FeatureFlagsLocalOverrideAgent />,
<ShortcutProvider />,
<TemporarySettingsProvider temporarySettingsStorage={temporarySettingsStorage} />,
<SearchResultsCacheProvider />,

28
client/web/src/enterprise/own/RepositoryOwnPage.story.tsx
View File

@ -5,7 +5,7 @@ import { subDays } from 'date-fns'
import { getDocumentNode } from '@sourcegraph/http-client'
import { WebStory } from '../../components/WebStory'
import { MockedFeatureFlagsProvider } from '../../featureFlags/MockedFeatureFlagsProvider'
import { createFlagMock } from '../../featureFlags/createFlagMock'
import {
ExternalServiceKind,
GetIngestedCodeownersResult,
@ -59,23 +59,21 @@ const empyResponse: MockedResponse<GetIngestedCodeownersResult, GetIngestedCodeo
},
}
const searchOwnershipFlagMock = createFlagMock('search-ownership', true)
export const EmptyNonAdmin: Story = () => (
<WebStory mocks={[empyResponse]}>
<WebStory mocks={[empyResponse, searchOwnershipFlagMock]}>
{({ useBreadcrumb }) => (
<MockedFeatureFlagsProvider overrides={{ 'search-ownership': true }}>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: false }} useBreadcrumb={useBreadcrumb} />
</MockedFeatureFlagsProvider>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: false }} useBreadcrumb={useBreadcrumb} />
)}
</WebStory>
)
EmptyNonAdmin.storyName = 'Empty (non-admin)'
export const EmptyAdmin: Story = () => (
<WebStory mocks={[empyResponse]}>
<WebStory mocks={[empyResponse, searchOwnershipFlagMock]}>
{({ useBreadcrumb }) => (
<MockedFeatureFlagsProvider overrides={{ 'search-ownership': true }}>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: true }} useBreadcrumb={useBreadcrumb} />
</MockedFeatureFlagsProvider>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: true }} useBreadcrumb={useBreadcrumb} />
)}
</WebStory>
)
@ -104,22 +102,18 @@ const populatedResponse: MockedResponse<GetIngestedCodeownersResult, GetIngested
}
export const PopulatedNonAdmin: Story = () => (
<WebStory mocks={[populatedResponse]}>
<WebStory mocks={[populatedResponse, searchOwnershipFlagMock]}>
{({ useBreadcrumb }) => (
<MockedFeatureFlagsProvider overrides={{ 'search-ownership': true }}>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: false }} useBreadcrumb={useBreadcrumb} />
</MockedFeatureFlagsProvider>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: false }} useBreadcrumb={useBreadcrumb} />
)}
</WebStory>
)
PopulatedNonAdmin.storyName = 'Populated (non-admin)'
export const PopulatedAdmin: Story = () => (
<WebStory mocks={[populatedResponse]}>
<WebStory mocks={[populatedResponse, searchOwnershipFlagMock]}>
{({ useBreadcrumb }) => (
<MockedFeatureFlagsProvider overrides={{ 'search-ownership': true }}>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: true }} useBreadcrumb={useBreadcrumb} />
</MockedFeatureFlagsProvider>
<RepositoryOwnPage repo={repo} authenticatedUser={{ siteAdmin: true }} useBreadcrumb={useBreadcrumb} />
)}
</WebStory>
)

34
client/web/src/featureFlags/FeatureFlagsProvider.tsx
View File

@ -1,19 +1,10 @@
import React, { createContext, useEffect } from 'react'
import { FC, PropsWithChildren, useEffect } from 'react'
import { logger } from '@sourcegraph/common'
import { requestGraphQL } from '../backend/graphql'
import { removeFeatureFlagOverride, setFeatureFlagOverride } from './lib/feature-flag-local-overrides'
import { FeatureFlagClient } from './lib/FeatureFlagClient'
import { parseUrlOverrideFeatureFlags } from './lib/parseUrlOverrideFeatureFlags'
interface FeatureFlagsContextValue {
client?: FeatureFlagClient
}
export const FeatureFlagsContext = createContext<FeatureFlagsContextValue>({})
/**
* Overrides feature flag based on initial URL query parameters
*
@ -23,7 +14,7 @@ export const FeatureFlagsContext = createContext<FeatureFlagsContextValue>({})
* Remove/reset local override: "/?feature-flag-key=my-feature"
* Multiple values: /?feature-flag-key=my-feature-one,my-feature-two&feature-flag-value=false,true
*/
const FeatureFlagsLocalOverrideAgent = React.memo(() => {
export const FeatureFlagsLocalOverrideAgent: FC<PropsWithChildren<{}>> = ({ children }) => {
useEffect(() => {
try {
const overrideFeatureFlags = parseUrlOverrideFeatureFlags(location.search) || {}
@ -44,25 +35,6 @@ const FeatureFlagsLocalOverrideAgent = React.memo(() => {
logger.error(error)
}
}, [])
return null
})
const MINUTE = 60000
const featureFlagsContextValue = {
client: new FeatureFlagClient(requestGraphQL, MINUTE * 10),
} satisfies FeatureFlagsContextValue
interface FeatureFlagsProviderProps {
isLocalOverrideEnabled?: boolean
return <>{children}</>
}
export const FeatureFlagsProvider: React.FunctionComponent<React.PropsWithChildren<FeatureFlagsProviderProps>> = ({
isLocalOverrideEnabled = true,
children,
}) => (
<FeatureFlagsContext.Provider value={featureFlagsContextValue}>
{isLocalOverrideEnabled && <FeatureFlagsLocalOverrideAgent />}
{children}
</FeatureFlagsContext.Provider>
)

58
client/web/src/featureFlags/MockedFeatureFlagsProvider.tsx
View File

@ -1,58 +0,0 @@
import React, { useEffect, useMemo } from 'react'
import { Observable, of, throwError } from 'rxjs'
import { requestGraphQL } from '../backend/graphql'
import { FeatureFlagName } from './featureFlags'
import { FeatureFlagsContext } from './FeatureFlagsProvider'
import { FeatureFlagClient } from './lib/FeatureFlagClient'
interface MockedFeatureFlagsProviderProps {
overrides: Partial<Record<FeatureFlagName, boolean | Error>>
cacheTimeToLive?: number
}
/**
* Provides mocked feature flag value for testing purposes.
*
* @example
* return (<MockedFeatureFlagsProvider overrides={{'my-feature-flag': true}}>
* <ComponentUsingFeatureFlag />
* </MockedFeatureFlagsProvider>)
*/
export const MockedFeatureFlagsProvider: React.FunctionComponent<
React.PropsWithChildren<MockedFeatureFlagsProviderProps>
> = ({ overrides, cacheTimeToLive, children }) => {
const mockRequestGraphQL = useMemo(
() =>
(
query: string,
variables: any
): Observable<{
data: { evaluateFeatureFlag: boolean | null }
}> => {
const value = overrides[variables.flagName as FeatureFlagName]
if (value instanceof Error) {
return throwError(value)
}
return of({
data: { evaluateFeatureFlag: value ?? null },
})
},
[overrides]
)
const providerValue = useMemo(
() => ({ client: new FeatureFlagClient(mockRequestGraphQL as typeof requestGraphQL, cacheTimeToLive) }),
// eslint-disable-next-line react-hooks/exhaustive-deps
[]
)
useEffect(() => {
providerValue.client.setRequestGraphQLFunction(mockRequestGraphQL as typeof requestGraphQL)
}, [providerValue, mockRequestGraphQL])
return <FeatureFlagsContext.Provider value={providerValue}>{children}</FeatureFlagsContext.Provider>
}

27
client/web/src/featureFlags/createFlagMock.ts Normal file
View File

@ -0,0 +1,27 @@
import { MockedResponse } from '@apollo/client/testing'
import { EvaluateFeatureFlagResult, EvaluateFeatureFlagVariables } from '../graphql-operations'
import { FeatureFlagName } from './featureFlags'
import { EVALUATE_FEATURE_FLAG_QUERY } from './useFeatureFlag'
type FlagMock = MockedResponse<EvaluateFeatureFlagResult, EvaluateFeatureFlagVariables>
export const createFlagMock = (flagName: FeatureFlagName, valueOrError: boolean | Error): FlagMock => ({
request: {
query: EVALUATE_FEATURE_FLAG_QUERY,
variables: {
flagName,
},
},
...(typeof valueOrError === 'boolean' && {
result: {
data: {
evaluateFeatureFlag: valueOrError,
},
},
}),
...(typeof valueOrError === 'object' && {
error: valueOrError,
}),
})

151
client/web/src/featureFlags/lib/FeatureFlagClient.test.ts
View File

@ -1,151 +0,0 @@
import delay from 'delay'
import { of } from 'rxjs'
import sinon, { SinonSpy } from 'sinon'
import { requestGraphQL } from '../../backend/graphql'
import type { FeatureFlagName } from '../featureFlags'
import { setFeatureFlagOverride } from './feature-flag-local-overrides'
import { FeatureFlagClient } from './FeatureFlagClient'
describe('FeatureFlagClient', () => {
const ENABLED_FLAG = 'enabled-flag' as FeatureFlagName
const DISABLED_FLAG = 'disabled-flag' as FeatureFlagName
const NON_EXISTING_FLAG = 'non-existing-flag' as FeatureFlagName
const mockRequestGraphQL = sinon.spy((query, variables) =>
of({
data: {
evaluateFeatureFlag:
variables.flagName === ENABLED_FLAG ? true : variables.flagName === DISABLED_FLAG ? false : null,
},
errors: [],
})
) as typeof requestGraphQL & SinonSpy
beforeEach(() => mockRequestGraphQL.resetHistory())
it('does not make initial API call ', () => {
new FeatureFlagClient(mockRequestGraphQL)
sinon.assert.notCalled(mockRequestGraphQL)
})
it('returns [true] response from API call for feature flag evaluation', async () => {
const client = new FeatureFlagClient(mockRequestGraphQL)
expect.assertions(1)
const value = await client.get(ENABLED_FLAG)
expect(value).toBe(true)
sinon.assert.calledOnce(mockRequestGraphQL)
})
it('returns [false] response from API call for feature flag evaluation', async () => {
const client = new FeatureFlagClient(mockRequestGraphQL)
expect.assertions(1)
const value = await client.get(DISABLED_FLAG)
expect(value).toBe(false)
sinon.assert.calledOnce(mockRequestGraphQL)
})
it('returns [defaultValue] correctly', async () => {
const client = new FeatureFlagClient(mockRequestGraphQL)
expect.assertions(1)
const value = await client.get(NON_EXISTING_FLAG)
expect(value).toBeNull()
sinon.assert.calledOnce(mockRequestGraphQL)
})
it('makes only single API call per feature flag evaluation', async () => {
const client = new FeatureFlagClient(mockRequestGraphQL)
expect.assertions(2)
const [value1, value2] = await Promise.all([client.get(ENABLED_FLAG), client.get(ENABLED_FLAG)])
expect(value1).toBe(true)
expect(value2).toBe(true)
sinon.assert.calledOnce(mockRequestGraphQL)
})
it('makes only single API call per feature flag if cache is still active', async () => {
const cacheTimeToLive = 10
const client = new FeatureFlagClient(mockRequestGraphQL, cacheTimeToLive)
expect.assertions(3)
const value1 = await client.get(ENABLED_FLAG)
expect(value1).toBe(true)
await delay(5)
const value2 = await client.get(ENABLED_FLAG)
expect(value2).toBe(true)
sinon.assert.calledOnce(mockRequestGraphQL)
await delay(5)
const value3 = await client.get(ENABLED_FLAG)
expect(value3).toBe(true)
sinon.assert.calledTwice(mockRequestGraphQL)
})
it('updates on new/different value after cache TTL', async () => {
let index = -1
const mockRequestGraphQL = sinon.spy((query, variables) => {
index++
return of({
data: { evaluateFeatureFlag: [ENABLED_FLAG].includes(variables.flagName) && index === 0 },
errors: [],
})
}) as typeof requestGraphQL & SinonSpy
const cacheTimeToLive = 1
const client = new FeatureFlagClient(mockRequestGraphQL, cacheTimeToLive)
expect.assertions(2)
const value1 = await client.get(ENABLED_FLAG)
expect(value1).toBe(true)
await delay(cacheTimeToLive)
const value2 = await client.get(ENABLED_FLAG)
expect(value2).toBe(false)
})
describe('local feature flag overrides', () => {
beforeEach(() => {
// remove local overrides
localStorage.clear()
})
it('returns [false] override if it exists', async () => {
const client = new FeatureFlagClient(mockRequestGraphQL)
setFeatureFlagOverride(ENABLED_FLAG, false)
expect.assertions(1)
const value = await client.get(ENABLED_FLAG)
expect(value).toBe(false)
sinon.assert.notCalled(mockRequestGraphQL)
})
it('returns [true] override if it exists', async () => {
const client = new FeatureFlagClient(mockRequestGraphQL)
setFeatureFlagOverride(DISABLED_FLAG, true)
expect.assertions(1)
const value = await client.get(DISABLED_FLAG)
expect(value).toBe(true)
sinon.assert.notCalled(mockRequestGraphQL)
})
it('does not use non-boolean override', async () => {
const client = new FeatureFlagClient(mockRequestGraphQL)
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
setFeatureFlagOverride(DISABLED_FLAG, 'something else')
expect.assertions(1)
const value = await client.get(DISABLED_FLAG)
expect(value).toBe(false)
sinon.assert.calledOnce(mockRequestGraphQL)
})
})
})

77
client/web/src/featureFlags/lib/FeatureFlagClient.ts
View File

@ -1,77 +0,0 @@
import { map } from 'rxjs/operators'
import { dataOrThrowErrors, gql } from '@sourcegraph/http-client'
import type { requestGraphQL } from '../../backend/graphql'
import { EvaluateFeatureFlagResult } from '../../graphql-operations'
import { FeatureFlagName } from '../featureFlags'
import { getFeatureFlagOverrideValue } from './feature-flag-local-overrides'
/**
* Evaluate feature flags for the current user
*/
const fetchEvaluateFeatureFlag = (
requestGraphQLFunc: typeof requestGraphQL,
flagName: FeatureFlagName
): Promise<EvaluateFeatureFlagResult['evaluateFeatureFlag']> =>
requestGraphQLFunc<EvaluateFeatureFlagResult>(
gql`
query EvaluateFeatureFlag($flagName: String!) {
evaluateFeatureFlag(flagName: $flagName)
}
`,
{
flagName,
}
)
.pipe(
map(dataOrThrowErrors),
map(data => data.evaluateFeatureFlag)
)
.toPromise()
/**
* Feature flag client service. Should be used as singleton for the whole application.
*/
export class FeatureFlagClient {
private flags = new Map<FeatureFlagName, Promise<EvaluateFeatureFlagResult['evaluateFeatureFlag']>>()
/**
* @param requestGraphQLFunction function to use for making GQL API calls.
* @param cacheTimeToLive milliseconds to keep the value in the in-memory client-side cache.
*/
constructor(private requestGraphQLFunction: typeof requestGraphQL, private cacheTimeToLive?: number) {}
/**
* For mocking/testing purposes
*
* @see {MockedFeatureFlagsProvider}
*/
public setRequestGraphQLFunction(requestGraphQLFunction: typeof requestGraphQL): void {
this.requestGraphQLFunction = requestGraphQLFunction
}
/**
* Evaluates and returns feature flag value
*/
public get(flagName: FeatureFlagName): Promise<EvaluateFeatureFlagResult['evaluateFeatureFlag']> {
if (!this.flags.has(flagName)) {
const overriddenValue = getFeatureFlagOverrideValue(flagName)
// Use local feature flag override if exists
if (overriddenValue !== null) {
return Promise.resolve(overriddenValue)
}
const flag = fetchEvaluateFeatureFlag(this.requestGraphQLFunction, flagName)
this.flags.set(flagName, flag)
if (this.cacheTimeToLive) {
setTimeout(() => this.flags.delete(flagName), this.cacheTimeToLive)
}
}
return this.flags.get(flagName)!
}
}

90
client/web/src/featureFlags/useFeatureFlag.test.tsx
View File

@ -1,69 +1,36 @@
import React from 'react'
import { renderHook, waitFor } from '@testing-library/react'
import delay from 'delay'
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
import { createFlagMock } from './createFlagMock'
import { FeatureFlagName } from './featureFlags'
import { MockedFeatureFlagsProvider } from './MockedFeatureFlagsProvider'
import { useFeatureFlag } from './useFeatureFlag'
describe('useFeatureFlag', () => {
const ENABLED_FLAG = 'enabled-flag' as FeatureFlagName
const DISABLED_FLAG = 'disabled-flag' as FeatureFlagName
const ERROR_FLAG = 'error-flag' as FeatureFlagName
const NON_EXISTING_FLAG = 'non-existing-flag' as FeatureFlagName
const MOCKS = [
createFlagMock(ENABLED_FLAG, true),
createFlagMock(DISABLED_FLAG, false),
createFlagMock(ENABLED_FLAG, false),
createFlagMock(ERROR_FLAG, new Error('oops')),
]
const Wrapper: React.JSXElementConstructor<{
children: React.ReactElement
nextOverrides?: Partial<Record<FeatureFlagName, boolean | Error>>
cacheTimeToLive?: number
}> = ({ nextOverrides, children, cacheTimeToLive }) => {
// New `renderHook` doesn't pass any props into Wrapper component like the old one
// so couldn't find a way to reproduce `state.setRender(...)` with custom `overrides`
// we have to use this state together with `nextOverrides` as an alternative.
const [overrides, setOverrides] = React.useState({
[ENABLED_FLAG]: true,
[DISABLED_FLAG]: false,
[ERROR_FLAG]: new Error('Some error'),
})
React.useEffect(() => {
setTimeout(() => {
setOverrides(current => nextOverrides ?? current)
}, 0)
}, [nextOverrides])
return (
<MockedFeatureFlagsProvider overrides={overrides} cacheTimeToLive={cacheTimeToLive}>
{children}
</MockedFeatureFlagsProvider>
)
}
const setup = (
initialFlagName: FeatureFlagName,
defaultValue = false,
cacheTimeToLive?: number,
nextOverrides?: Partial<Record<FeatureFlagName, boolean | Error>>
) => {
const setup = (initialFlagName: FeatureFlagName, defaultValue = false, cacheTTL?: number) => {
const flagStates = new Map()
const allResults: ReturnType<typeof useFeatureFlag>[] = []
const result = renderHook<
ReturnType<typeof useFeatureFlag>,
{
flagName: FeatureFlagName
}
>(
const result = renderHook(
({ flagName }) => {
const result = useFeatureFlag(flagName, defaultValue)
const result = useFeatureFlag(flagName, defaultValue, cacheTTL, flagStates)
allResults.push(result)
return result
},
{
wrapper: props => (
<Wrapper cacheTimeToLive={cacheTimeToLive} nextOverrides={nextOverrides} {...props} />
),
wrapper: ({ children }) => <MockedTestProvider mocks={MOCKS}>{children}</MockedTestProvider>,
initialProps: {
flagName: initialFlagName,
},
@ -85,18 +52,6 @@ describe('useFeatureFlag', () => {
await waitFor(() => expect(state.result.current).toStrictEqual([false, 'loaded', undefined]))
})
it('returns [defaultValue=true] correctly', async () => {
const state = setup(NON_EXISTING_FLAG, true)
await waitFor(() => expect(state.result.current).toEqual(expect.arrayContaining([true, 'loaded'])))
})
it('returns [defaultValue=false] correctly', async () => {
const state = setup(NON_EXISTING_FLAG, false)
await waitFor(() => expect(state.result.current).toEqual(expect.arrayContaining([false, 'loaded'])))
})
it('returns [true] value correctly', async () => {
const state = setup(ENABLED_FLAG)
// Initial state
@ -107,22 +62,26 @@ describe('useFeatureFlag', () => {
})
it('updates on value change', async () => {
const state = setup(ENABLED_FLAG, false, 100, { [ENABLED_FLAG]: false })
const state = setup(ENABLED_FLAG, false, 100)
// Initial state
expect(state.allResults[0]).toStrictEqual([false, 'initial', undefined])
// Loaded state
await waitFor(() => expect(state.result.current).toStrictEqual([true, 'loaded', undefined]))
await delay(100)
// Rerender and wait for new state
// The cached value is used because of TTL.
await delay(50)
state.rerender({ flagName: ENABLED_FLAG })
await waitFor(() => expect(state.result.current).toStrictEqual([true, 'loaded', undefined]))
// The new value is fetched because the cache is stale.
await delay(50)
state.rerender({ flagName: ENABLED_FLAG })
await waitFor(() => expect(state.result.current).toStrictEqual([false, 'loaded', undefined]))
})
it('updates when feature flag prop changes', async () => {
const state = setup(ENABLED_FLAG, false, undefined, {})
const state = setup(ENABLED_FLAG, false, undefined)
// Initial state
expect(state.result.current).toStrictEqual([false, 'initial', undefined])
// Loaded state
@ -133,11 +92,6 @@ describe('useFeatureFlag', () => {
await waitFor(() => expect(state.result.current).toStrictEqual([false, 'loaded', undefined]))
})
it('returns "error" when no context parent', () => {
const state = renderHook(() => useFeatureFlag(ENABLED_FLAG))
expect(state.result.current).toEqual(expect.arrayContaining([false, 'error']))
})
it('returns "error" when unhandled error', async () => {
const state = setup(ERROR_FLAG)

142
client/web/src/featureFlags/useFeatureFlag.ts
View File

@ -1,57 +1,115 @@
import { useContext, useState } from 'react'
import { logger } from '@sourcegraph/common'
import { useIsMounted } from '@sourcegraph/wildcard'
import { getDocumentNode, gql, useQuery } from '@sourcegraph/http-client'
import { EvaluateFeatureFlagResult, EvaluateFeatureFlagVariables } from '../graphql-operations'
import { FeatureFlagName } from './featureFlags'
import { FeatureFlagsContext } from './FeatureFlagsProvider'
import { getFeatureFlagOverrideValue } from './lib/feature-flag-local-overrides'
export const EVALUATE_FEATURE_FLAG_QUERY = getDocumentNode(gql`
query EvaluateFeatureFlag($flagName: String!) {
evaluateFeatureFlag(flagName: $flagName)
}
`)
type FetchStatus = 'initial' | 'loaded' | 'error'
const MISSING_CLIENT_ERROR =
'[useFeatureFlag]: No FeatureFlagClient is configured. All feature flags will default to "false" value.'
type FlagName = string
type EvaluateError = unknown
const MINUTE = 60000
const FEATURE_FLAG_CACHE_TTL = MINUTE * 10
const FLAG_STATES = new Map<FlagName, 'new_value' | 'valid' | 'stale' | 'refetch'>()
/**
* Returns an evaluated feature flag for the current user
*
* @returns [flagValue, fetchStatus, error]
* Evaluates the feature flag via GraphQL query and returns the value.
* Prioritizes feature flag overrides. It uses the value cached by Apollo
* Client if available and not stale. The hook render starts the timeout,
* which ensures that we mark the cached value as stale after `cacheTTL`.
* If the cache TTL is elapsed on the next render, we refetch the value.
*/
export function useFeatureFlag(flagName: FeatureFlagName, defaultValue = false): [boolean, FetchStatus, any?] {
const isMounted = useIsMounted()
const { client } = useContext(FeatureFlagsContext)
const [{ value, status, error }, setResult] = useState<{ value: boolean | null; status: FetchStatus; error?: any }>(
export function useFeatureFlag(
flagName: FeatureFlagName,
defaultValue = false,
/**
* Used for tests only
*/
cacheTTL = FEATURE_FLAG_CACHE_TTL,
/**
* Used for tests only
*/
flagStates = FLAG_STATES
): [boolean, FetchStatus, EvaluateError?] {
const overriddenValue = getFeatureFlagOverrideValue(flagName)
const overriddenValueExists = typeof overriddenValue === 'boolean'
const { data, error, refetch } = useQuery<EvaluateFeatureFlagResult, EvaluateFeatureFlagVariables>(
EVALUATE_FEATURE_FLAG_QUERY,
{
status: 'initial',
value: defaultValue,
variables: { flagName },
fetchPolicy: 'cache-first',
// When updating the feature flag value with refetch, we want to skip the cache and rely on the network.
nextFetchPolicy: 'network-only',
skip: overriddenValueExists,
}
)
if (!client) {
if (status !== 'error') {
logger.warn(MISSING_CLIENT_ERROR)
setResult(({ value }) => ({ value, status: 'error', error: new Error(MISSING_CLIENT_ERROR) }))
}
} else {
// We want to `client.get(flagName)` on every render and update the state only
// on the value change. We won't be sending an API request on every render
// because evaluated feature flags are cached in memory for a short period of time.
async function getValue(): Promise<void> {
const newValue = await client!.get(flagName)
if (newValue === value && status !== 'initial') {
return
}
if (isMounted()) {
setResult({ value: newValue, status: 'loaded' })
}
}
getValue().catch(error => {
if (isMounted() && status !== 'error') {
setResult(({ value }) => ({ value, status: 'error', error }))
}
})
/**
* Skip the GraphQL query and return the `overriddenValue` if it exists.
*/
if (overriddenValueExists) {
return [overriddenValue, 'loaded']
}
return [typeof value === 'boolean' ? value : defaultValue, status, error]
if (data) {
/**
* Flag states are shared between all instances of the React hook, which
* eliminates the possibility of race conditions and ensures that each flag
* can have only one scheduled refetch call.
*
* Switch cases are defined in chronological order.
*/
switch (flagStates.get(flagName)) {
/**
* Upon receiving the new value from the API, we start the timer unique
* for the feature flag, marking it as stale after `cacheTTL`.
*/
case undefined:
case 'new_value': {
flagStates.set(flagName, 'valid')
setTimeout(() => flagStates.set(flagName, 'stale'), cacheTTL)
break
}
/**
* Do nothing. The `setTimeout` is in progress and we can use the cached value for now.
*/
case 'valid':
break
/**
* If we have the stale value, initiate the refetch with the `network-only`
* strategy. The hook will be re-rendered only if the value has changed after the refetch.
* On refetch success, we mark the value as the new one to restart the state cycle.
*/
case 'stale': {
flagStates.set(flagName, 'refetch')
refetch()
.then(() => flagStates.set(flagName, 'new_value'))
.catch(logger.error)
break
}
/**
* Do nothing. The `refetch` is in progress and we can use the cached value for now.
*/
case 'refetch':
break
}
}
const value = data?.evaluateFeatureFlag ?? defaultValue
const status = error ? 'error' : data ? 'loaded' : 'initial'
return [value, status, error?.networkError]
}

25
client/web/src/featureFlags/withFeatureFlag.test.tsx
View File

@ -1,7 +1,9 @@
import { render } from '@testing-library/react'
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
import { createFlagMock } from './createFlagMock'
import { FeatureFlagName } from './featureFlags'
import { MockedFeatureFlagsProvider } from './MockedFeatureFlagsProvider'
import { withFeatureFlag } from './withFeatureFlag'
describe('withFeatureFlag', () => {
@ -9,13 +11,14 @@ describe('withFeatureFlag', () => {
const falseComponentTestId = 'false-component'
const TrueComponent = () => <div data-testid={trueComponentTestId}>rendered when flag is true</div>
const FalseComponent = () => <div data-testid={falseComponentTestId}>rendered when flag is false</div>
const Wrapper = withFeatureFlag('test-flag' as FeatureFlagName, TrueComponent, FalseComponent)
const TEST_FLAG = 'test-flag' as FeatureFlagName
const Wrapper = withFeatureFlag(TEST_FLAG, TrueComponent, FalseComponent)
it('renders correctly when flagValue=true', async () => {
const { findByTestId } = render(
<MockedFeatureFlagsProvider overrides={{ 'test-flag': true } as Partial<Record<FeatureFlagName, boolean>>}>
<MockedTestProvider mocks={[createFlagMock(TEST_FLAG, true)]}>
<Wrapper />
</MockedFeatureFlagsProvider>
</MockedTestProvider>
)
expect(await findByTestId(trueComponentTestId)).toBeTruthy()
@ -23,9 +26,9 @@ describe('withFeatureFlag', () => {
it('renders correctly when flagValue=false', async () => {
const { findByTestId } = render(
<MockedFeatureFlagsProvider overrides={{ 'test-flag': false } as Partial<Record<FeatureFlagName, boolean>>}>
<MockedTestProvider mocks={[createFlagMock(TEST_FLAG, false)]}>
<Wrapper />
</MockedFeatureFlagsProvider>
</MockedTestProvider>
)
expect(await findByTestId(falseComponentTestId)).toBeTruthy()
@ -33,11 +36,9 @@ describe('withFeatureFlag', () => {
it('waits until flag value is resolved', () => {
const { queryByTestId } = render(
<MockedFeatureFlagsProvider
overrides={{ 'test-flag': new Error('Failed to fetch') } as Partial<Record<FeatureFlagName, boolean>>}
>
<MockedTestProvider mocks={[createFlagMock(TEST_FLAG, new Error('Failed to fetch'))]}>
<Wrapper />
</MockedFeatureFlagsProvider>
</MockedTestProvider>
)
expect(queryByTestId(falseComponentTestId)).toBeFalsy()
@ -47,9 +48,9 @@ describe('withFeatureFlag', () => {
it('renders correctly when flagValue=false and FalseComponent omitted', () => {
const LocalWrapper = withFeatureFlag('test-flag' as FeatureFlagName, TrueComponent)
const { findByTestId } = render(
<MockedFeatureFlagsProvider overrides={{ 'test-flag': false } as Partial<Record<FeatureFlagName, boolean>>}>
<MockedTestProvider mocks={[createFlagMock(TEST_FLAG, false)]}>
<LocalWrapper />
</MockedFeatureFlagsProvider>
</MockedTestProvider>
)
expect(() => findByTestId(trueComponentTestId)).rejects.toBeTruthy()

37
client/web/src/nav/UserNavItem.test.tsx
View File

@ -3,6 +3,7 @@ import userEvent from '@testing-library/user-event'
import { MemoryRouter } from 'react-router-dom'
import sinon from 'sinon'
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
import { AnchorLink, RouterLink, setLinkComponent } from '@sourcegraph/wildcard'
import { renderWithBrandedContext } from '@sourcegraph/wildcard/src/testing'
@ -50,14 +51,16 @@ describe('UserNavItem', () => {
expect(
render(
<MemoryRouter>
<UserNavItem
showKeyboardShortcutsHelp={() => undefined}
authenticatedUser={USER}
isSourcegraphDotCom={true}
isSourcegraphApp={false}
codeHostIntegrationMessaging="browser-extension"
showFeedbackModal={() => undefined}
/>
<MockedTestProvider>
<UserNavItem
showKeyboardShortcutsHelp={() => undefined}
authenticatedUser={USER}
isSourcegraphDotCom={true}
isSourcegraphApp={false}
codeHostIntegrationMessaging="browser-extension"
showFeedbackModal={() => undefined}
/>
</MockedTestProvider>
</MemoryRouter>
).asFragment()
).toMatchSnapshot()
@ -65,14 +68,16 @@ describe('UserNavItem', () => {
test('logout click triggers page refresh instead of performing client-side only navigation', async () => {
const result = renderWithBrandedContext(
<UserNavItem
showKeyboardShortcutsHelp={() => undefined}
authenticatedUser={USER}
isSourcegraphDotCom={true}
isSourcegraphApp={false}
codeHostIntegrationMessaging="browser-extension"
showFeedbackModal={() => undefined}
/>
<MockedTestProvider>
<UserNavItem
showKeyboardShortcutsHelp={() => undefined}
authenticatedUser={USER}
isSourcegraphDotCom={true}
isSourcegraphApp={false}
codeHostIntegrationMessaging="browser-extension"
showFeedbackModal={() => undefined}
/>
</MockedTestProvider>
)
// Prevent console.error cause by "Not implemented: navigation (except hash changes)"

7
client/web/src/storm/pages/SearchPage/SearchPage.loader.ts
View File

@ -15,11 +15,12 @@ import { createPreloadedQuery, QueryReference } from '../../backend/route-loader
* TODO: We need the `evaluateFeatureFlags` query that can evaluate multiple feature flags.
*/
const SEARCH_PAGE_QUERY = gql`
query SearchPageQuery($flagName: String!) {
query SearchPageQuery {
externalServices {
totalCount
}
evaluateFeatureFlag(flagName: $flagName)
codehostWidgetFlag: evaluateFeatureFlag(flagName: "plg-enable-add-codehost-widget")
searchOwnershipFlag: evaluateFeatureFlag(flagName: "search-ownership")
}
`
@ -29,5 +30,5 @@ export const { queryLoader, usePreloadedQueryData } = createPreloadedQuery<
>(SEARCH_PAGE_QUERY)
export function loader(): Promise<Record<string, QueryReference | undefined>> {
return queryLoader({ flagName: 'plg-enable-add-codehost-widget' })
return queryLoader({})
}

2
client/web/src/storm/pages/SearchPage/SearchPage.tsx
View File

@ -10,7 +10,7 @@ export const SearchPage: FC = () => {
const { authenticatedUser } = useLegacyContext_onlyInStormRoutes()
const shouldShowAddCodeHostWidget = getShouldShowAddCodeHostWidget({
isAddCodeHostWidgetEnabled: !!data?.evaluateFeatureFlag,
isAddCodeHostWidgetEnabled: !!data?.codehostWidgetFlag,
isSiteAdmin: authenticatedUser?.siteAdmin,
externalServicesCount: data?.externalServices.totalCount,
})