-
Notifications
You must be signed in to change notification settings - Fork 165
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: allow to reuse a browser by passing a browserContext
#884
Conversation
browserContext
browserContext
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no suggestions.
…arity and maintainability
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
src/scrapers/base-scraper-with-browser.ts:85
- [nitpick] The word 'bang' might be confusing. It would be clearer to use 'exclamation mark (!)' instead.
NOTICE - it is discouraged to use bang (!) in general.
src/scrapers/base-isracard-amex.ts:33
- The ExtendedScraperOptions interface was removed, but there was no mention of its removal in the context provided. Ensure that this interface is not used elsewhere in the codebase.
type CompanyServiceOptions = {
src/scrapers/base-isracard-amex.ts:258
- [nitpick] The parameter name 'options' is of type 'CompanyServiceOptions'. It would be clearer to rename it to 'companyServiceOptions'.
async function getExtraScrapTransaction(page: Page, options:CompanyServiceOptions, month: Moment, accountIndex: number, transaction: Transaction): Promise<Transaction> {
…ptions * **ExternalBrowserOptions** - Add example using an externally created browser instance * **ExternalBrowserContextOptions** - Add example using an externally managed browser context
@baruchiro Thanks for approving! Do you have an estimation for the merging of the PR? |
🎉 This PR is included in version 5.4.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Motivation:
Changes:
browserContext
instead of abrowser
.ScraperOptions
interface now has a union of threeBrowserOptions
interfaces:DefaultBrowserOptions
for when the scraper creates the browser.ExternalBrowserOptions
for when the caller supplies an external single-use browser.skipCloseBrowser
flag was added to allow the caller to manage the browser lifecycle.ExternalBrowserContextOptions
for when the caller supplies aBrowserContext
and manages the browser lifecycle.