-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add missing slowMo to launchPersistentContext #1597
Conversation
040b0ed
to
a60979b
Compare
src/server/chromium.ts
Outdated
async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise<BrowserContext> { | ||
const { | ||
timeout = 30000, | ||
slowMo = undefined |
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.
nit: it should be ok to drop = undefined
initialization and leave just slowMo
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.
Changed to = 0
. I don't like destructing an optional value without a default. Feels like a bug to me at first glance, even though it isn't.
src/server/browserType.ts
Outdated
wsEndpoint: string | ||
} & BrowserOptions; | ||
|
||
type BrowserOptions = { |
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.
I am personally lost in all these types. For example, I don't understand why BrowserOptions
include slowMo
. Can we just inline all the one-offs?
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.
I inlined BrowserOptions, and changed the types we export.
BrowserArgsOptions: For default args method
LaunchOptions: For the launch and launchPersistent methods
LaunchServerOptions: For launchServer and _launchServer methods
ConnectOptions: For connect method
Hopefully things are easier to follow now!
a60979b
to
b5ab088
Compare
slowMo
was missing inlaunchPersistentContext
, and I refactored the types a bit.