From 8fb7f193104b012bd72bc6397274604a41b8446a Mon Sep 17 00:00:00 2001 From: Quinn Slack Date: Fri, 19 Jul 2024 04:26:15 -0700 Subject: [PATCH 01/10] upgrade browser extension to manifest v3 Chrome's and the Chrome Web Store's push for [Manifest V3](https://developer.chrome.com/docs/extensions/develop/migrate) means that our browser extension will start to be unusable in a few months unless we upgrade to Manifest V3. - Removes "Enable Sourcegraph on this domain" context menu item that didn't seem to work. We were adding this ourselves (using `webext-domain-permission-toggle`), and it didn't seem to work. Our options popup shows a permission request, which is sufficient. - Otherwise preserves all existing functionality as best I could tell. Test plan: 1. Confirm that the browser extension continues to inject the Sourcegraph icon on https://github.com/hashicorp/errwrap. 1. Load up https://gitlab.com/sqs/web in my local dev Sourcegraph instance and confirm that the browser extension injects the Sourcegraph icon on https://gitlab.com/sqs/web code files. --- client/browser/BUILD.bazel | 1 - .../src/browser-extension/manifest.spec.json | 45 ++++++++------- .../scripts/backgroundPage.main.ts | 4 -- .../types/webextension-polyfill/index.d.ts | 57 ++++++++++++++----- package.json | 1 - pnpm-lock.yaml | 51 ++++++----------- third-party-licenses/ThirdPartyLicenses.csv | 2 - 7 files changed, 83 insertions(+), 78 deletions(-) diff --git a/client/browser/BUILD.bazel b/client/browser/BUILD.bazel index c6cce7565b90..56c9a5d25978 100644 --- a/client/browser/BUILD.bazel +++ b/client/browser/BUILD.bazel @@ -262,7 +262,6 @@ ts_project( "//:node_modules/utility-types", "//:node_modules/uuid", "//:node_modules/vitest", - "//:node_modules/webext-domain-permission-toggle", "//:node_modules/webextension-polyfill", #keep ], ) diff --git a/client/browser/src/browser-extension/manifest.spec.json b/client/browser/src/browser-extension/manifest.spec.json index 9dda82492780..c51a57839990 100644 --- a/client/browser/src/browser-extension/manifest.spec.json +++ b/client/browser/src/browser-extension/manifest.spec.json @@ -1,16 +1,17 @@ { - "$schema": "http://json.schemastore.org/webextension", + "$schema": "https://json.schemastore.org/chrome-manifest", "version": "0.0.0", "name": "Sourcegraph", - "manifest_version": 2, + "manifest_version": 3, "description": "Adds code intelligence to GitHub, GitLab, and other hosts: hovers, definitions, references. For 20+ languages.", - "browser_action": { + "action": { "default_title": "Sourcegraph", "default_icon": { "32": "img/icon-32.png", "48": "img/icon-48.png", "128": "img/icon-128.png" - } + }, + "default_popup": "options.html" }, "icons": { "32": "img/icon-32.png", @@ -18,8 +19,18 @@ "128": "img/icon-128.png" }, "background": { - "scripts": ["js/backgroundPage.main.bundle.js"] + "service_worker": "js/backgroundPage.main.bundle.js" }, + "content_scripts": [ + { + "matches": ["https://github.com/*"], + "js": ["js/contentPage.main.bundle.js"], + "run_at": "document_idle" + } + ], + "permissions": ["activeTab", "storage"], + "host_permissions": ["https://github.com/*", "https://sourcegraph.com/*"], + "optional_host_permissions": ["https://*/*", "http://*/*"], "options_ui": { "page": "options.html", "open_in_tab": true @@ -27,31 +38,21 @@ "storage": { "managed_schema": "schema.json" }, - "optional_permissions": ["tabs", "http://*/*", "https://*/*"], - "content_security_policy": "script-src 'self' blob:; object-src 'self'", - "web_accessible_resources": ["img/*", "css/*"], + "web_accessible_resources": [ + { + "resources": ["img/*", "css/*"], + "matches": [""] + } + ], "omnibox": { "keyword": "src" }, - "applications": { + "browser_specific_settings": { "gecko": { "id": "sourcegraph-for-firefox@sourcegraph.com" } }, "dev": { - "permissions": [ - "storage", - "activeTab", - "contextMenus", - "https://github.com/*", - "https://gitlab.com/*", - "https://localhost:3443/*", - "https://sourcegraph.com/*", - "http://localhost:32773/*" - ], "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCl2X/axNHMbP0K/NCpMzGo/pgBSsHB2xKx6tfohORKtEv2wUMBPmkK3++kirrwYO2f8Ficyq6pjlXV8LjwPSjSw9KZj6bkDn8QNoSdCp6x9i8ZOWPw6UTQ6s54b3rGQNyvrvfD7S6LphxGEx8rNlkjpWKcrvY3+DyoFKHP/hax7wIDAQAB" - }, - "prod": { - "permissions": ["activeTab", "storage", "contextMenus", "https://github.com/*", "https://sourcegraph.com/*"] } } diff --git a/client/browser/src/browser-extension/scripts/backgroundPage.main.ts b/client/browser/src/browser-extension/scripts/backgroundPage.main.ts index ea95d4416aad..52597912e514 100644 --- a/client/browser/src/browser-extension/scripts/backgroundPage.main.ts +++ b/client/browser/src/browser-extension/scripts/backgroundPage.main.ts @@ -18,7 +18,6 @@ import { catchError, distinctUntilChanged, } from 'rxjs/operators' -import addDomainPermissionToggle from 'webext-domain-permission-toggle' import { isDefined, fetchCache } from '@sourcegraph/common' import { type GraphQLResult, requestGraphQLCommon } from '@sourcegraph/http-client' @@ -293,9 +292,6 @@ async function main(): Promise { // loaded in the popup or in th standalone options page. browser.browserAction.setPopup({ popup: 'options.html?popup=true' }) - // Add "Enable Sourcegraph on this domain" context menu item - addDomainPermissionToggle() - const ENDPOINT_KIND_REGEX = /^(proxy|expose)-/ const portKind = (port: browser.runtime.Port): string | undefined => { diff --git a/client/browser/src/types/webextension-polyfill/index.d.ts b/client/browser/src/types/webextension-polyfill/index.d.ts index 7400dc5eed81..b6b10d81c6ce 100644 --- a/client/browser/src/types/webextension-polyfill/index.d.ts +++ b/client/browser/src/types/webextension-polyfill/index.d.ts @@ -162,7 +162,10 @@ declare namespace browser.browserAction { function openPopup(): Promise function setBadgeText(details: { text: string | null; tabId?: number }): void function getBadgeText(details: { tabId?: number }): Promise - function setBadgeBackgroundColor(details: { color: string | ColorArray | null; tabId?: number }): void + function setBadgeBackgroundColor(details: { + color: string | ColorArray | null + tabId?: number + }): void function getBadgeBackgroundColor(details: { tabId?: number }): Promise interface SetBadgeTextColorDetails { @@ -176,12 +179,10 @@ declare namespace browser.browserAction { } function setBadgeTextColor(details: SetBadgeTextColorDetails & { tabId?: number }): void // a union type would allow specifying both, which is not allowed. - // eslint-disable-next-line @typescript-eslint/unified-signatures function setBadgeTextColor(details: SetBadgeTextColorDetails & { windowId?: number }): void function getBadgeTextColor(details: { tabId?: string }): Promise // a union type would allow specifying both, which is not allowed. - // eslint-disable-next-line @typescript-eslint/unified-signatures function getBadgeTextColor(details: { windowId?: string }): Promise function enable(tabId?: number): void @@ -342,7 +343,11 @@ declare namespace browser.contextualIdentities { name: string } - function create(details: { name: string; color: IdentityColor; icon: IdentityIcon }): Promise + function create(details: { + name: string + color: IdentityColor + icon: IdentityIcon + }): Promise function get(cookieStoreId: string): Promise function query(details: { name?: string }): Promise function update( @@ -424,13 +429,19 @@ declare namespace browser.contentScripts { unregister: () => void } - function register(contentScriptOptions: RegisteredContentScriptOptions): Promise + function register( + contentScriptOptions: RegisteredContentScriptOptions + ): Promise } declare namespace browser.devtools.inspectedWindow { const tabId: number - function reload(reloadOptions?: { ignoreCache?: boolean; userAgent?: string; injectedScript?: string }): void + function reload(reloadOptions?: { + ignoreCache?: boolean + userAgent?: string + injectedScript?: string + }): void } declare namespace browser.devtools.network { @@ -690,7 +701,10 @@ declare namespace browser.history { function deleteUrl(details: { url: string }): Promise - function deleteRange(range: { startTime: number | string | Date; endTime: number | string | Date }): Promise + function deleteRange(range: { + startTime: number | string | Date + endTime: number | string | Date + }): Promise function deleteAll(): Promise @@ -787,8 +801,12 @@ declare namespace browser.omnibox { function setDefaultSuggestion(suggestion: { description: string }): void const onInputStarted: EventEmitter - const onInputChanged: CallbackEventEmitter<(text: string, suggest: (arg: SuggestResult[]) => void) => void> - const onInputEntered: CallbackEventEmitter<(text: string, disposition: OnInputEnteredDisposition) => void> + const onInputChanged: CallbackEventEmitter< + (text: string, suggest: (arg: SuggestResult[]) => void) => void + > + const onInputEntered: CallbackEventEmitter< + (text: string, disposition: OnInputEnteredDisposition) => void + > const onInputCancelled: EventEmitter } @@ -803,7 +821,11 @@ declare namespace browser.pageAction { function getTitle(details: { tabId: number }): Promise - function setIcon(details: { tabId: number; path?: string | object; imageData?: ImageDataType }): Promise + function setIcon(details: { + tabId: number + path?: string | object + imageData?: ImageDataType + }): Promise function setPopup(details: { tabId: number; popup: string }): void @@ -1403,7 +1425,10 @@ declare namespace browser.tabs { function captureVisibleTab(windowId?: number, options?: extensionTypes.ImageDetails): Promise function detectLanguage(tabId?: number): Promise function duplicate(tabId: number): Promise - function executeScript(tabId: number | undefined, details: extensionTypes.InjectDetails): Promise + function executeScript( + tabId: number | undefined, + details: extensionTypes.InjectDetails + ): Promise function get(tabId: number): Promise // deprecated: function getAllInWindow(): x; function getCurrent(): Promise @@ -1415,7 +1440,10 @@ declare namespace browser.tabs { // windowId?: number, // tabs: number[]|number, // }): Promise; - function insertCSS(tabId: number | undefined, details: extensionTypes.InjectDetailsCSS): Promise + function insertCSS( + tabId: number | undefined, + details: extensionTypes.InjectDetailsCSS + ): Promise function removeCSS(tabId: number | undefined, details: extensionTypes.InjectDetails): Promise function move( tabIds: number | number[], @@ -1930,7 +1958,10 @@ declare namespace browser.windows { function getCurrent(getInfo?: { populate?: boolean; windowTypes?: WindowType[] }): Promise - function getLastFocused(getInfo?: { populate?: boolean; windowTypes?: WindowType[] }): Promise + function getLastFocused(getInfo?: { + populate?: boolean + windowTypes?: WindowType[] + }): Promise function getAll(getInfo?: { populate?: boolean; windowTypes?: WindowType[] }): Promise diff --git a/package.json b/package.json index 8f48ecb0c0db..0baa21b5eefe 100644 --- a/package.json +++ b/package.json @@ -427,7 +427,6 @@ "utility-types": "^3.10.0", "uuid": "^8.3.0", "vscode-uri": "^3.0.7", - "webext-domain-permission-toggle": "^1.0.1", "webextension-polyfill": "^0.6.0", "whatwg-url": "^14.0.0", "yaml-ast-parser": "^0.0.43", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7c22d3bde2e9..69d66eefcaa2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -487,9 +487,6 @@ importers: vscode-uri: specifier: ^3.0.7 version: 3.0.7 - webext-domain-permission-toggle: - specifier: ^1.0.1 - version: 1.0.1 webextension-polyfill: specifier: ^0.6.0 version: 0.6.0 @@ -6037,7 +6034,7 @@ packages: dependencies: '@jest/fake-timers': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 20.11.19 + '@types/node': 20.8.0 jest-mock: 29.7.0 dev: false @@ -6047,7 +6044,7 @@ packages: dependencies: '@jest/types': 29.6.3 '@sinonjs/fake-timers': 10.3.0 - '@types/node': 20.11.19 + '@types/node': 20.8.0 jest-message-util: 29.7.0 jest-mock: 29.7.0 jest-util: 29.7.0 @@ -6086,7 +6083,7 @@ packages: dependencies: '@types/istanbul-lib-coverage': 2.0.6 '@types/istanbul-reports': 3.0.4 - '@types/node': 20.11.19 + '@types/node': 20.8.0 '@types/yargs': 15.0.19 chalk: 4.1.2 dev: false @@ -6109,7 +6106,7 @@ packages: '@jest/schemas': 29.6.3 '@types/istanbul-lib-coverage': 2.0.6 '@types/istanbul-reports': 3.0.4 - '@types/node': 20.11.19 + '@types/node': 20.8.0 '@types/yargs': 17.0.23 chalk: 4.1.2 @@ -10337,7 +10334,7 @@ packages: '@storybook/preview-api': 7.6.17 '@storybook/theming': 7.6.17(react-dom@18.1.0)(react@18.1.0) '@storybook/types': 7.6.17 - '@types/lodash': 4.17.0 + '@types/lodash': 4.14.167 color-convert: 2.0.1 dequal: 2.0.3 lodash: 4.17.21 @@ -11976,13 +11973,6 @@ packages: '@types/responselike': 1.0.0 dev: false - /@types/chrome@0.0.106: - resolution: {integrity: sha512-sCQa+QJL/Kl+6ngU4xD1VjTFIkQDizOHN94zbb7byK9D3U79x/KXjqXyS7lHoa2q9xw3bWePb+giLhBKOILRuA==} - dependencies: - '@types/filesystem': 0.0.29 - '@types/har-format': 1.2.4 - dev: false - /@types/chrome@0.0.127: resolution: {integrity: sha512-hBB9EApLYKKn2GvklVkTxVP6vZvxsH9okyIRUinNtMzZHIgIKWQk/ESbX+O5g4Bihfy38+aFGn7Kl7Cxou5JUg==} dependencies: @@ -12184,9 +12174,11 @@ packages: resolution: {integrity: sha512-85/1KfRedmfPGsbK8YzeaQUyV1FQAvMPMTuWFQ5EkLd2w7szhNO96bk3Rh/SKmOfd9co2rCLf0Voy4o7ECBOvw==} dependencies: '@types/filewriter': 0.0.28 + dev: true /@types/filewriter@0.0.28: resolution: {integrity: sha512-AR2KUJIMdSfl/SaAHpRotBAlaJpmhgHwehEeSJQOG0hS3IrjDU16xUEEUTdqcvdLa1q16ZK5MMrtOagfLvm0gw==} + dev: true /@types/find-cache-dir@3.2.1: resolution: {integrity: sha512-frsJrz2t/CeGifcu/6uRo4b+SzAwT4NYCVPu1GN8IB9XTzrpPkGuV0tmh9mN+/L0PklAlsC3u5Fxt0ju00LXIw==} @@ -12214,11 +12206,12 @@ packages: /@types/graceful-fs@4.1.5: resolution: {integrity: sha512-anKkLmZZ+xm4p8JWBf4hElkM4XR+EZeA2M9BAkkTldmcyDY4mbdIJnRghDJH3Ov5ooY7/UAoENtmdMSkaAd7Cw==} dependencies: - '@types/node': 20.11.19 + '@types/node': 20.8.0 dev: true /@types/har-format@1.2.4: resolution: {integrity: sha512-iUxzm1meBm3stxUMzRqgOVHjj4Kgpgu5w9fm4X7kPRfSgVRzythsucEN7/jtOo8SQzm+HfcxWWzJS0mJDH/3DQ==} + dev: true /@types/hast@2.3.1: resolution: {integrity: sha512-viwwrB+6xGzw+G1eWpF9geV3fnsDgXqHG+cqgiHrvQfDUW5hzhCyV7Sy3UJxhfRFBsgky2SSW33qi/YrIkjX5Q==} @@ -15002,7 +14995,7 @@ packages: engines: {node: '>=12.13.0'} hasBin: true dependencies: - '@types/node': 20.11.19 + '@types/node': 20.8.0 escape-string-regexp: 4.0.0 is-wsl: 2.2.0 lighthouse-logger: 1.4.2 @@ -15037,7 +15030,7 @@ packages: /chromium-edge-launcher@1.0.0: resolution: {integrity: sha512-pgtgjNKZ7i5U++1g1PWv75umkHvhVTDOQIZ+sjeUX9483S7Y6MUvO0lrd7ShGlQlFHMN4SwKTCq/X8hWrbv2KA==} dependencies: - '@types/node': 20.11.19 + '@types/node': 20.8.0 escape-string-regexp: 4.0.0 is-wsl: 2.2.0 lighthouse-logger: 1.4.2 @@ -20183,7 +20176,7 @@ packages: '@jest/environment': 29.7.0 '@jest/fake-timers': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 20.11.19 + '@types/node': 20.8.0 jest-mock: 29.7.0 jest-util: 29.7.0 dev: false @@ -20204,7 +20197,7 @@ packages: dependencies: '@jest/types': 29.6.3 '@types/graceful-fs': 4.1.5 - '@types/node': 20.11.19 + '@types/node': 20.8.0 anymatch: 3.1.3 fb-watchman: 2.0.2 graceful-fs: 4.2.11 @@ -20262,7 +20255,7 @@ packages: engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} dependencies: '@jest/types': 29.6.3 - '@types/node': 20.11.19 + '@types/node': 20.8.0 jest-util: 29.7.0 dev: false @@ -20276,7 +20269,7 @@ packages: engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} dependencies: '@jest/types': 29.6.3 - '@types/node': 20.11.19 + '@types/node': 20.8.0 chalk: 4.1.2 ci-info: 3.9.0 graceful-fs: 4.2.11 @@ -20307,7 +20300,7 @@ packages: resolution: {integrity: sha512-eIz2msL/EzL9UFTFFx7jBTkeZfku0yUAyZZZmJ93H2TYEiroIx2PQjEXcwYtYl8zXCxb+PAmA2hLIt/6ZEkPHw==} engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} dependencies: - '@types/node': 20.11.19 + '@types/node': 20.8.0 jest-util: 29.7.0 merge-stream: 2.0.0 supports-color: 8.1.1 @@ -28239,18 +28232,6 @@ packages: tslib: 2.1.0 dev: true - /webext-additional-permissions@1.0.0: - resolution: {integrity: sha512-QHMAah++muVoK/d+aYM56BY2DdpMAFMvW/dG28/+SdYJxgUsShsm3fOuwCPIiAIgsC8lsM52hZNJ1ROeii1lKg==} - dependencies: - '@types/chrome': 0.0.106 - dev: false - - /webext-domain-permission-toggle@1.0.1: - resolution: {integrity: sha512-vP3Io5KB+9YSlexDpSykcPKyu/U9pH07YGRIN1snwl92mvHy0aQlOmwYhAdfxLf4KKT7E1te1Bo0+ZvZJYFK2g==} - dependencies: - webext-additional-permissions: 1.0.0 - dev: false - /webextension-polyfill@0.6.0: resolution: {integrity: sha512-PlYwiX8e4bNZrEeBFxbFFsLtm0SMPxJliLTGdNCA0Bq2XkWrAn2ejUd+89vZm+8BnfFB1BclJyCz3iKsm2atNg==} dev: false diff --git a/third-party-licenses/ThirdPartyLicenses.csv b/third-party-licenses/ThirdPartyLicenses.csv index bf2737a3ffc1..01b099cb58c9 100644 --- a/third-party-licenses/ThirdPartyLicenses.csv +++ b/third-party-licenses/ThirdPartyLicenses.csv @@ -2615,8 +2615,6 @@ PNPM,web-encoding,1.1.5,MIT,"",Approved PNPM,web-streams-polyfill,3.2.1,MIT,"",Approved PNPM,webcomponents.js,0.7.20,New BSD,"",Approved PNPM,webcrypto-core,1.7.5,MIT,"",Approved -PNPM,webext-additional-permissions,1.0.0,MIT,"",Approved -PNPM,webext-domain-permission-toggle,1.0.1,MIT,"",Approved PNPM,webextension-polyfill,0.6.0,Mozilla Public License 2.0,"",Approved PNPM,webidl-conversions,7.0.0,Simplified BSD,"",Approved PNPM,webpack,5.88.2,MIT,"",Approved From e60d3f5ba4714b4f8672e735c13ae9a7ad373add Mon Sep 17 00:00:00 2001 From: Quinn Slack Date: Fri, 19 Jul 2024 16:37:30 -0700 Subject: [PATCH 02/10] remove unused createBlobURLForBundle --- .../scripts/backgroundPage.main.ts | 5 ----- .../web-extension-api/runtime.ts | 1 - .../web-extension-api/types.ts | 1 - client/browser/src/shared/platform/worker.ts | 22 ------------------- 4 files changed, 29 deletions(-) delete mode 100644 client/browser/src/shared/platform/worker.ts diff --git a/client/browser/src/browser-extension/scripts/backgroundPage.main.ts b/client/browser/src/browser-extension/scripts/backgroundPage.main.ts index 52597912e514..2ce0453b556d 100644 --- a/client/browser/src/browser-extension/scripts/backgroundPage.main.ts +++ b/client/browser/src/browser-extension/scripts/backgroundPage.main.ts @@ -29,7 +29,6 @@ import { getHeaders } from '../../shared/backend/headers' import { fetchSite } from '../../shared/backend/server' import { initializeOmniboxInterface } from '../../shared/cli' import { browserPortToMessagePort, findMessagePorts } from '../../shared/platform/ports' -import { createBlobURLForBundle } from '../../shared/platform/worker' import { initSentry } from '../../shared/sentry' import { ConditionalTelemetryRecorderProvider } from '../../shared/telemetry' import { EventLogger } from '../../shared/tracking/eventLogger' @@ -227,10 +226,6 @@ async function main(): Promise { await browser.runtime.openOptionsPage() }, - async createBlobURL(bundleUrl: string): Promise { - return createBlobURLForBundle(bundleUrl) - }, - async requestGraphQL({ request, variables, diff --git a/client/browser/src/browser-extension/web-extension-api/runtime.ts b/client/browser/src/browser-extension/web-extension-api/runtime.ts index 41be0d2c0b4a..108c93d07691 100644 --- a/client/browser/src/browser-extension/web-extension-api/runtime.ts +++ b/client/browser/src/browser-extension/web-extension-api/runtime.ts @@ -18,7 +18,6 @@ const messageSender = * Functions that can be invoked from content scripts that will be executed in the background page. */ export const background: BackgroundPageApi = { - createBlobURL: messageSender('createBlobURL'), openOptionsPage: messageSender('openOptionsPage'), requestGraphQL: messageSender('requestGraphQL'), notifyRepoSyncError: messageSender('notifyRepoSyncError'), diff --git a/client/browser/src/browser-extension/web-extension-api/types.ts b/client/browser/src/browser-extension/web-extension-api/types.ts index 1c24809943f2..bbe5c5dd4068 100644 --- a/client/browser/src/browser-extension/web-extension-api/types.ts +++ b/client/browser/src/browser-extension/web-extension-api/types.ts @@ -68,7 +68,6 @@ export interface ManagedStorageItems extends SourcegraphURL { */ export interface BackgroundPageApi { openOptionsPage(): Promise - createBlobURL(bundleUrl: string): Promise requestGraphQL(options: { request: string variables: V diff --git a/client/browser/src/shared/platform/worker.ts b/client/browser/src/shared/platform/worker.ts deleted file mode 100644 index 615ef50f4a05..000000000000 --- a/client/browser/src/shared/platform/worker.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { checkOk } from '@sourcegraph/http-client' - -import { isDefaultSourcegraphUrl } from '../util/context' - -/* - * See [freeze_legacy_extensions.go](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@e8abca577d556b557372518170aaf045093ea760/-/blob/cmd/frontend/internal/registry/scripts/freeze_legacy_extensions.go) - * and [sourcegraph/pull/45923](https://github.com/sourcegraph/sourcegraph/pull/45923) for more context. - */ -const LEGACY_EXTENSIONS_BUCKET_URL = 'https://storage.googleapis.com/sourcegraph-legacy-extensions/' - -export async function createBlobURLForBundle(bundleURL: string): Promise { - const { origin, hostname, href } = new URL(bundleURL) - // Include credentials when fetching extensions from the private registry - const includeCredentials = - !isDefaultSourcegraphUrl(origin) && hostname !== 'localhost' && !href.startsWith(LEGACY_EXTENSIONS_BUCKET_URL) - const response = await fetch(bundleURL, { - credentials: includeCredentials ? 'include' : 'omit', - }) - checkOk(response) - const blob = await response.blob() - return window.URL.createObjectURL(blob) -} From bd47bbd5ad107eab3238f34e63418ab80dbe8cba Mon Sep 17 00:00:00 2001 From: Quinn Slack Date: Fri, 19 Jul 2024 16:57:28 -0700 Subject: [PATCH 03/10] fix telemetry in browser extension comlink - Need to just call recordEvent instead of pass the entire TelemetryRecorder object across the Comlink interface. - Comlink's `Remote<...>` generic type struggles with the generics on the `TelemetryRecorder.recordEvent`, so just use strings. --- client/browser/BUILD.bazel | 1 - .../shared/src/api/client/mainthread-api.ts | 4 +-- client/shared/src/api/contract.ts | 33 ++++++++++++------- client/shared/src/api/extension/activation.ts | 13 +++----- .../shared/src/api/extension/extensionHost.ts | 4 +-- .../src/api/extension/test/activation.test.ts | 8 ++--- 6 files changed, 32 insertions(+), 31 deletions(-) diff --git a/client/browser/BUILD.bazel b/client/browser/BUILD.bazel index 56c9a5d25978..1f8014377874 100644 --- a/client/browser/BUILD.bazel +++ b/client/browser/BUILD.bazel @@ -206,7 +206,6 @@ ts_project( "src/shared/platform/inlineExtensionsService.ts", "src/shared/platform/ports.ts", "src/shared/platform/settings.ts", - "src/shared/platform/worker.ts", "src/shared/polyfills.ts", "src/shared/repo/backend.tsx", "src/shared/repo/index.tsx", diff --git a/client/shared/src/api/client/mainthread-api.ts b/client/shared/src/api/client/mainthread-api.ts index ede9a8765ca3..857db9eddf5a 100644 --- a/client/shared/src/api/client/mainthread-api.ts +++ b/client/shared/src/api/client/mainthread-api.ts @@ -1,5 +1,5 @@ import { type Remote, proxy } from 'comlink' -import { type Unsubscribable, Subscription, from, of, lastValueFrom, ReplaySubject } from 'rxjs' +import { type Unsubscribable, from, lastValueFrom, of, ReplaySubject, Subscription } from 'rxjs' import { share, switchMap } from 'rxjs/operators' import { logger } from '@sourcegraph/common' @@ -141,7 +141,7 @@ export const initMainThreadAPI = ( return proxySubscribable(of([])) }, logEvent: (eventName, eventProperties) => platformContext.telemetryService?.log(eventName, eventProperties), - getTelemetryRecorder: () => platformContext.telemetryRecorder, + recordEvent: platformContext.telemetryRecorder.recordEvent as MainThreadAPI['recordEvent'], logExtensionMessage: (...data) => logger.log(...data), } diff --git a/client/shared/src/api/contract.ts b/client/shared/src/api/contract.ts index 50e6af884d9b..dbc4745c9e46 100644 --- a/client/shared/src/api/contract.ts +++ b/client/shared/src/api/contract.ts @@ -1,22 +1,22 @@ -import type { Remote, ProxyMarked } from 'comlink' +import type { ProxyMarked, Remote } from 'comlink' import type { Unsubscribable } from 'rxjs' import type { Contributions, Evaluated, + HoverMerged, Raw, TextDocumentPositionParameters, - HoverMerged, } from '@sourcegraph/client-api' import type { MaybeLoadingResult } from '@sourcegraph/codeintellify' import type * as clientType from '@sourcegraph/extension-api-types' import type { GraphQLResult } from '@sourcegraph/http-client' +import type { KnownKeys, TelemetryEventParameters } from '@sourcegraph/telemetry' import type { DocumentHighlight, ReferenceContext } from '../codeintel/legacy-extensions/api' import type { Occurrence } from '../codeintel/scip' import type { ConfiguredExtension } from '../extensions/extension' import type { SettingsCascade } from '../settings/settings' -import type { TelemetryV2Props } from '../telemetry' import type { SettingsEdit } from './client/services/settings' import type { ExecutableExtension } from './extension/activation' @@ -75,7 +75,6 @@ export interface FlatExtensionHostAPI extends CodeIntelExtensionHostAPI { /** * Sets the given context keys and values. * If a value is `null`, the context key is removed. - * * @param update Object with context keys as values */ updateContext: (update: { [k: string]: unknown }) => void @@ -89,7 +88,6 @@ export interface FlatExtensionHostAPI extends CodeIntelExtensionHostAPI { /** * Returns an observable that emits all contributions (merged) evaluated in the current model * (with the optional scope). It emits whenever there is any change. - * * @template T Extra allowed property value types for the {@link Context} value. See * {@link Context}'s `T` type parameter for more information. * @param scope The scope in which contributions are fetched. A scope can be a sub-component of @@ -111,7 +109,6 @@ export interface FlatExtensionHostAPI extends CodeIntelExtensionHostAPI { /** * Add a viewer. - * * @param viewer The description of the viewer to add. * @returns The added code viewer (which must be passed as the first argument to other * viewer methods to operate on this viewer). @@ -125,7 +122,6 @@ export interface FlatExtensionHostAPI extends CodeIntelExtensionHostAPI { /** * Sets the selections for a CodeEditor. - * * @param codeEditor The editor for which to set the selections. * @param selections The new selections to apply. * @throws if no editor exists with the given editor ID. @@ -136,7 +132,6 @@ export interface FlatExtensionHostAPI extends CodeIntelExtensionHostAPI { /** * Removes a viewer. * Also removes the corresponding model if no other viewer is referencing it. - * * @param viewer The viewer to remove. */ removeViewer(viewer: ViewerId): void @@ -176,15 +171,29 @@ export interface MainThreadAPI { /** * Log an event (by sending it to the server). - * - * @deprecated use getTelemetryRecorder().recordEvent instead + * @deprecated use {@link MainThreadAPI.recordEvent} instead */ logEvent: (eventName: string, eventProperties?: any) => void /** - * Get a TelemetryRecorder for recording telemetry events to the server. + * Record a telemetry event. + * + * The type signature should be kept in sync with {@link TelemetryRecorder.recordEvent}. */ - getTelemetryRecorder: () => TelemetryV2Props['telemetryRecorder'] + recordEvent: ( + feature: string, + action: string, + parameters?: TelemetryEventParameters< + KnownKeys< + string, + { + [key in string]: number + } + >, + string, + string + > + ) => void /** * Log messages from extensions in the main thread. Makes it easier to debug extensions for applications diff --git a/client/shared/src/api/extension/activation.ts b/client/shared/src/api/extension/activation.ts index bd170670588e..b926e55857c3 100644 --- a/client/shared/src/api/extension/activation.ts +++ b/client/shared/src/api/extension/activation.ts @@ -4,7 +4,7 @@ import { catchError, concatMap, distinctUntilChanged, first, map, switchMap, tap import type sourcegraph from 'sourcegraph' import type { Contributions } from '@sourcegraph/client-api' -import { asError, isErrorLike, hashCode, logger } from '@sourcegraph/common' +import { asError, hashCode, isErrorLike, logger } from '@sourcegraph/common' import { type ConfiguredExtension, @@ -66,18 +66,17 @@ const DEPRECATED_EXTENSION_IDS = new Set(['sourcegraph/code-stats-insights', 'so export function activateExtensions( state: Pick, - mainAPI: Remote>, + mainAPI: Remote>, createExtensionAPI: (extensionID: string) => typeof sourcegraph, - mainThreadAPIInitializations: Observable, /** * Function that activates an extension. * Returns a promise that resolves once the extension is activated. - * */ + */ activate = activateExtension, /** * Function that de-activates an extension. * Returns a promise that resolves once the extension is de-activated. - * */ + */ deactivate = deactivateExtension ): Subscription { const previouslyActivatedExtensions = new Set() @@ -168,8 +167,7 @@ export function activateExtensions( .catch(() => { // noop }) - const telemetryRecorder = await mainAPI.getTelemetryRecorder() - telemetryRecorder.recordEvent('blob.extension', 'activate', { + mainAPI.recordEvent('blob.extension', 'activate', { privateMetadata: { extensionID: telemetryExtensionID }, }) } catch (error) { @@ -365,7 +363,6 @@ export interface ExecutableExtension extends Pick { it('logs events for activated extensions', async () => { const logEvent = sinon.spy() - const mockMain = pretendRemote>({ + const mockMain = pretendRemote>({ logEvent, - getTelemetryRecorder: () => noOpTelemetryRecorder, + recordEvent: noOpTelemetryRecorder.recordEvent as MainThreadAPI['recordEvent'], }) const FIXTURE_EXTENSION: ExecutableExtension = { @@ -53,8 +53,6 @@ describe('Extension activation', () => { function createExtensionAPI() { return {} as typeof sourcegraph }, - of(true), - noopPromise, noopPromise ) From cc0d8af103ad853c14ef4c5da0a404564c70abfd Mon Sep 17 00:00:00 2001 From: Quinn Slack Date: Fri, 19 Jul 2024 21:09:09 -0700 Subject: [PATCH 04/10] remove "after install" page that has not been updated in a long time --- client/browser/BUILD.bazel | 4 - client/browser/config/esbuild.ts | 1 - .../AfterInstallPageContent.module.scss | 12 - .../AfterInstallPageContent.story.tsx | 21 -- .../AfterInstallPageContent.tsx | 239 ------------------ .../pages/after_install.html | 15 -- .../scripts/afterInstallPage.main.tsx | 28 -- .../scripts/backgroundPage.main.ts | 16 +- client/browser/src/end-to-end/BUILD.bazel | 2 - client/browser/src/end-to-end/github.test.ts | 7 +- client/browser/src/end-to-end/shared.ts | 20 -- client/browser/src/integration/github.test.ts | 2 - client/browser/src/integration/gitlab.test.ts | 2 - client/browser/src/integration/shared.ts | 25 -- client/shared/src/testing/driver.ts | 2 +- 15 files changed, 10 insertions(+), 386 deletions(-) delete mode 100644 client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.module.scss delete mode 100644 client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.story.tsx delete mode 100644 client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.tsx delete mode 100644 client/browser/src/browser-extension/pages/after_install.html delete mode 100644 client/browser/src/browser-extension/scripts/afterInstallPage.main.tsx delete mode 100644 client/browser/src/integration/shared.ts diff --git a/client/browser/BUILD.bazel b/client/browser/BUILD.bazel index 1f8014377874..86f4e6d237aa 100644 --- a/client/browser/BUILD.bazel +++ b/client/browser/BUILD.bazel @@ -116,14 +116,12 @@ ts_project( srcs = [ "code-intel-extensions.json", "src/browser-extension/ThemeWrapper.tsx", - "src/browser-extension/after-install-page/AfterInstallPageContent.tsx", "src/browser-extension/browser-action-icon.ts", "src/browser-extension/environmentAssertion.ts", "src/browser-extension/knownCodeHosts.ts", "src/browser-extension/options-menu/OptionsPage.tsx", "src/browser-extension/options-menu/OptionsPageAdvancedSettings.tsx", "src/browser-extension/options-menu/components/OptionsPageContainer.tsx", - "src/browser-extension/scripts/afterInstallPage.main.tsx", "src/browser-extension/scripts/backgroundPage.main.ts", "src/browser-extension/scripts/contentPage.main.ts", "src/browser-extension/scripts/optionsPage.main.tsx", @@ -365,7 +363,6 @@ esbuild( "src/browser-extension/scripts/backgroundPage.main.js", "src/browser-extension/scripts/contentPage.main.js", "src/browser-extension/scripts/optionsPage.main.js", - "src/browser-extension/scripts/afterInstallPage.main.js", "src/native-integration/nativeIntegration.main.js", "src/native-integration/phabricator/phabricatorNativeIntegration.main.js", "src/app.css", @@ -404,7 +401,6 @@ copy_to_directory( ts_project( name = "stories", srcs = [ - "src/browser-extension/after-install-page/AfterInstallPageContent.story.tsx", "src/browser-extension/options-menu/OptionsPage.story.tsx", "src/shared/components/HoverOverlay.story.tsx", ], diff --git a/client/browser/config/esbuild.ts b/client/browser/config/esbuild.ts index 23546fd72397..7e0b4dde9077 100644 --- a/client/browser/config/esbuild.ts +++ b/client/browser/config/esbuild.ts @@ -20,7 +20,6 @@ export function esbuildBuildOptions(mode: 'dev' | 'prod', extraPlugins: esbuild. path.resolve(browserSourcePath, 'browser-extension/scripts/backgroundPage.main.ts'), path.resolve(browserSourcePath, 'browser-extension/scripts/contentPage.main.ts'), path.resolve(browserSourcePath, 'browser-extension/scripts/optionsPage.main.tsx'), - path.resolve(browserSourcePath, 'browser-extension/scripts/afterInstallPage.main.tsx'), // Common native integration entry point (Gitlab, Bitbucket) path.resolve(browserSourcePath, 'native-integration/nativeIntegration.main.ts'), diff --git a/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.module.scss b/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.module.scss deleted file mode 100644 index ee4ae10793a0..000000000000 --- a/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.module.scss +++ /dev/null @@ -1,12 +0,0 @@ -.sourcegraph-logo { - height: 1.5rem; -} - -.code-host-logo { - height: 2rem !important; - width: 2rem !important; -} - -.code-host-titles { - line-height: 2; -} diff --git a/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.story.tsx b/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.story.tsx deleted file mode 100644 index 736532786d1b..000000000000 --- a/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.story.tsx +++ /dev/null @@ -1,21 +0,0 @@ -import type { Meta, StoryFn } from '@storybook/react' - -import { BrandedStory } from '@sourcegraph/wildcard/src/stories' - -import { AfterInstallPageContent } from './AfterInstallPageContent' - -import brandedStyles from '../../branded.scss' - -const config: Meta = { - title: 'browser/AfterInstallPage', - parameters: { - chromatic: { - enableDarkMode: true, - disableSnapshot: false, - }, - }, -} - -export default config - -export const Default: StoryFn = () => {AfterInstallPageContent} diff --git a/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.tsx b/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.tsx deleted file mode 100644 index 61921b60eda4..000000000000 --- a/client/browser/src/browser-extension/after-install-page/AfterInstallPageContent.tsx +++ /dev/null @@ -1,239 +0,0 @@ -import React, { type VideoHTMLAttributes } from 'react' - -import { mdiOpenInNew, mdiGithub, mdiCheck, mdiGitlab, mdiBitbucket, mdiLock, mdiBookOpenPageVariant } from '@mdi/js' -import classNames from 'classnames' - -import { SourcegraphLogo } from '@sourcegraph/branded/src/components/SourcegraphLogo' -import { PhabricatorIcon } from '@sourcegraph/shared/src/components/icons' -import { Link, Icon, Code, H1, H2, H3, Text } from '@sourcegraph/wildcard' - -import { getPlatformName } from '../../shared/util/context' - -import styles from './AfterInstallPageContent.module.scss' - -interface VideoProps extends Pick, 'width' | 'height'> { - name: string - isLightTheme: boolean -} - -const Video: React.FC = ({ name, isLightTheme, width, height }) => { - const suffix = isLightTheme ? 'Light' : 'Dark' - return ( -