diff --git a/babel.config.js b/babel.config.js index 1d382115f2fd7..b7b0f51375742 100644 --- a/babel.config.js +++ b/babel.config.js @@ -36,7 +36,7 @@ module.exports = api => { 'web.url-search-params', // Commonly needed by extensions (used by vscode-jsonrpc) 'web.immediate', - // Avoids issues with RxJS interop + // Always define Symbol.observable before libraries are loaded, ensuring interopability between different libraries. 'esnext.symbol.observable', // Webpack v4 chokes on optional chaining and nullish coalescing syntax, fix will be released with webpack v5. '@babel/plugin-proposal-optional-chaining', diff --git a/client/shared/src/components/activation/ActivationChecklist.test.tsx b/client/shared/src/components/activation/ActivationChecklist.test.tsx index c9598eaaa8acf..176ea5f08cc6a 100644 --- a/client/shared/src/components/activation/ActivationChecklist.test.tsx +++ b/client/shared/src/components/activation/ActivationChecklist.test.tsx @@ -1,4 +1,3 @@ -import * as H from 'history' import React from 'react' import renderer from 'react-test-renderer' @@ -8,11 +7,8 @@ jest.mock('mdi-react/CheckboxBlankCircleIcon', () => 'CheckboxBlankCircleIcon') jest.mock('mdi-react/CheckIcon', () => 'CheckIcon') describe('ActivationChecklist', () => { - const history = H.createMemoryHistory({ keyLength: 0 }) test('render loading', () => { - const component = renderer.create( - - ) + const component = renderer.create() expect(component.toJSON()).toMatchSnapshot() }) test('render 0/1 complete', () => { @@ -27,7 +23,6 @@ describe('ActivationChecklist', () => { }, ]} completed={{}} - history={history} /> ) expect(component.toJSON()).toMatchSnapshot() @@ -43,7 +38,6 @@ describe('ActivationChecklist', () => { }, ]} completed={{ EnabledRepository: true }} // another item - history={history} /> ) expect(component.toJSON()).toMatchSnapshot() @@ -60,7 +54,6 @@ describe('ActivationChecklist', () => { }, ]} completed={{ ConnectedCodeHost: true }} // same item as in steps - history={H.createMemoryHistory({ keyLength: 0 })} /> ) expect(component.toJSON()).toMatchSnapshot() diff --git a/client/shared/src/components/activation/ActivationChecklist.tsx b/client/shared/src/components/activation/ActivationChecklist.tsx index 0145c32ba7cf3..33340ec0b9b13 100644 --- a/client/shared/src/components/activation/ActivationChecklist.tsx +++ b/client/shared/src/components/activation/ActivationChecklist.tsx @@ -1,6 +1,5 @@ import { Accordion, AccordionItem, AccordionButton, AccordionPanel } from '@reach/accordion' import classNames from 'classnames' -import * as H from 'history' import CheckboxBlankCircleOutlineIcon from 'mdi-react/CheckboxBlankCircleOutlineIcon' import CheckCircleIcon from 'mdi-react/CheckCircleIcon' import ChevronDownIcon from 'mdi-react/ChevronDownIcon' @@ -13,7 +12,6 @@ import { ActivationCompletionStatus, ActivationStep } from './Activation' interface ActivationChecklistItemProps extends ActivationStep { done: boolean - history: H.History className?: string } @@ -45,7 +43,6 @@ export const ActivationChecklistItem: React.FunctionComponent { - public render(): JSX.Element { - return this.props.completed ? ( - - - {this.props.steps.map(step => ( - - - - - - {step.detail} - - - ))} - - - ) : ( - - ) +export const ActivationChecklist: React.FunctionComponent = ({ + className, + steps, + completed, + buttonClassName, +}) => { + if (!completed) { + return } + + return ( + + + {steps.map(step => ( + + + + + + {step.detail} + + + ))} + + + ) } diff --git a/client/shared/src/components/activation/__snapshots__/ActivationDropdown.test.tsx.snap b/client/shared/src/components/activation/__snapshots__/ActivationDropdown.test.tsx.snap index 04c0e3490b749..9945ba9778f14 100644 --- a/client/shared/src/components/activation/__snapshots__/ActivationDropdown.test.tsx.snap +++ b/client/shared/src/components/activation/__snapshots__/ActivationDropdown.test.tsx.snap @@ -1246,7 +1246,6 @@ exports[`ActivationDropdown renders the activation dropdown 1`] = ` } done={false} - history="[History]" id="DidSearch" key="DidSearch" title="Search your code" @@ -1442,7 +1440,6 @@ exports[`ActivationDropdown renders the activation dropdown 1`] = ` ({ variables?: V }): Observable> { const nameMatch = request.match(/^\s*(?:query|mutation)\s+(\w+)/) - const apiURL = `/.api/graphql${nameMatch ? '?' + nameMatch[1] : ''}` + const apiURL = `${GRAPHQL_URI}${nameMatch ? '?' + nameMatch[1] : ''}` return fromFetch(baseUrl ? new URL(apiURL, baseUrl).href : apiURL, { ...options, method: 'POST', @@ -77,3 +95,63 @@ export function requestGraphQLCommon({ selector: response => checkOk(response).json(), }) } + +export const graphQLClient = ({ headers }: { headers: RequestInit['headers'] }): ApolloClient => + new ApolloClient({ + uri: GRAPHQL_URI, + cache: new InMemoryCache(), + link: createHttpLink({ + uri: ({ operationName }) => `${GRAPHQL_URI}?${operationName}`, + headers, + }), + }) + +type RequestDocument = string | DocumentNode + +/** + * Returns a `DocumentNode` value to support integrations with GraphQL clients that require this. + * + * @param document The GraphQL operation payload + * @returns The created `DocumentNode` + */ +export const getDocumentNode = (document: RequestDocument): DocumentNode => { + if (typeof document === 'string') { + return apolloGql(document) + } + return document +} + +const useDocumentNode = (document: RequestDocument): DocumentNode => + useMemo(() => getDocumentNode(document), [document]) + +/** + * Send a query to GraphQL and respond to updates. + * Wrapper around Apollo `useQuery` that supports `DocumentNode` and `string` types. + * + * @param query GraphQL operation payload. + * @param options Operation variables and request configuration + * @returns GraphQL response + */ +export function useQuery( + query: RequestDocument, + options: QueryHookOptions +): QueryResult { + const documentNode = useDocumentNode(query) + return useApolloQuery(documentNode, options) +} + +/** + * Send a mutation to GraphQL and respond to updates. + * Wrapper around Apollo `useMutation` that supports `DocumentNode` and `string` types. + * + * @param mutation GraphQL operation payload. + * @param options Operation variables and request configuration + * @returns GraphQL response + */ +export function useMutation( + mutation: RequestDocument, + options?: MutationHookOptions +): MutationTuple { + const documentNode = useDocumentNode(mutation) + return useApolloMutation(documentNode, options) +} diff --git a/client/shared/src/polyfills/polyfill.ts b/client/shared/src/polyfills/polyfill.ts index 68336a749f448..d15e6e8ea5b15 100644 --- a/client/shared/src/polyfills/polyfill.ts +++ b/client/shared/src/polyfills/polyfill.ts @@ -1,2 +1,4 @@ // This gets expanded into only the imports we need by @babel/preset-env import 'core-js/stable' +// Avoids issues with RxJS interop +import 'core-js/features/symbol/observable' diff --git a/client/web/src/SourcegraphWebApp.tsx b/client/web/src/SourcegraphWebApp.tsx index 8c4c8abaa4f76..ff12df7bf3253 100644 --- a/client/web/src/SourcegraphWebApp.tsx +++ b/client/web/src/SourcegraphWebApp.tsx @@ -1,5 +1,6 @@ import 'focus-visible' +import { ApolloProvider } from '@apollo/client' import { ShortcutProvider } from '@slimsag/react-shortcuts' import ServerIcon from 'mdi-react/ServerIcon' import * as React from 'react' @@ -32,6 +33,7 @@ import { } from '@sourcegraph/shared/src/util/useRedesignToggle' import { authenticatedUser, AuthenticatedUser } from './auth' +import { client } from './backend/graphql' import { ErrorBoundary } from './components/ErrorBoundary' import { queryExternalServices } from './components/externalServices/backend' import { FeedbackText } from './components/FeedbackText' @@ -504,89 +506,91 @@ class ColdSourcegraphWebApp extends React.Component - - - ( - - - - )} + + + + + ( + + + + )} + /> + + + - - - - - + + + ) } diff --git a/client/web/src/backend/graphql.ts b/client/web/src/backend/graphql.ts index c608bd0026b81..bd9c6b4b9fe9f 100644 --- a/client/web/src/backend/graphql.ts +++ b/client/web/src/backend/graphql.ts @@ -1,6 +1,6 @@ import { Observable } from 'rxjs' -import { GraphQLResult, requestGraphQLCommon } from '@sourcegraph/shared/src/graphql/graphql' +import { graphQLClient, GraphQLResult, requestGraphQLCommon } from '@sourcegraph/shared/src/graphql/graphql' import * as GQL from '@sourcegraph/shared/src/graphql/schema' const getHeaders = (): { [header: string]: string } => ({ @@ -60,3 +60,5 @@ export const mutateGraphQL = (request: string, variables?: {}): Observable ({ + UserAreaUserProfile: () => ({ user: { __typename: 'User', id: 'user123', diff --git a/client/web/src/integration/profile.test.ts b/client/web/src/integration/profile.test.ts index 171dd429e6c8f..6b95ab4c8f2ae 100644 --- a/client/web/src/integration/profile.test.ts +++ b/client/web/src/integration/profile.test.ts @@ -45,7 +45,7 @@ describe('User profile page', () => { } testContext.overrideGraphQL({ ...commonWebGraphQlResults, - UserArea: () => ({ + UserAreaUserProfile: () => ({ user: USER, }), UpdateUser: () => ({ updateUser: { ...USER, displayName: 'Test2' } }), diff --git a/client/web/src/integration/settings.test.ts b/client/web/src/integration/settings.test.ts index 93027cd89f722..78e5dbff4b88f 100644 --- a/client/web/src/integration/settings.test.ts +++ b/client/web/src/integration/settings.test.ts @@ -53,7 +53,7 @@ describe('Settings', () => { }, }, }), - UserArea: () => ({ + UserAreaUserProfile: () => ({ user: { __typename: 'User', id: testUserID, diff --git a/client/web/src/site-admin/overview/SiteAdminOverviewPage.tsx b/client/web/src/site-admin/overview/SiteAdminOverviewPage.tsx index 69807d7920970..4464091414a6c 100644 --- a/client/web/src/site-admin/overview/SiteAdminOverviewPage.tsx +++ b/client/web/src/site-admin/overview/SiteAdminOverviewPage.tsx @@ -1,4 +1,3 @@ -import * as H from 'history' import OpenInNewIcon from 'mdi-react/OpenInNewIcon' import React, { useEffect, useMemo } from 'react' import { Observable, of } from 'rxjs' @@ -24,7 +23,6 @@ import { eventLogger } from '../../tracking/eventLogger' import { UsageChart } from '../SiteAdminUsageStatisticsPage' interface Props extends ActivationProps, ThemeProps { - history: H.History overviewComponents: readonly React.ComponentType[] /** For testing only */ @@ -112,7 +110,6 @@ const fetchWeeklyActiveUsers = (): Observable => * A page displaying an overview of site admin information. */ export const SiteAdminOverviewPage: React.FunctionComponent = ({ - history, isLightTheme, activation, overviewComponents, @@ -170,7 +167,6 @@ export const SiteAdminOverviewPage: React.FunctionComponent = ({ > {activation.completed && ( => - requestGraphQL( - gql` - query UserArea($username: String!, $siteAdmin: Boolean!) { - user(username: $username) { - ...UserAreaUserFields - ...EditUserProfilePage - } - } - ${UserAreaGQLFragment} - `, - args - ).pipe( - map(dataOrThrowErrors), - map(data => { - if (!data.user) { - throw new Error(`User not found: ${JSON.stringify(args.username)}`) - } - return data.user - }) - ) +export const USER_AREA_USER_PROFILE = gql` + query UserAreaUserProfile($username: String!, $siteAdmin: Boolean!) { + user(username: $username) { + ...UserAreaUserFields + } + } + ${UserAreaGQLFragment} +` export interface UserAreaRoute extends RouteDescriptor {} @@ -137,9 +120,6 @@ export interface UserAreaRouteContext */ user: UserAreaUserFields - /** Called when the user is updated, with the full new user data. */ - onUserUpdate: (newUser: UserAreaUserFields) => void - /** * The currently authenticated user, NOT (necessarily) the user who is the subject of the page. * @@ -165,43 +145,44 @@ export const UserArea: React.FunctionComponent = ({ }, ...props }) => { - // When onUserUpdate is called (e.g., when updating a user's profile), we want to immediately - // use the newly updated user data instead of re-querying it. Therefore, we store the user in - // state. The initial GraphQL query populates it, and onUserUpdate calls update it. - const [user, setUser] = useState() - useObservable( - useMemo( - () => - fetchUser({ - username, - siteAdmin: Boolean(props.authenticatedUser?.siteAdmin), - }).pipe(tap(setUser)), - [props.authenticatedUser?.siteAdmin, username] - ) + const { data, error, loading } = useQuery( + USER_AREA_USER_PROFILE, + { + variables: { username, siteAdmin: Boolean(props.authenticatedUser?.siteAdmin) }, + } ) const childBreadcrumbSetters = useBreadcrumb( useMemo( () => - user + data?.user ? { key: 'UserArea', - link: { to: user.url, label: user.username }, + link: { to: data.user.url, label: data.user.username }, } : null, - [user] + [data] ) ) - if (user === undefined) { - return null // loading + if (loading) { + return null + } + + if (error) { + throw new Error(error.message) + } + + const user = data?.user + + if (!user) { + throw new Error(`User not found: ${JSON.stringify(username)}`) } const context: UserAreaRouteContext = { ...props, url, user, - onUserUpdate: setUser, namespace: user, ...childBreadcrumbSetters, } diff --git a/client/web/src/user/settings/profile/EditUserProfileForm.test.tsx b/client/web/src/user/settings/profile/EditUserProfileForm.test.tsx deleted file mode 100644 index b6f01a65a3788..0000000000000 --- a/client/web/src/user/settings/profile/EditUserProfileForm.test.tsx +++ /dev/null @@ -1,21 +0,0 @@ -import { mount } from 'enzyme' -import React from 'react' - -import { EditUserProfileForm } from './EditUserProfileForm' - -jest.mock('./UserProfileFormFields', () => ({ UserProfileFormFields: 'mock-UserProfileFormFields' })) - -describe('EditUserProfileForm', () => { - test('simple', () => - expect( - mount( - {}} - after="AFTER" - /> - ) - ).toMatchSnapshot()) -}) diff --git a/client/web/src/user/settings/profile/EditUserProfileForm.tsx b/client/web/src/user/settings/profile/EditUserProfileForm.tsx index 97c139595de51..70d4655e45467 100644 --- a/client/web/src/user/settings/profile/EditUserProfileForm.tsx +++ b/client/web/src/user/settings/profile/EditUserProfileForm.tsx @@ -1,113 +1,95 @@ import React, { useCallback, useState } from 'react' -import { map } from 'rxjs/operators' +import { useHistory } from 'react-router' import { Form } from '@sourcegraph/branded/src/components/Form' -import { dataOrThrowErrors, gql } from '@sourcegraph/shared/src/graphql/graphql' +import { gql, useMutation } from '@sourcegraph/shared/src/graphql/graphql' import * as GQL from '@sourcegraph/shared/src/graphql/schema' -import { isErrorLike } from '@sourcegraph/shared/src/util/errors' import { Container } from '@sourcegraph/wildcard' -import { AuthenticatedUser } from '../../../auth' -import { requestGraphQL } from '../../../backend/graphql' -import { UpdateUserResult, UpdateUserVariables, UserAreaUserFields } from '../../../graphql-operations' +import { refreshAuthenticatedUser } from '../../../auth' +import { UpdateUserResult, UpdateUserVariables } from '../../../graphql-operations' import { eventLogger } from '../../../tracking/eventLogger' -import { UserAreaGQLFragment } from '../../area/UserArea' import { UserProfileFormFields, UserProfileFormFieldsValue } from './UserProfileFormFields' +export const UPDATE_USER = gql` + mutation UpdateUser($user: ID!, $username: String!, $displayName: String, $avatarURL: String) { + updateUser(user: $user, username: $username, displayName: $displayName, avatarURL: $avatarURL) { + id + username + displayName + avatarURL + } + } +` + interface Props { - authenticatedUser: Pick user: Pick - initialValue: UserProfileFormFieldsValue - - /** Called when the user is successfully updated. */ - onUpdate: (newValue: UserAreaUserFields) => void - after?: React.ReactFragment } /** * A form to edit a user's profile. */ -export const EditUserProfileForm: React.FunctionComponent = ({ - user, - authenticatedUser, - initialValue, - onUpdate, - after, -}) => { - const [value, setValue] = useState(initialValue) +export const EditUserProfileForm: React.FunctionComponent = ({ user, initialValue, after }) => { + const history = useHistory() + const [updateUser, { data, loading, error }] = useMutation(UPDATE_USER, { + onCompleted: ({ updateUser }) => { + eventLogger.log('UserProfileUpdated') + history.replace(`/users/${updateUser.username}/settings/profile`) + + // In case the edited user is the current user, immediately reflect the changes in the + // UI. + // TODO: Migrate this to use the Apollo cache + refreshAuthenticatedUser() + .toPromise() + .finally(() => {}) + }, + onError: () => eventLogger.log('UpdateUserFailed'), + }) + + const [userFields, setUserFields] = useState(initialValue) const onChange = useCallback['onChange']>( - newValue => setValue(previous => ({ ...previous, ...newValue })), + newValue => setUserFields(previous => ({ ...previous, ...newValue })), [] ) - /** Operation state: false (initial state), true (loading), Error, or 'success'. */ - const [opState, setOpState] = useState(false) const onSubmit = useCallback( - async event => { + event => { event.preventDefault() - setOpState(true) eventLogger.log('UpdateUserClicked') - try { - const updatedUser = await requestGraphQL( - gql` - mutation UpdateUser( - $user: ID! - $username: String! - $displayName: String - $avatarURL: String - $siteAdmin: Boolean! - ) { - updateUser( - user: $user - username: $username - displayName: $displayName - avatarURL: $avatarURL - ) { - ...UserAreaUserFields - } - } - ${UserAreaGQLFragment} - `, - { ...value, user: user.id, siteAdmin: authenticatedUser.siteAdmin } - ) - .pipe( - map(dataOrThrowErrors), - map(data => data.updateUser) - ) - .toPromise() - eventLogger.log('UserProfileUpdated') - setOpState('success') - onUpdate(updatedUser) - } catch (error) { - eventLogger.log('UpdateUserFailed') - setOpState(error) - } + return updateUser({ + variables: { + user: user.id, + username: userFields.username, + displayName: userFields.displayName, + avatarURL: userFields.avatarURL, + }, + }) }, - [onUpdate, user.id, authenticatedUser.siteAdmin, value] + [updateUser, user.id, userFields] ) return ( Save - {isErrorLike(opState) && {opState.message}} - {opState === 'success' && ( + {error && {error.message}} + {data?.updateUser && ( User profile updated. diff --git a/client/web/src/user/settings/profile/UserProfileFormFields.test.tsx b/client/web/src/user/settings/profile/UserProfileFormFields.test.tsx deleted file mode 100644 index 6ec8b86c750af..0000000000000 --- a/client/web/src/user/settings/profile/UserProfileFormFields.test.tsx +++ /dev/null @@ -1,16 +0,0 @@ -import { mount } from 'enzyme' -import React from 'react' - -import { UserProfileFormFields } from './UserProfileFormFields' - -describe('UserProfileFormFields', () => { - test('simple', () => - expect( - mount( - {}} - /> - ) - ).toMatchSnapshot()) -}) diff --git a/client/web/src/user/settings/profile/UserSettingsProfilePage.test.tsx b/client/web/src/user/settings/profile/UserSettingsProfilePage.test.tsx new file mode 100644 index 0000000000000..9cb0aeb042bb0 --- /dev/null +++ b/client/web/src/user/settings/profile/UserSettingsProfilePage.test.tsx @@ -0,0 +1,107 @@ +import { MockedProvider, MockedResponse } from '@apollo/client/testing' +import { fireEvent, render, RenderResult, act } from '@testing-library/react' +import React from 'react' +import { MemoryRouter } from 'react-router' + +import { getDocumentNode } from '@sourcegraph/shared/src/graphql/graphql' + +import { UPDATE_USER } from './EditUserProfileForm' +import { UserSettingsProfilePage } from './UserSettingsProfilePage' + +const mockUser = { + id: 'x', + username: 'initial-username', + displayName: 'Initial Name', + avatarURL: 'https://example.com/image.jpg', + viewerCanChangeUsername: true, + createdAt: new Date().toISOString(), +} + +const newUserValues = { + username: 'new-username', + displayName: 'New Name', + avatarURL: 'https://example.com/other-image.jpg', +} + +const mocks: readonly MockedResponse[] = [ + { + request: { + query: getDocumentNode(UPDATE_USER), + variables: { + user: 'x', + ...newUserValues, + }, + }, + result: { + data: { + updateUser: { + id: 'x', + ...newUserValues, + }, + }, + }, + }, +] + +describe('UserSettingsProfilePage', () => { + let queries: RenderResult + + beforeAll(() => { + window.context = { sourcegraphDotComMode: false } as any + }) + + beforeEach(() => { + queries = render( + + + + + + ) + }) + + it('renders header correctly', () => { + const heading = queries.getByRole('heading', { level: 2 }) + expect(heading).toBeVisible() + expect(heading).toHaveTextContent('Profile') + }) + + it('renders username field correctly', () => { + const usernameField = queries.getByLabelText('Username') + expect(usernameField).toBeVisible() + expect(usernameField).toHaveValue(mockUser.username) + }) + + it('renders display name field correctly', () => { + const displayNameField = queries.getByLabelText('Display name') + expect(displayNameField).toBeVisible() + expect(displayNameField).toHaveValue(mockUser.displayName) + }) + + it('renders avatar URL field correctly', () => { + const avatarURLField = queries.getByLabelText('Avatar URL') + expect(avatarURLField).toBeVisible() + expect(avatarURLField).toHaveValue(mockUser.avatarURL) + }) + + describe('modifying values', () => { + it('updates values correctly', async () => { + const usernameField = queries.getByLabelText('Username') + const displayNameField = queries.getByLabelText('Display name') + const avatarURLField = queries.getByLabelText('Avatar URL') + + fireEvent.change(usernameField, { target: { value: newUserValues.username } }) + fireEvent.change(displayNameField, { target: { value: newUserValues.displayName } }) + fireEvent.change(avatarURLField, { target: { value: newUserValues.avatarURL } }) + fireEvent.click(queries.getByText('Save')) + + // Wait next tick to skip loading state + await act(() => new Promise(resolve => setTimeout(resolve, 0))) + + expect(queries.getByText('User profile updated.')).toBeVisible() + expect(usernameField).toHaveValue(newUserValues.username) + expect(displayNameField).toHaveValue(newUserValues.displayName) + expect(avatarURLField).toHaveValue(newUserValues.avatarURL) + }) + }) +}) diff --git a/client/web/src/user/settings/profile/UserSettingsProfilePage.tsx b/client/web/src/user/settings/profile/UserSettingsProfilePage.tsx index 69e715e0c8f7e..a39bdf12c5775 100644 --- a/client/web/src/user/settings/profile/UserSettingsProfilePage.tsx +++ b/client/web/src/user/settings/profile/UserSettingsProfilePage.tsx @@ -1,13 +1,10 @@ -import H from 'history' -import React, { useCallback, useEffect } from 'react' +import React, { useEffect } from 'react' import { percentageDone } from '@sourcegraph/shared/src/components/activation/Activation' import { ActivationChecklist } from '@sourcegraph/shared/src/components/activation/ActivationChecklist' import { gql } from '@sourcegraph/shared/src/graphql/graphql' -import { isErrorLike } from '@sourcegraph/shared/src/util/errors' import { Container, PageHeader } from '@sourcegraph/wildcard' -import { refreshAuthenticatedUser } from '../../../auth' import { PageTitle } from '../../../components/PageTitle' import { Timestamp } from '../../../components/time/Timestamp' import { EditUserProfilePage as EditUserProfilePageFragment } from '../../../graphql-operations' @@ -27,39 +24,13 @@ export const EditUserProfilePageGQLFragment = gql` } ` -interface Props extends Pick { +interface Props extends Pick { user: EditUserProfilePageFragment - - history: H.History - location: H.Location } -export const UserSettingsProfilePage: React.FunctionComponent = ({ - user, - authenticatedUser, - onUserUpdate: parentOnUpdate, - ...props -}) => { +export const UserSettingsProfilePage: React.FunctionComponent = ({ user, ...props }) => { useEffect(() => eventLogger.logViewEvent('UserProfile'), []) - const onUpdate = useCallback['onUpdate']>( - newValue => { - // Handle when username changes. - if (newValue.username !== user.username) { - props.history.push(`/users/${newValue.username}/settings/profile`) - } - - parentOnUpdate(newValue) - - // In case the edited user is the current user, immediately reflect the changes in the - // UI. - refreshAuthenticatedUser() - .toPromise() - .finally(() => {}) - }, - [parentOnUpdate, props.history, user.username] - ) - return ( @@ -84,19 +55,13 @@ export const UserSettingsProfilePage: React.FunctionComponent = ({ Almost there! Complete the steps below to finish onboarding to Sourcegraph. - + )} - {user && !isErrorLike(user) && ( + {user && ( diff --git a/client/web/src/user/settings/profile/__snapshots__/EditUserProfileForm.test.tsx.snap b/client/web/src/user/settings/profile/__snapshots__/EditUserProfileForm.test.tsx.snap deleted file mode 100644 index 38738e735f458..0000000000000 --- a/client/web/src/user/settings/profile/__snapshots__/EditUserProfileForm.test.tsx.snap +++ /dev/null @@ -1,68 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`EditUserProfileForm simple 1`] = ` - - - - - - - - Save - - - AFTER - - - - - -`; diff --git a/client/web/src/user/settings/profile/__snapshots__/UserProfileFormFields.test.tsx.snap b/client/web/src/user/settings/profile/__snapshots__/UserProfileFormFields.test.tsx.snap deleted file mode 100644 index d9138a50b98ad..0000000000000 --- a/client/web/src/user/settings/profile/__snapshots__/UserProfileFormFields.test.tsx.snap +++ /dev/null @@ -1,117 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`UserProfileFormFields simple 1`] = ` - - - - - Username - - - - - - A username consists of letters, numbers, hyphens (-), dots (.) and may not begin or end with a dot, nor begin with a hyphen. - - - - - Display name - - - - - - - Avatar URL - - - - - - - - - -`; diff --git a/doc/dev/background-information/web/graphql.md b/doc/dev/background-information/web/graphql.md index 11a5e33ccec71..02e7cbc35cd15 100644 --- a/doc/dev/background-information/web/graphql.md +++ b/doc/dev/background-information/web/graphql.md @@ -1,33 +1,179 @@ -# Working with GraphQL queries +# Working with GraphQL The webapp and browser extension interact with our backend through a strongly typed GraphQL API. We auto-generate TypeScript types for the schema and all queries, mutations and fragments to ensure the client uses the API correctly. -## Writing a type-safe GraphQL query +## GraphQL Client +We use [Apollo Client](https://www.apollographql.com/docs/react/) to manage data-fetching and caching within our app. It provides a set of declarative interfaces which abstract away a lot of repeated code, whilst supporting a 'stale-while-revalidate' strategy through a normalized client cache of requested data. -Write GraphQL queries in plain template strings and tag them with the `gql` template string tag. -Each query must also have a globally unique name. -This makes sure the query gets picked up by the type generation and you receive autocompletion, syntax highlighting, hover tooltips and validation. +**Writing and running your query** -The preferred way to get a typed result from a GraphQL query or mutation is to pass the auto-generated interface for the query result and variables as type parameters to `requestGraphQL()`: +We use `gql` template strings to declare our GraphQL queries. + +Each query must have a globally unique name as per the [GraphQL specification](https://spec.graphql.org/June2018/#sec-Operation-Name-Uniqueness). Typically we should name our queries similarly to how we might name a function, by describing what the query will return. For mutations, we should prefix the name with a verb like `Delete` or `Update`, this will help avoid collisions between queries and mutations. + +Using each unique query, we can generate specific types so you can receive autocompletion, syntax highlighting, hover tooltips and validation in your IDE. + +Once you have built your query, `graphql-codegen` will generate the correct request and response types. This process should happen automatically through local development, you can also manually trigger this by running `yarn generate` or `yarn watch-generate`. + +Using a `useQuery` hook, we can easily fire a request and handle the response correctly. ```ts -import { DemoResult, DemoVariables} from '../graphql-operations' +// ./MyComponent.tsx +import { useQuery, gql } from '@sourcegraph/shared/src/graphql/graphql' + +import { UserDisplayNameResult, UserDisplayNameVariables } from '../../graphql-operations' + +export const USER_DISPLAY_NAME = gql` + query UserDisplayName($username: String!) { + user(username: $username) { + id + displayName + } + } +` -requestGraphQl(gql` - query Demo($input: String) { - foo(input: $input) { - bar - } +const MyComponent = ({ username }: { username: string }) => { + const { data, loading, error } = useQuery(USER_DISPLAY_NAME, { variables: { username } }); + + if (loading) { + // handle loading state + } + + if (error) { + // handle error state + } + + if (data) { + // display to user + } +} +``` + +Equally, it is possible to create our own wrapper hooks, when we want to modify data accordingly. + +```ts +const useFullName = (variables: UserDisplayNameVariables): ApolloQueryResult<{ fullName: string }> => { + const response = useQuery(USER_DISPLAY_NAME, { variables }) + + if (response.error) { + // Handle error + } + + return { + ...response, + data: { + fullName: `${response.data.user.firstName} ${response.data.user.lastName}`, + }, } -`, { - input: 'Hello world' +} +``` + +## Frequently asked questions + +### How do I use the Apollo cache? +Apollo uses a normalized in-memory cache to store the results of different queries, most of this happens automatically! + +Apollo generates a composite key for each identifiable object in a response. This is typically done by combining the `__typeName` with the `id` that is returned in the response. For example: + +When firing this request: +``` +{ + user { + __typename + id + displayName + } +} +``` + +Assuming `__typename === User` and `id === 1234`, the response data is added to the cache under the key: +`User:1234`. + +When a different query requests similar data, Apollo will merge both responses into the cache and update both parts of the UI with the latest data. This is useful to form links between components whilst still ensuring that the components are self-contained. For example, if a mutation updated `displayName` in a different component, then these components that *query* `displayName` would automatically receive the updated value and re-render. + +**All queries should return an object identifier**. If this identifier is not under the `id` field, we need to inform Apollo which field to use in order to generate the correct key. See the [docs](https://www.apollographql.com/docs/react/caching/cache-configuration/#customizing-identifier-generation-by-type) for more information on this. + +### How should I write tests that handle data-fetching? +Apollo lets us easily mock queries in our tests without having to actually mock out our own logic. The tests will fail if an un-mocked query fires through Apollo, so it is important to accurately build mock requests. In order to test how the UI displays a response, you can provide a mocked result. See this example: + +```ts +import { MockedProvider } from '@apollo/client/testing' +import { render } from '@testing-library/react' + +import { getDocumentNode } from '@sourcegraph/shared/src/graphql/graphql' + +import { MyComponent, USER_DISPLAY_NAME } from './MyComponent' + +const mocks = [ + { + request: { + query: getDocumentNode(USER_DISPLAY_NAME), + variables: { + username: 'mock_username', + }, + }, + result: { + data: { + user: { + displayName: 'Mock DisplayName', + }, + }, + }, + }, +] + +describe('My Test', () => { + it('works', () => { + const { getByText } = render( + + + + ) + expect(getByText('Your display name is: Mock DisplayName')).toBeVisible(); + }) }) ``` -**Note**: A lot of older code uses the non-generic `queryGraphQl()` and `mutateGraphQl()` functions. +### How can I run a query outside of React? +Most queries should be requested in the context of our UI and should use hooks. If there is a scenario where this is not possible, it is still possible to realise the benefits of Apollo without relying this approach. We can imperatively trigger any query using `client.query`. + +```ts +import { getDocumentNode } from '@sourcegraph/shared/src/graphql/graphql' + +import { client } from './backend/graphql' +import { + UserDisplayNameResult, + UserDisplayNameVariables, +} from './graphql-operations' + +const getUserDisplayName = async (username: string): Promise => { + const { data, error } = await client.query({ + query: getDocumentNode(UserDisplayName), + variables: { username }, + }) + + if (error) { + // handle error + } + + return data +} +``` + +### I have an issue, how can I debug? +Aside from typical debugging methods, Apollo provides [Apollo Client Devtools](https://www.apollographql.com/docs/react/development-testing/developer-tooling/#apollo-client-devtools) as a browser extension to help with debugging. This extension will automatically track query requests and responses, and provide a visual representation of cached data. + +### We have different ways of requesting data from GraphQL, why? +Our code isn't yet fully aligned to a single-approach, but this is something we are working towards over time. + +A lot of older code uses the non-generic `queryGraphQl()` and `mutateGraphQl()` functions. These are less type-safe, because they return schema types with _all_ fields of the schema present, no matter whether they were queried or not. +Other code uses `requestGraphQL()`, this is an improved approach which provides better types, but it doesn't scale well when requesting data across multiple areas of the application, and often leads to cross-component dependencies. + +Our goal is to migrate more code to use Apollo. This should make our components more self-contained, increase perceived performance with client-side caching and make it easier to write effective tests. + ## Writing a React component or function that takes an API object as input React components are often structured to display a subfield of a query. @@ -35,7 +181,7 @@ The best way to declare this input type is to define a _GraphQL fragment_ with t This ensures the parent component don't forget to query a required field and it makes it easy to hard-code stub results in tests. ```tsx -import { PersionFields } from '../graphql-operations' +import { PersonFields } from '../graphql-operations' export const personFields = gql` fragment PersonFields on Person { @@ -43,7 +189,7 @@ export const personFields = gql` } ` -export const Greeting: React.FunctionComponent<{ person: PersionFields }> = ({ person }) => +export const Greeting: React.FunctionComponent<{ person: PersonFields }> = ({ person }) => Hello, {person.name}! ``` @@ -52,7 +198,7 @@ Since the fragment is exported, parent components can use it in their queries to ```ts import { personFields } from './greeting', -requestGraphQl(gql` +export const PEOPLE = gql` query People { people { nodes { @@ -61,7 +207,7 @@ requestGraphQl(gql` } } ${personFields} -`) +` ``` **Note**: A lot of older components still use all-fields types generated from the whole schema (as opposed to from a fragment), usually referenced from the namespace `GQL.*`. diff --git a/package.json b/package.json index fa6260d36692e..847bfec33b70f 100644 --- a/package.json +++ b/package.json @@ -304,6 +304,7 @@ "yarn-deduplicate": "^3.1.0" }, "dependencies": { + "@apollo/client": "^3.3.20", "@hot-loader/react-dom": "^16.14.0", "@reach/accordion": "^0.10.2", "@reach/combobox": "^0.15.1", diff --git a/yarn.lock b/yarn.lock index c92ce278be52a..d3c9acc81732b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -38,6 +38,25 @@ call-me-maybe "^1.0.1" js-yaml "^3.13.1" +"@apollo/client@^3.3.20": + version "3.3.20" + resolved "https://registry.npmjs.org/@apollo/client/-/client-3.3.20.tgz#8f0935fa991857e9cf2e73c9bd378ad7ec97caf8" + integrity sha512-hS7UmBwJweudw/J3M0RAcusMHNiRuGqkRH6g91PM2ev8cXScIMdXr/++9jo7wD1nAITMCMF4HQQ3LFaw/Or0Bw== + dependencies: + "@graphql-typed-document-node/core" "^3.0.0" + "@types/zen-observable" "^0.8.0" + "@wry/context" "^0.6.0" + "@wry/equality" "^0.5.0" + fast-json-stable-stringify "^2.0.0" + graphql-tag "^2.12.0" + hoist-non-react-statics "^3.3.2" + optimism "^0.16.0" + prop-types "^15.7.2" + symbol-observable "^4.0.0" + ts-invariant "^0.7.0" + tslib "^1.10.0" + zen-observable "^0.8.14" + "@ardatan/aggregate-error@0.0.1": version "0.0.1" resolved "https://registry.npmjs.org/@ardatan/aggregate-error/-/aggregate-error-0.0.1.tgz#1403ac5de10d8ca689fc1f65844c27179ae1d44f" @@ -1996,6 +2015,11 @@ is-promise "4.0.0" tslib "~2.0.1" +"@graphql-typed-document-node/core@^3.0.0": + version "3.1.0" + resolved "https://registry.npmjs.org/@graphql-typed-document-node/core/-/core-3.1.0.tgz#0eee6373e11418bfe0b5638f654df7a4ca6a3950" + integrity sha512-wYn6r8zVZyQJ6rQaALBEln5B1pzxb9shV5Ef97kTvn6yVGrqyXVnDqnU24MXnFubR+rZjBY9NWuxX3FB2sTsjg== + "@hot-loader/react-dom@^16.14.0": version "16.14.0" resolved "https://registry.npmjs.org/@hot-loader/react-dom/-/react-dom-16.14.0.tgz#3cfc64e40bb78fa623e59b582b8f09dcdaad648a" @@ -5081,6 +5105,11 @@ dependencies: "@types/node" "*" +"@types/zen-observable@^0.8.0": + version "0.8.2" + resolved "https://registry.npmjs.org/@types/zen-observable/-/zen-observable-0.8.2.tgz#808c9fa7e4517274ed555fa158f2de4b4f468e71" + integrity sha512-HrCIVMLjE1MOozVoD86622S7aunluLb2PJdPfb3nYiEtohm8mIB/vyv0Fd37AdeMFrTUQXEunw78YloMA3Qilg== + "@typescript-eslint/eslint-plugin@^4.9.1": version "4.9.1" resolved "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-4.9.1.tgz#66758cbe129b965fe9c63b04b405d0cf5280868b" @@ -5565,6 +5594,27 @@ resolved "https://registry.npmjs.org/@webpack-cli/serve/-/serve-1.3.1.tgz#911d1b3ff4a843304b9c3bacf67bb34672418441" integrity sha512-0qXvpeYO6vaNoRBI52/UsbcaBydJCggoBBnIo/ovQQdn6fug0BgwsjorV1hVS7fMqGVTZGcVxv8334gjmbj5hw== +"@wry/context@^0.6.0": + version "0.6.0" + resolved "https://registry.npmjs.org/@wry/context/-/context-0.6.0.tgz#f903eceb89d238ef7e8168ed30f4511f92d83e06" + integrity sha512-sAgendOXR8dM7stJw3FusRxFHF/ZinU0lffsA2YTyyIOfic86JX02qlPqPVqJNZJPAxFt+2EE8bvq6ZlS0Kf+Q== + dependencies: + tslib "^2.1.0" + +"@wry/equality@^0.5.0": + version "0.5.1" + resolved "https://registry.npmjs.org/@wry/equality/-/equality-0.5.1.tgz#b22e4e1674d7bf1439f8ccdccfd6a785f6de68b0" + integrity sha512-FZKbdpbcVcbDxQrKcaBClNsQaMg9nof1RKM7mReJe5DKUzM5u8S7T+PqwNqvib5O2j2xxF1R4p5O3+b6baTrbw== + dependencies: + tslib "^2.1.0" + +"@wry/trie@^0.3.0": + version "0.3.0" + resolved "https://registry.npmjs.org/@wry/trie/-/trie-0.3.0.tgz#3245e74988c4e3033299e479a1bf004430752463" + integrity sha512-Yw1akIogPhAT6XPYsRHlZZIS0tIGmAl9EYXHi2scf7LPKKqdqmow/Hu4kEqP2cJR3EjaU/9L0ZlAjFf3hFxmug== + dependencies: + tslib "^2.1.0" + "@xstate/fsm@^1.4.0": version "1.4.0" resolved "https://registry.npmjs.org/@xstate/fsm/-/fsm-1.4.0.tgz#6fd082336fde4d026e9e448576189ee5265fa51a" @@ -12461,10 +12511,12 @@ graphql-schema-linter@^2.0.1: glob "^7.1.2" graphql "^15.0.0" -graphql-tag@^2.11.0: - version "2.11.0" - resolved "https://registry.npmjs.org/graphql-tag/-/graphql-tag-2.11.0.tgz#1deb53a01c46a7eb401d6cb59dec86fa1cccbffd" - integrity sha512-VmsD5pJqWJnQZMUeRwrDhfgoyqcfwEkvtpANqcoUG8/tOLkwNgU9mzub/Mc78OJMhHjx7gfAMTxzdG43VGg3bA== +graphql-tag@^2.11.0, graphql-tag@^2.12.0: + version "2.12.4" + resolved "https://registry.npmjs.org/graphql-tag/-/graphql-tag-2.12.4.tgz#d34066688a4f09e72d6f4663c74211e9b4b7c4bf" + integrity sha512-VV1U4O+9x99EkNpNmCUV5RZwq6MnK4+pGbRYWG+lA/m3uo7TSqJF81OkcOP148gFP6fzdl7JWYBrwWVTS9jXww== + dependencies: + tslib "^2.1.0" graphql-upload@^11.0.0: version "11.0.0" @@ -12856,10 +12908,10 @@ hmac-drbg@^1.0.0: minimalistic-assert "^1.0.0" minimalistic-crypto-utils "^1.0.1" -hoist-non-react-statics@^3.1.0, hoist-non-react-statics@^3.3.0: - version "3.3.0" - resolved "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-3.3.0.tgz#b09178f0122184fb95acf525daaecb4d8f45958b" - integrity sha512-0XsbTXxgiaCDYDIWFcwkmerZPSwywfUqYmwT4jzewKTQSWoE6FCMoUVOeBJWK3E/CrWbxRG3m5GzY4lnIwGRBA== +hoist-non-react-statics@^3.1.0, hoist-non-react-statics@^3.3.0, hoist-non-react-statics@^3.3.2: + version "3.3.2" + resolved "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-3.3.2.tgz#ece0acaf71d62c2969c2ec59feff42a4b1a85b45" + integrity sha512-/gGivxi8JPKWNm/W0jSmzcMPpfpPLc3dY/6GxhX2hQ9iGj3aDfklV4ET7NjKpSinLpJ5vafa9iiGIEZg10SfBw== dependencies: react-is "^16.7.0" @@ -17157,6 +17209,14 @@ opn@^5.5.0: dependencies: is-wsl "^1.1.0" +optimism@^0.16.0: + version "0.16.1" + resolved "https://registry.npmjs.org/optimism/-/optimism-0.16.1.tgz#7c8efc1f3179f18307b887e18c15c5b7133f6e7d" + integrity sha512-64i+Uw3otrndfq5kaoGNoY7pvOhSsjFEN4bdEFh80MWVk/dbgJfMv7VFDeCT8LxNAlEVhQmdVEbfE7X2nWNIIg== + dependencies: + "@wry/context" "^0.6.0" + "@wry/trie" "^0.3.0" + optionator@^0.8.1, optionator@^0.8.2: version "0.8.3" resolved "https://registry.npmjs.org/optionator/-/optionator-0.8.3.tgz#84fa1d036fe9d3c7e21d99884b601167ec8fb495" @@ -21779,6 +21839,11 @@ symbol-observable@^1.1.0: resolved "https://registry.npmjs.org/symbol-observable/-/symbol-observable-1.2.0.tgz#c22688aed4eab3cdc2dfeacbb561660560a00804" integrity sha512-e900nM8RRtGhlV36KGEU9k65K3mPb1WV70OdjfxlG2EAuM1noi/E/BaW/uMhL7bPEssK8QV57vN3esixjUvcXQ== +symbol-observable@^4.0.0: + version "4.0.0" + resolved "https://registry.npmjs.org/symbol-observable/-/symbol-observable-4.0.0.tgz#5b425f192279e87f2f9b937ac8540d1984b39205" + integrity sha512-b19dMThMV4HVFynSAM1++gBHAbk2Tc/osgLIBZMKsyqh34jb2e8Os7T6ZW/Bt3pJFdBTd2JwAnAAEQV7rSNvcQ== + symbol-tree@^3.2.2: version "3.2.2" resolved "https://registry.npmjs.org/symbol-tree/-/symbol-tree-3.2.2.tgz#ae27db38f660a7ae2e1c3b7d1bc290819b8519e6" @@ -22340,6 +22405,13 @@ ts-essentials@^2.0.3: resolved "https://registry.npmjs.org/ts-essentials/-/ts-essentials-2.0.12.tgz#c9303f3d74f75fa7528c3d49b80e089ab09d8745" integrity sha512-3IVX4nI6B5cc31/GFFE+i8ey/N2eA0CZDbo6n0yrz0zDX8ZJ8djmU1p+XRz7G3is0F3bB3pu2pAroFdAWQKU3w== +ts-invariant@^0.7.0: + version "0.7.5" + resolved "https://registry.npmjs.org/ts-invariant/-/ts-invariant-0.7.5.tgz#f9658719f9a7737b117d09820d952aacf6263f9c" + integrity sha512-qfVyqTYWEqADMtncLqwpUdMjMSXnsqOeqGtj1LeJNFDjz8oqZ1YxLEp29YCOq65z0LgEiERqQ8ThVjnfibJNpg== + dependencies: + tslib "^2.1.0" + ts-key-enum@^2.0.0, ts-key-enum@^2.0.7: version "2.0.7" resolved "https://registry.npmjs.org/ts-key-enum/-/ts-key-enum-2.0.7.tgz#ea2c6f3bd770257a3d70fd60625bc3e99d19ab1e" @@ -24149,6 +24221,11 @@ yocto-queue@^0.1.0: resolved "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== +zen-observable@^0.8.14: + version "0.8.15" + resolved "https://registry.npmjs.org/zen-observable/-/zen-observable-0.8.15.tgz#96415c512d8e3ffd920afd3889604e30b9eaac15" + integrity sha512-PQ2PC7R9rslx84ndNBZB/Dkv8V8fZEpk83RLgXtYd0fwUgEjseMn1Dgajh2x6S8QbZAFa9p2qVCEuYZNgve0dQ== + zip-dir@1.0.2: version "1.0.2" resolved "https://registry.npmjs.org/zip-dir/-/zip-dir-1.0.2.tgz#253f907aead62a21acd8721d8b88032b2411c051"
Complete the steps below to finish onboarding to Sourcegraph.