chore(server): convert remaining browsers code to typescript #23556
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,11 +18,9 @@ import utils from './utils' | |
| import type { Browser } from './types' | ||
| import { BrowserCriClient } from './browser-cri-client' | ||
| import type { LaunchedBrowser } from '@packages/launcher/lib/browsers' | ||
| import type { CriClient } from './cri-client' | ||
| import type { Automation } from '../automation' | ||
| import type { BrowserLaunchOpts, BrowserNewTabOpts } from '@packages/types' | ||
|
|
||
| const debug = debugModule('cypress:server:browsers:chrome') | ||
|
|
||
|
|
@@ -123,7 +121,7 @@ const DEFAULT_ARGS = [ | |
| '--disable-dev-shm-usage', | ||
| ] | ||
|
|
||
| let browserCriClient: BrowserCriClient | undefined | ||
|
|
||
| /** | ||
| * Reads all known preference files (CHROME_PREFERENCE_PATHS) from disk and retur | ||
|
|
@@ -320,15 +318,15 @@ const _handleDownloads = async function (client, dir, automation) { | |
| let frameTree | ||
| let gettingFrameTree | ||
|
|
||
| const onReconnect = (client: CriClient) => { | ||
| // if the client disconnects (e.g. due to a computer sleeping), update | ||
| // the frame tree on reconnect in cases there were changes while | ||
| // the client was disconnected | ||
| return _updateFrameTree(client, 'onReconnect')() | ||
| } | ||
|
|
||
| // eslint-disable-next-line @cypress/dev/arrow-body-multiline-braces | ||
| const _updateFrameTree = (client: CriClient, eventName) => async () => { | ||
| debug(`update frame tree for ${eventName}`) | ||
|
|
||
| gettingFrameTree = new Promise<void>(async (resolve) => { | ||
|
|
@@ -433,8 +431,8 @@ const _handlePausedRequests = async (client) => { | |
| }) | ||
| } | ||
|
|
||
| const _setAutomation = async (client: CriClient, automation: Automation, resetBrowserTargets: (shouldKeepTabOpen: boolean) => Promise<void>, options: BrowserLaunchOpts) => { | ||
| const cdpAutomation = await CdpAutomation.create(client.send, client.on, resetBrowserTargets, automation, !!options.experimentalSessionAndOrigin) | ||
|
|
||
| return automation.use(cdpAutomation) | ||
| } | ||
|
|
@@ -490,7 +488,7 @@ export = { | |
| return extensionDest | ||
| }, | ||
|
|
||
| _getArgs (browser: Browser, options: BrowserLaunchOpts, port: string) { | ||
| const args = ([] as string[]).concat(DEFAULT_ARGS) | ||
|
|
||
| if (os.platform() === 'linux') { | ||
|
|
@@ -551,43 +549,60 @@ export = { | |
| return args | ||
| }, | ||
|
|
||
| async connectToNewSpec (browser: Browser, options: BrowserNewTabOpts, automation: Automation) { | ||
| debug('connecting to new chrome tab in existing instance with url and debugging port', { url: options.url }) | ||
|
|
||
| const browserCriClient = this._getBrowserCriClient() | ||
|
|
||
| if (!browserCriClient) throw new Error('Missing browserCriClient in connectToNewSpec') | ||
|
There was a problem hiding this comment. I think assert(browserCriClient, 'Missing browserCriClient in connectToNewSpec')There was a problem hiding this comment.
There was a problem hiding this comment. This should actually close #22451 There was a problem hiding this comment. Yeah, at least it will give a clearer error message. There was a problem hiding this comment. Well...it doesn't fix it but it just give a better warning. I remember talking with @ryanthemanuel about this
|
||
|
|
||
| const pageCriClient = browserCriClient.currentlyAttachedTarget | ||
|
|
||
| if (!pageCriClient) throw new Error('Missing pageCriClient in connectToNewSpec') | ||
|
|
||
| if (!options.url) throw new Error('Missing url in connectToNewSpec') | ||
|
|
||
| await this.attachListeners(browser, options.url, pageCriClient, automation, options) | ||
|
There was a problem hiding this comment. Instead of marking this as async There was a problem hiding this comment. Readability-wise, I prefer |
||
| }, | ||
|
|
||
| async connectToExisting (browser: Browser, options: BrowserLaunchOpts, automation) { | ||
| const port = await protocol.getRemoteDebuggingPort() | ||
|
|
||
| debug('connecting to existing chrome instance with url and debugging port', { url: options.url, port }) | ||
| if (!options.onError) throw new Error('Missing onError in connectToExisting') | ||
|
|
||
| const browserCriClient = await BrowserCriClient.create(port, browser.displayName, options.onError, onReconnect) | ||
|
|
||
| if (!options.url) throw new Error('Missing url in connectToExisting') | ||
|
|
||
| const pageCriClient = await browserCriClient.attachToTargetUrl(options.url) | ||
|
|
||
| await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options) | ||
| }, | ||
|
|
||
| async attachListeners (browser: Browser, url: string, pageCriClient, automation: Automation, options: BrowserLaunchOpts & { onInitializeNewBrowserTab?: () => void }) { | ||
| if (!browserCriClient) throw new Error('Missing browserCriClient in attachListeners') | ||
|
|
||
| await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options) | ||
|
|
||
| await pageCriClient.send('Page.enable') | ||
|
|
||
| await options.onInitializeNewBrowserTab?.() | ||
|
|
||
| await Promise.all([ | ||
| this._maybeRecordVideo(pageCriClient, options, browser.majorVersion), | ||
| this._handleDownloads(pageCriClient, options.downloadsFolder, automation), | ||
| ]) | ||
|
|
||
| await this._navigateUsingCRI(pageCriClient, url) | ||
|
|
||
| if (options.experimentalSessionAndOrigin) { | ||
| await this._handlePausedRequests(pageCriClient) | ||
| _listenForFrameTreeChanges(pageCriClient) | ||
| } | ||
| }, | ||
|
|
||
| async open (browser: Browser, url, options: BrowserLaunchOpts, automation: Automation): Promise<LaunchedBrowser> { | ||
| const { isTextTerminal } = options | ||
|
|
||
| const userDir = utils.getProfileDir(browser, isTextTerminal) | ||
|
|
@@ -646,6 +661,8 @@ export = { | |
| // SECOND connect to the Chrome remote interface | ||
| // and when the connection is ready | ||
| // navigate to the actual url | ||
| if (!options.onError) throw new Error('Missing onError in chrome#open') | ||
|
|
||
| browserCriClient = await BrowserCriClient.create(port, browser.displayName, options.onError, onReconnect) | ||
|
|
||
| la(browserCriClient, 'expected Chrome remote interface reference', browserCriClient) | ||
|
|
@@ -669,7 +686,7 @@ export = { | |
| debug('closing remote interface client') | ||
|
|
||
| // Do nothing on failure here since we're shutting down anyway | ||
| browserCriClient?.close().catch() | ||
| browserCriClient = undefined | ||
|
|
||
| debug('closing chrome') | ||
|
|
@@ -679,21 +696,7 @@ export = { | |
|
|
||
| const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') | ||
|
|
||
|
There was a problem hiding this comment. Nice deduping of this code here. Good clean up |
||
| await this.attachListeners(browser, url, pageCriClient, automation, options) | ||
|
|
||
| // return the launched browser process | ||
| // with additional method to close the remote connection | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the interface is
I wonder why we need the
ts-expect-error? What is the error?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only
OpenProjectLaunchOptsat this point, which doesn't haveurl. This is an area that should be refactored, we're putting a lot of functionality intoproject.launchProjectthat makes it difficult to reason with (and difficult to sanely type)