Revert "Revert "app: open external and help links in system default browser"" (#51421)

This reverts commit af29d8e634.

The failure was happening because external service instructions were
created statically instead of being rendered when needed, so the correct
Link component was not yet set. The second commit in this PR addresses
the issue and converts all external service instructions into function
components that use the right Link components set at application start.

## Test plan

Verify main-dry-run passes
This commit is contained in:
Juliana Peña 2023-05-03 12:32:57 -07:00 committed by GitHub
parent dbb0a2e3d1
commit df50e67d80
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 140 additions and 68 deletions

View File

@ -138,6 +138,7 @@ ts_project(
"src/api/ApiConsole.tsx",
"src/api/ApiConsoleToolbar.tsx",
"src/api/NoApiConsole.tsx",
"src/app/TauriLink.tsx",
"src/app/TauriNavigation.tsx",
"src/app/useHistoryStack.tsx",
"src/auth.ts",

View File

@ -37,7 +37,7 @@ import {
} from '@sourcegraph/shared/src/settings/settings'
import { TemporarySettingsProvider } from '@sourcegraph/shared/src/settings/temporary/TemporarySettingsProvider'
import { TemporarySettingsStorage } from '@sourcegraph/shared/src/settings/temporary/TemporarySettingsStorage'
import { setLinkComponent, RouterLink, WildcardThemeContext, WildcardTheme } from '@sourcegraph/wildcard'
import { WildcardThemeContext, WildcardTheme } from '@sourcegraph/wildcard'
import { authenticatedUser as authenticatedUserSubject, AuthenticatedUser, authenticatedUserValue } from './auth'
import { getWebGraphQLClient } from './backend/graphql'
@ -82,8 +82,6 @@ const WILDCARD_THEME: WildcardTheme = {
isBranded: true,
}
setLinkComponent(RouterLink)
/**
* The root component.
*/

View File

@ -27,7 +27,7 @@ import {
SettingsSubjectCommonFields,
} from '@sourcegraph/shared/src/settings/settings'
import { TemporarySettingsProvider } from '@sourcegraph/shared/src/settings/temporary/TemporarySettingsProvider'
import { setLinkComponent, RouterLink, WildcardThemeContext, WildcardTheme } from '@sourcegraph/wildcard'
import { WildcardThemeContext, WildcardTheme } from '@sourcegraph/wildcard'
import { authenticatedUser as authenticatedUserSubject, AuthenticatedUser, authenticatedUserValue } from './auth'
import { ComponentsComposer } from './components/ComponentsComposer'
@ -89,8 +89,6 @@ const WILDCARD_THEME: WildcardTheme = {
isBranded: true,
}
setLinkComponent(RouterLink)
const suspenseCache = new SuspenseCache()
/**

View File

@ -0,0 +1,56 @@
// Since we're using forwardRef for everything in this file, we need
// to forward all the props to the underlying component.
/* eslint-disable no-restricted-syntax */
import React, { useMemo } from 'react'
import isAbsoluteUrl from 'is-absolute-url'
import { addSourcegraphAppOutboundUrlParameters } from '@sourcegraph/shared/src/util/url'
import { AnchorLink, Link, RouterLink } from '@sourcegraph/wildcard'
/**
* A link that opens in the browser if the URL is absolute, otherwise uses the router.
* If the URL is a help page, it will open in the browser with the appropriate outbound URL parameters.
*
* With the `shell-open` feature enabled in the Tauri app, `target="_blank"` links will automatically
* open in the user's default browser; there is no need to call the Tauri API directly.
*/
export const TauriLink = React.forwardRef(({ to, children, ...rest }, reference) => {
if (to && isAbsoluteUrl(to)) {
return (
<AnchorLink {...rest} to={to} ref={reference} target="_blank">
{children}
</AnchorLink>
)
}
if (to?.startsWith('/help/') || to === '/help') {
return (
<TauriHelpLink {...rest} to={to} ref={reference}>
{children}
</TauriHelpLink>
)
}
return (
<RouterLink {...rest} to={to} ref={reference}>
{children}
</RouterLink>
)
}) as Link
const TauriHelpLink = React.forwardRef(function TauriHelpLink({ to, children, ...rest }, reference) {
const toWithParams = useMemo(() => {
const absoluteTo = to.replace(/^\/help/, 'https://docs.sourcegraph.com')
return addSourcegraphAppOutboundUrlParameters(absoluteTo)
}, [to])
return (
<AnchorLink {...rest} to={toWithParams} ref={reference} target="_blank">
{children}
</AnchorLink>
)
}) as Link
TauriLink.displayName = 'TauriLink'

View File

@ -38,6 +38,7 @@ export const AddExternalServicePage: FC<Props> = ({
const [config, setConfig] = useState(externalService.defaultConfig)
const [displayName, setDisplayName] = useState(externalService.defaultDisplayName)
const navigate = useNavigate()
const { Instructions } = externalService
useEffect(() => {
telemetryService.logPageView('AddExternalService')
@ -111,8 +112,14 @@ export const AddExternalServicePage: FC<Props> = ({
<div className="mb-3">
<ExternalServiceCard {...externalService} />
</div>
<H3>Instructions:</H3>
<div className="mb-4">{externalService.instructions}</div>
{Instructions && (
<>
<H3>Instructions:</H3>
<div className="mb-4">
<Instructions />
</div>
</>
)}
<ExternalServiceForm
telemetryService={telemetryService}
error={error}

View File

@ -16,7 +16,7 @@ import NpmIcon from 'mdi-react/NpmIcon'
import { hasProperty } from '@sourcegraph/common'
import { PerforceIcon, PhabricatorIcon } from '@sourcegraph/shared/src/components/icons'
import { Link, Code, Text, setLinkComponent, RouterLink } from '@sourcegraph/wildcard'
import { Link, Code, Text } from '@sourcegraph/wildcard'
import awsCodeCommitSchemaJSON from '../../../../../schema/aws_codecommit.schema.json'
import azureDevOpsSchemaJSON from '../../../../../schema/azuredevops.schema.json'
@ -46,8 +46,6 @@ import { EditorAction } from '../../settings/EditorActionsGroup'
import { GerritIcon } from './GerritIcon'
setLinkComponent(RouterLink)
/**
* Metadata associated with adding a given external service.
*/
@ -72,7 +70,7 @@ export interface AddExternalServiceOptions {
/**
* Instructions that will appear on the add / edit page
*/
instructions?: React.ReactNode | string
Instructions?: React.FunctionComponent
/**
* The JSON schema of the external service configuration
@ -145,7 +143,7 @@ const Value: React.FunctionComponent<{ children: React.ReactNode | string | stri
<Code className="hljs-attr">{props.children}</Code>
)
const githubInstructions = (isEnterprise: boolean): React.ReactNode => (
const GitHubInstructions: React.FunctionComponent<{ isEnterprise: boolean }> = ({ isEnterprise }) => (
<div>
<ol>
{isEnterprise && (
@ -199,7 +197,7 @@ const githubInstructions = (isEnterprise: boolean): React.ReactNode => (
</div>
)
const gitlabInstructions = (isSelfManaged: boolean): JSX.Element => (
const GitLabInstructions: React.FunctionComponent<{ isSelfManaged: boolean }> = ({ isSelfManaged }) => (
<div>
<ol>
{isSelfManaged && (
@ -544,7 +542,7 @@ const GITHUB_DOTCOM: AddExternalServiceOptions = {
icon: GithubIcon,
jsonSchema: githubSchemaJSON,
editorActions: githubEditorActions(false),
instructions: githubInstructions(false),
Instructions: () => <GitHubInstructions isEnterprise={false} />,
defaultDisplayName: 'GitHub',
defaultConfig: `{
"url": "https://github.com",
@ -561,7 +559,7 @@ const GITHUB_ENTERPRISE: AddExternalServiceOptions = {
"orgs": []
}`,
editorActions: githubEditorActions(true),
instructions: githubInstructions(true),
Instructions: () => <GitHubInstructions isEnterprise={true} />,
}
const AWS_CODE_COMMIT: AddExternalServiceOptions = {
kind: ExternalServiceKind.AWSCODECOMMIT,
@ -578,7 +576,7 @@ const AWS_CODE_COMMIT: AddExternalServiceOptions = {
"password": "<password>"
}
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -733,7 +731,7 @@ const BITBUCKET_CLOUD: AddExternalServiceOptions = {
},
},
],
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -785,7 +783,7 @@ const BITBUCKET_SERVER: AddExternalServiceOptions = {
"all"
]
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -948,12 +946,12 @@ const GITLAB_DOTCOM: AddExternalServiceOptions = {
]
}`,
editorActions: gitlabEditorActions(false),
instructions: gitlabInstructions(false),
Instructions: () => <GitLabInstructions isSelfManaged={false} />,
}
const GITLAB_SELF_MANAGED: AddExternalServiceOptions = {
...GITLAB_DOTCOM,
title: 'GitLab Self-Managed',
instructions: gitlabInstructions(true),
Instructions: () => <GitLabInstructions isSelfManaged={true} />,
editorActions: gitlabEditorActions(true),
defaultConfig: `{
"url": "https://gitlab.example.com",
@ -977,7 +975,7 @@ const SRC_SERVE_GIT: AddExternalServiceOptions = {
// Do not change this. Sourcegraph uses this as a signal that url is 'src serve'.
"repos": ["src-serve"]
}`,
instructions: (
Instructions: () => (
<div>
<Text>
In the configuration below, set <Field>url</Field> to be the URL of src serve-git.
@ -1013,7 +1011,7 @@ const GITOLITE: AddExternalServiceOptions = {
"host": "git@gitolite.example.com",
"prefix": "gitolite.example.com/"
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1128,7 +1126,7 @@ const GENERIC_GIT: AddExternalServiceOptions = {
"url": "https://git.example.com",
"repos": []
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1181,7 +1179,7 @@ const PERFORCE: AddExternalServiceOptions = {
"p4.passwd": "<ticket value>",
"depots": []
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1236,7 +1234,7 @@ const JVM_PACKAGES: AddExternalServiceOptions = {
"dependencies": []
}
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1267,7 +1265,7 @@ const PAGURE: AddExternalServiceOptions = {
defaultConfig: `{
"url": "https://pagure.example.com",
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1288,7 +1286,7 @@ const GERRIT: AddExternalServiceOptions = {
defaultConfig: `{
"url": "https://gerrit.example.com"
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1314,7 +1312,7 @@ const AZUREDEVOPS: AddExternalServiceOptions = {
"orgs": [],
"projects": []
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1353,7 +1351,7 @@ const NPM_PACKAGES: AddExternalServiceOptions = {
"registry": "https://registry.npmjs.org",
"dependencies": []
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1378,7 +1376,7 @@ const NPM_PACKAGES: AddExternalServiceOptions = {
editorActions: [],
}
const GO_MODULES = {
const GO_MODULES: AddExternalServiceOptions = {
kind: ExternalServiceKind.GOMODULES,
title: 'Go Dependencies',
icon: LanguageGoIcon,
@ -1388,7 +1386,7 @@ const GO_MODULES = {
"urls": ["https://proxy.golang.org"],
"dependencies": []
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1409,7 +1407,7 @@ const GO_MODULES = {
editorActions: [],
}
const PYTHON_PACKAGES = {
const PYTHON_PACKAGES: AddExternalServiceOptions = {
kind: ExternalServiceKind.PYTHONPACKAGES,
title: 'Python Dependencies',
icon: LanguagePythonIcon,
@ -1419,7 +1417,7 @@ const PYTHON_PACKAGES = {
"urls": ["https://pypi.org/simple"],
"dependencies": []
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1441,7 +1439,7 @@ const PYTHON_PACKAGES = {
editorActions: [],
}
const RUST_PACKAGES = {
const RUST_PACKAGES: AddExternalServiceOptions = {
kind: ExternalServiceKind.RUSTPACKAGES,
title: 'Rust Dependencies',
icon: LanguageRustIcon,
@ -1450,7 +1448,7 @@ const RUST_PACKAGES = {
defaultConfig: `{
"dependencies": []
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>
@ -1475,7 +1473,7 @@ const RUBY_PACKAGES: AddExternalServiceOptions = {
"repository": "https://rubygems.org/",
"dependencies": ["shopify_api@12.0.0"]
}`,
instructions: (
Instructions: () => (
<div>
<ol>
<li>

View File

@ -18,13 +18,17 @@ import '../monitoring/initMonitoring'
import { createRoot } from 'react-dom/client'
import { logger } from '@sourcegraph/common'
import { setLinkComponent } from '@sourcegraph/wildcard'
import { TauriLink } from '../app/TauriLink'
import { initAppShell } from '../storm/app-shell-init'
import { EnterpriseWebApp } from './EnterpriseWebApp'
const appShellPromise = initAppShell()
setLinkComponent(TauriLink)
// It's important to have a root component in a separate file to create a react-refresh boundary and avoid page reload.
// https://github.com/pmmmwh/react-refresh-webpack-plugin/blob/main/docs/TROUBLESHOOTING.md#edits-always-lead-to-full-reload
window.addEventListener('DOMContentLoaded', async () => {

View File

@ -13,6 +13,7 @@ import '../monitoring/initMonitoring'
import { createRoot } from 'react-dom/client'
import { logger } from '@sourcegraph/common'
import { RouterLink, setLinkComponent } from '@sourcegraph/wildcard'
import { initAppShell } from '../storm/app-shell-init'
@ -20,6 +21,8 @@ import { EnterpriseWebApp } from './EnterpriseWebApp'
const appShellPromise = initAppShell()
setLinkComponent(RouterLink)
// It's important to have a root component in a separate file to create a react-refresh boundary and avoid page reload.
// https://github.com/pmmmwh/react-refresh-webpack-plugin/blob/main/docs/TROUBLESHOOTING.md#edits-always-lead-to-full-reload
window.addEventListener('DOMContentLoaded', async () => {

View File

@ -319,6 +319,7 @@ export const GlobalNavbar: React.FunctionComponent<React.PropsWithChildren<Globa
'navbar'
)}
size="sm"
target="_blank"
onClick={() =>
eventLogger.log('ClickedOnEnterpriseCTA', { location: 'NavBarSourcegraphApp' })
}

View File

@ -100,7 +100,7 @@ export function CodeHostJSONFormContent(props: CodeHostJSONFormContentProps): Re
<FormGroup
name="Configuration"
title="Configuration"
subtitle={<CodeHostInstructions instructions={externalServiceOptions.instructions} />}
subtitle={<CodeHostInstructions instructions={externalServiceOptions.Instructions} />}
labelClassName={styles.configurationGroupLabel}
>
<DynamicallyImportedMonacoSettingsEditor
@ -132,13 +132,17 @@ export function CodeHostJSONFormContent(props: CodeHostJSONFormContentProps): Re
}
interface CodeHostInstructionsProps {
instructions: ReactNode
instructions?: React.FunctionComponent
}
const CodeHostInstructions: FC<CodeHostInstructionsProps> = props => {
const { instructions } = props
const { instructions: Instructions } = props
const [isInstructionOpen, setInstructionOpen] = useState(false)
if (!Instructions) {
return null
}
return (
<Collapse isOpen={isInstructionOpen} onOpenChange={setInstructionOpen}>
<CollapseHeader
@ -151,7 +155,9 @@ const CodeHostInstructions: FC<CodeHostInstructionsProps> = props => {
See instructions how to fill out JSONC configuration{' '}
<Icon aria-hidden={true} svgPath={isInstructionOpen ? mdiChevronDown : mdiChevronUp} className="mr-1" />
</CollapseHeader>
<CollapsePanel className={styles.configurationGroupInstructions}>{instructions}</CollapsePanel>
<CollapsePanel className={styles.configurationGroupInstructions}>
<Instructions />
</CollapsePanel>
</Collapse>
)
}

View File

@ -52,6 +52,7 @@ const {
SENTRY_DOT_COM_AUTH_TOKEN,
SENTRY_ORGANIZATION,
SENTRY_PROJECT,
SOURCEGRAPH_APP,
} = ENVIRONMENT_CONFIG
const IS_PERSISTENT_CACHE_ENABLED = IS_DEVELOPMENT && !IS_CI
@ -126,9 +127,13 @@ const config = {
}),
},
entry: {
// Enterprise vs. OSS builds use different entrypoints. The enterprise entrypoint imports a
// strict superset of the OSS entrypoint.
app: ENTERPRISE ? path.join(enterpriseDirectory, 'main.tsx') : path.join(__dirname, 'src', 'main.tsx'),
// Desktop app vs. Enterprise vs. OSS builds use different entrypoints. The enterprise entrypoint imports a
// strict superset of the OSS entrypoint. The app endoint imports a strict superset of the enterprise entrypoint.
app: SOURCEGRAPH_APP
? path.join(enterpriseDirectory, 'app-main.tsx')
: ENTERPRISE
? path.join(enterpriseDirectory, 'main.tsx')
: path.join(__dirname, 'src', 'main.tsx'),
// Embedding entrypoint. It uses a small subset of the main webapp intended to be embedded into
// iframes on 3rd party sites. Added only in production enterprise builds or if embed development is enabled.
...(IS_EMBED_ENTRY_POINT_ENABLED && { embed: path.join(enterpriseDirectory, 'embed', 'main.tsx') }),

View File

@ -5,11 +5,12 @@ import (
"net/http"
"net/url"
"path"
"runtime"
"strings"
"github.com/coreos/go-semver/semver"
sglog "github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/cmd/frontend/envvar"
"github.com/sourcegraph/sourcegraph/internal/conf/deploy"
"github.com/sourcegraph/sourcegraph/internal/version"
@ -22,7 +23,20 @@ import (
func serveHelp(w http.ResponseWriter, r *http.Request) {
page := strings.TrimPrefix(r.URL.Path, "/help")
versionStr := version.Version()
logger := sglog.Scoped("serveHelp", "")
logger.Info("redirecting to docs", sglog.String("page", page), sglog.String("versionStr", versionStr))
// For App, help links are handled in the frontend. We should never get here.
sourcegraphAppMode := deploy.IsApp()
if sourcegraphAppMode {
// This should never happen, but if it does, we want to know about it.
logger.Error("help link was clicked in App and handled in the backend, this should never happer")
// Redirect back to the homepage. We don't want App to ever leave the locally-hosted frontend.
http.Redirect(w, r, "/", http.StatusTemporaryRedirect)
return
}
// For release builds, use the version string. Otherwise, don't use any
// version string because:
@ -30,10 +44,8 @@ func serveHelp(w http.ResponseWriter, r *http.Request) {
// - For unreleased dev builds, we serve the contents from the working tree.
// - Sourcegraph.com users probably want the latest docs on the default
// branch.
// - For Sourcegraph App users we also want to show the latest docs,
// but we add the app version as a query param.
var docRevPrefix string
if !version.IsDev(versionStr) && !envvar.SourcegraphDotComMode() && !sourcegraphAppMode {
if !version.IsDev(versionStr) && !envvar.SourcegraphDotComMode() {
v, err := semver.NewVersion(versionStr)
if err != nil {
// If not a semver, just use the version string and hope for the best
@ -50,7 +62,7 @@ func serveHelp(w http.ResponseWriter, r *http.Request) {
dest := &url.URL{
Path: path.Join("/", docRevPrefix, page),
}
if version.IsDev(versionStr) && !envvar.SourcegraphDotComMode() && !sourcegraphAppMode {
if version.IsDev(versionStr) && !envvar.SourcegraphDotComMode() {
dest.Scheme = "http"
dest.Host = "localhost:5080" // local documentation server (defined in Procfile) -- CI:LOCALHOST_OK
} else {
@ -58,25 +70,6 @@ func serveHelp(w http.ResponseWriter, r *http.Request) {
dest.Host = "docs.sourcegraph.com"
}
// For App, add UTM parameters to the docs url.
if sourcegraphAppMode {
q := dest.Query()
q.Set("utm_source", "sg_app")
q.Set("utm_medium", "referral")
// App OS and version
os := runtime.GOOS
if os == "darwin" {
// Use a more common name for mac because it'll be used for analytics.
os = "mac"
}
q.Set("app_os", os)
q.Set("app_version", versionStr)
dest.RawQuery = q.Encode()
}
// Use temporary, not permanent, redirect, because the destination URL changes (depending on the
// current product version).
http.Redirect(w, r, dest.String(), http.StatusTemporaryRedirect)

View File

@ -1417,6 +1417,7 @@ commandsets:
- tauri
env:
DISABLE_CODE_INSIGHTS: false
SOURCEGRAPH_APP: 1
tests:
# These can be run with `sg test [name]`

View File

@ -12,6 +12,7 @@
"tauri": {
"allowlist": {
"shell": {
"open": true,
"sidecar": true,
"scope": [
{