I have no idea how to fix this | TypeScript Community | Page 1

digital leaf Mar 2, 2024, 1:41 PM

#

even tho this makes sense to me, but the compilers keeps complaining for the type as it cannot have optional params. What could be the better solution? I have it have used either puppeteer option or playwright options depending on the page passed as arg

import {Page as PuppeteerPage, Frame as PuppeteerFrame} from "puppeteer";
import {Page as PlayWrightPage, Frame as PlaywrightFrame, } from "playwright";



type BrowserPage = PuppeteerPage | PlayWrightPage
type PageFrame = PuppeteerFrame | PlaywrightFrame


export abstract class Navigator<PageType extends BrowserPage>
{
    page: PageType;
    url: string;

    constructor(page: PageType, url: string)
    {
        this.page = page;
        this.url = url;
    }

    async waitRandom(minimum: number, maximum: number): Promise<void> 
    {
        return new Promise(resolve =>
            {
                const delay = Math.random() * (maximum - maximum) + maximum;
                setTimeout(
                    () =>
                    {
                        resolve();
                    }, delay * 1000
                        );
            });
    };

    async goto(url: string, options: Parameters<PageType['goto']>[1] = {timeout: 0,  waitUntil: "networkidle"}): Promise<void>
    {
        await this.page.goto(url, options as any);
    }

}

error

Argument of type 'GoToOptions | { referer?: string | undefined; timeout?: number | undefined; waitUntil?: "load" | "domcontentloaded" | "networkidle" | "commit" | undefined; }' is not assignable to parameter of type '(GoToOptions & { referer?: string | undefined; timeout?: number | undefined; 
....
....
          Type '"networkidle0"' is not assignable to type '"load" | "domcontentloaded" | "networkidle" | "commit" | undefined'. Did you mean '"networkidle"'?ts(2345)

analog notch Mar 2, 2024, 2:01 PM

#

where/how are you using the string "networkidle0"? i don't see that in the code you shared

#

oh nevermind, the as any was hiding the error

#

fundamentally you have two options here:

limit your callers to only using the subset of options that are compatible with both puppeteer and playwright
have different code paths to handle puppeteer and playwright (possibly encapsulate by using a discriminated union or some other means to narrow this.page before calling this.page.goto)

digital leaf Mar 2, 2024, 2:15 PM

#

analog notch fundamentally you have two options here: 1. limit your callers to only using the...

I still don't understand what you mean

analog notch Mar 2, 2024, 2:17 PM

#

i think you understand the core problem, right? your current code allows passing { waitUntil: "networkidle0" } to playwright, but playwright doesn't support that value

#

so you can change the type signature so that callers are not allowed to pass "networkidle0"

#

or you can change it so that the allowed options differ depending on whether this.page is a PuppeteerPage or a PlayWrightPage (but at that point maybe you shouldn't bother trying to abstract over them)

digital leaf Mar 2, 2024, 2:21 PM

#

analog notch or you can change it so that the allowed options differ depending on whether `th...

yes, but I am confused on how to achieve that

#

previously when I posted this question, I get the answer that I somehow need to relate the options with page object

analog notch Mar 2, 2024, 2:21 PM

#

so is #2 your preferred approach? it will mean you are leaking info about which kind of page you have

#

yeah, i can help with that if that's what you want

#

was just trying to establish what direction you wanted to go first 🙂

digital leaf Mar 2, 2024, 2:22 PM

#

I am trying hard, but my lack of skill and concept practice doesn't want me go anywhere

digital leaf Mar 2, 2024, 2:23 PM

#

analog notch yeah, i can help with that if that's what you want

your help would mean a lot to me

analog notch Mar 2, 2024, 2:24 PM

#

sure. before we go further can you help me understand the big picture? why are you trying to make the same Navigator work with either pupeteer or playwright?

#

also if you could give this handbook section a read and see if you understand it that'll help us establish some common terminology. you can ask me questions if it doesn't make sense

#

is PuppeteerPage always guaranteed to be an instance of the class from puppeteer? or are there ways to construct that type that give back a plain object? (i'm wondering if using an instanceof check would be safe)

analog notch Mar 2, 2024, 2:28 PM

#

analog notch is `PuppeteerPage` always guaranteed to be an instance of the class from puppete...

hmm on second thought what i had in mind probably won't work anyway, nevermind

digital leaf Mar 2, 2024, 2:29 PM

#

analog notch sure. before we go further can you help me understand the big picture? why are y...

so basically what I am trying to make a single reusable class which can work with puppeteer and as well as playwright (and in future i may decide to put selenium). Because i make browser automation apps a lot. I have to write sometimes in playwright or in puppeteer. So, a lot of time waste. I just need a single simple api that works with all of these different page objects

#

for example, other functions could be

bringToFront(): void;
    reload(options?: NavigationOptions): void;
    disablePrinting(): void;
    scrollUp(scrollableElement: HTMLElement, pixels?: number): Promise<void>;
    scrollDown(scrollableElement: HTMLElement, pixels?: number): Promise<void>;
    scrollToBottom(scrollableElement: HTMLElement): Promise<void>;
    scrollWindowToBottom(): Promise<void>;
    getNewScrollHeight(scrollableElement: HTMLElement): Promise<any>;
    typeQuery(selector: string, query: string): Promise<void>;
    pressEnter(): Promise<void>;
    click(locator: string): Promise<void>;
    selectOption(selector: string): Promise<void>;
    userAgent(): Promise<string>;
    close(): Promise<void>

analog notch Mar 2, 2024, 2:31 PM

#

digital leaf so basically what I am trying to make a single reusable class which can work wit...

and are you generally okay with limiting yourself to the common subset of functionality, or no? like if puppeteer has some specific feature that is not available in playwright or selenium, you just won't use it?

digital leaf Mar 2, 2024, 2:33 PM

#

analog notch and are you generally okay with limiting yourself to the common subset of functi...

i think this problem can be solved by seperating puppeteer class and playwright class and using factory to decide which one to instantiate?

analog notch Mar 2, 2024, 2:34 PM

#

digital leaf i think this problem can be solved by seperating puppeteer class and playwright ...

but after you instantiate it you'll still only have one scrollWindowToBottom() method or whatever on your class. if playwright can't support that feature for whatever reason, what do you do?

digital leaf Mar 2, 2024, 2:35 PM

#

analog notch but after you instantiate it you'll still only have one `scrollWindowToBottom()`...

it should throw error ig?

#

like I am fine as long as major functionalities are available

analog notch Mar 2, 2024, 2:36 PM

#

i ask because this kind of setup might be easier if that comes up a lot:

const puppeteerPage = new PuppeteerPage()
const commonOperations = new Navigator(puppeteerPage, 'https://example.com')

commonOperations.userAgent() // etc

puppeteerPage.doSpecificPuppeteerThing() // you still have a reference to the specific thing so you can use it when needed

#

(and you could treat goto like that and avoid making things complicated)

digital leaf Mar 2, 2024, 2:38 PM

#

analog notch i ask because this kind of setup might be easier if that comes up a lot: ```ts c...

for this kind of setup I was thinking to expose page object

#

like navigator.page.doSpecific

#

what do you think?

#

is this some kind of a complicated situation never dealt before?

analog notch Mar 2, 2024, 2:40 PM

#

that won't help though. page is typed as PuppeteerPage | PlayWrightPage. you've lost the information about which one it is

digital leaf Mar 2, 2024, 2:41 PM

#

i was thinking to read through crawlee as it seems to do it i think

digital leaf Mar 2, 2024, 2:41 PM

#

analog notch that won't help though. `page` is typed as `PuppeteerPage | PlayWrightPage`. you...

what could be changed to get the info back??

analog notch Mar 2, 2024, 2:44 PM

#

you'd probably have to use narrowing to check (not sure if you had a chance to read those handbook pages yet). but like it kind of defeats your whole idea... if you have a bunch of if checks everywhere before you can do anything, and have to write specific code for playwright vs puppeteer anyway it would be better to just use the original instance

#

i guess it depends how your class was originally instantiated too, you already have it generic

#

anyway, i'll play around a bit to see if i can give you an example of what i was thinking with the #2 item in my list above

digital leaf Mar 2, 2024, 2:46 PM

#

analog notch you'd probably have to use [narrowing](<https://www.typescriptlang.org/docs/hand...

i am going through it rn

#

ok so this fixes the error

type BrowserPage = PuppeteerPage | PlayWrightPage
type PageFrame = PuppeteerFrame | PlaywrightFrame
type Options<T extends unknown> = Record<keyof T, T>

export abstract class Navigator<PageType extends BrowserPage>
{
    page: PageType;
    url: string;

    constructor(page: PageType, url: string)
    {
        this.page = page;
        this.url = url;
    }

    async waitRandom(minimum: number, maximum: number): Promise<void> 
    {
        return new Promise(resolve =>
            {
                const delay = Math.random() * (maximum - maximum) + maximum;
                setTimeout(
                    () =>
                    {
                        resolve();
                    }, delay * 1000
                        );
            });
    };

    async goto(url: string, options?: Options<Parameters<PageType['goto']>[1]>): Promise<void>
    {
        await this.page.goto(url, options);
    }
}

analog notch Mar 2, 2024, 2:59 PM

#

huh, i guess because options has all-optional properties in the goto functions. that's super unsafe though

digital leaf Mar 2, 2024, 3:00 PM

#

analog notch huh, i guess because `options` has all-optional properties in the goto functions...

hmm so what's the alternative?

analog notch Mar 2, 2024, 3:00 PM

#

analog notch huh, i guess because `options` has all-optional properties in the goto functions...

like i can do stuff like this without type errors:

declare const puppeteerNavigator: Navigator<PuppeteerPage>
puppeteerNavigator.goto('https://example.com', { waitUntil: 'some completely made up string' })

but that will obviously cause problems at runtime

analog notch Mar 2, 2024, 3:01 PM

#

digital leaf hmm so what's the alternative?

i came up with this monstrosity using overload signatures:

split zephyrBOT Mar 2, 2024, 3:01 PM

#

mkantor#0

Playground Link

Preview:ts ... this: Navigator<PlayWrightPage>, url: string, options: Parameters<PlayWrightPage["goto"]>[1] ): Promise<void> async goto( url: string, options: Parameters<PuppeteerPage["goto"]>[1] & Parameters<PlayWrightPage["goto"]>[1] = { timeout: 0, waitUntil: "load", } ): Promise<void> { await this.page.goto(url, options) ...

analog notch Mar 2, 2024, 3:02 PM

#

but that's not pretty, and overloads can still be kinda unsafe (though safer than the other option)

#

personally it doesn't seem worth it. i'd just do whateverSpecificTypeOfPageYouHave.goto(...) and not try to abstract over it. i might have a different opinion if this method did more than just directly call this.page.goto, but as-is it's not saving any code (in fact it's strictly more code than the alternative)

digital leaf Mar 2, 2024, 3:04 PM

#

i think it's better if I list out apis for puppeteer differently and playwright differently and use factory to return the instances depends on the page type

#

right?

#

making abstract class is just a waste of time at this point

analog notch Mar 2, 2024, 3:09 PM

#

it's not necessarily a complete waste of time. maybe there's some common functionality that you can abstract over. but yeah it seems like in cases where callers have to care anyway which type of page they have then they might as well just use it directly. especially since they're the ones who decided that in the first place

digital leaf Mar 2, 2024, 3:10 PM

#

analog notch it's not necessarily a complete waste of time. maybe there's _some_ common funct...

i can use mixins where there are common functionalities that would be better?

analog notch Mar 2, 2024, 3:14 PM

#

i'm not sure exactly what you have in mind. "mixins" in JS usually refers to schemes where you dynamically create classes

#

in any case my opinions might not be the ones you want to listen to for that kind of thing. i don't do very much OOP and usually prefer FP using pure functions and immutable objects

digital leaf Mar 2, 2024, 3:17 PM

#

analog notch in any case my opinions might not be the ones you want to listen to for that kin...

sure you can also share your opinion. Would help

#

I have no idea why is there still this problem

import { GoToOptions, Page as PuppeteerPage} from "puppeteer";



class PuppeteerNavigator<PuppeteerPage>
{
    page: PuppeteerPage;
    url: string;

    constructor(page: PuppeteerPage, url: string)
    {
        this.page = page;
        this.url = url;
    }


    async goto(url: string, options?: GoToOptions): Promise<void>
    {
        await this.page.goto(url, options);
    }
}

Property 'goto' does not exist on type 'PuppeteerPage'.ts(2339)

analog notch Mar 2, 2024, 3:21 PM

#

you're shadowing the imported PuppeteerPage here: class PuppeteerNavigator<PuppeteerPage>

#

what you wrote is the same as this:

class PuppeteerNavigator<Blah>
{
    page: Blah;
    url: string;

    constructor(page: Blah, url: string)
    {
        this.page = page;
        this.url = url;
    }


    async goto(url: string, options?: GoToOptions): Promise<void>
    {
        await this.page.goto(url, options);
    }
}

#

if you don't have your editor set up to highlight unused symbols i would suggest trying to change that. i could immediately see that the PuppeteerPage import was unused when i pasted that into the playground

#

anyway i think you want to drop the type parameter in this case:

import { GoToOptions, Page as PuppeteerPage} from "puppeteer";



class PuppeteerNavigator
{
    page: PuppeteerPage;
    url: string;

    constructor(page: PuppeteerPage, url: string)
    {
        this.page = page;
        this.url = url;
    }


    async goto(url: string, options?: GoToOptions): Promise<void>
    {
        await this.page.goto(url, options);
    }
}

digital leaf Mar 2, 2024, 3:25 PM

#

aah I see now, thank you for help

digital leaf Mar 2, 2024, 5:32 PM

#

@analog notch I have done writing the apis, now having problems in returning the correct instances

import { PlayWrightNavigator } from "./abstract/playwright";
import { PuppeteerNavigator } from "./abstract/puppeteer";
import { Page as PuppeteerPage } from "puppeteer";
import { Page as PlayWrightPage} from "playwright";
import { INavigator } from "./abstract/interfaces";

type BrowserPage = PuppeteerPage | PlayWrightPage;

class NavigatorFactory
{
    static createNavigator(page: BrowserPage, url: string): INavigator
    {
        if (page.constructor === PuppeteerPage)
        {
            return new PuppeteerNavigator<PuppeteerPage>(page as PuppeteerPage, url);
        }
        else if ("emulate" in page)
        {
            return new PlayWrightNavigator<PlayWrightPage>(page as PlayWrightPage, url);
        }
        else
        {
            throw new Error("Invalid Browser Page");
        }
    }
}

error

Conversion of type 'import("d:/projects/my_js_repo/PageKit/node_modules/puppeteer/lib/types").Page' to type 'import("d:/projects/my_js_repo/PageKit/node_modules/playwright-core/types/types").Page' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Page' is missing the following properties from type 'Page': addInitScript, exposeBinding, addListener, removeListener, and 48 more.

analog notch Mar 2, 2024, 5:34 PM

#

@digital leaf can you share a complete reproduction using the playground?

#

i need to see all of the relevant types, etc to help debug

#

you can import third-party packages from the playground, but will need to inline types like INavigator

digital leaf Mar 2, 2024, 5:38 PM

#

how can I send you my playground link?

analog notch Mar 2, 2024, 5:38 PM

#

just copy the URL. it should have a big hash that encodes all of the state

digital leaf Mar 2, 2024, 5:39 PM

#

'

📎 message.txt

split zephyrBOT Mar 2, 2024, 5:39 PM

#

.scarletrie#0

Playground Link

Preview:```ts
import {
Page as PuppeteerPage,
GoToOptions,
} from "puppeteer"
import {Page as PlayWrightPage} from "playwright"

export interface INavigator {
page: PuppeteerPage | PlayWrightPage
url: string

waitRandom(
minimum: number,
maximum: number
): Promise<void>
wait(seconds: number): Promise<void>
goto(
url: st
...```

analog notch Mar 2, 2024, 5:39 PM

#

thanks

digital leaf Mar 2, 2024, 5:41 PM

#

this is the minimal I've wrote to see if it really works

analog notch Mar 2, 2024, 5:42 PM

#

ah i see

digital leaf Mar 2, 2024, 5:42 PM

#

and there's also many code duplication

analog notch Mar 2, 2024, 5:42 PM

#

checking .constructor isn't enough to narrow the value

#

you can use instanceof instead which does

#

(though it's not strictly safe, that's why before i was asking if you're sure you'll only ever have class instances)

#

(but it's unsafe in the same way that checking .constructor would be)

digital leaf Mar 2, 2024, 5:43 PM

#

it doesn't work in my case

analog notch Mar 2, 2024, 5:44 PM

#

digital leaf it doesn't work in my case

seems to work for me:

split zephyrBOT Mar 2, 2024, 5:44 PM

#

mkantor#0

Playground Link

Preview:ts ... createNavigator( page: BrowserPage, url: string ): INavigator { if (page instanceof PuppeteerPage) { return new PuppeteerNavigator<PuppeteerPage>( page as PuppeteerPage, url ) } else { return new PlayWrightNavigator<PlayWrightPage>( page as PlayWrightPage, url ) } } ...

analog notch Mar 2, 2024, 5:44 PM

#

you don't need the else if either, since there are only two possible options and you've eliminated one of them

#

you don't need the assertions (as) either. i meant to eliminate them, sorry

digital leaf Mar 2, 2024, 5:45 PM

#

oh i was using instanceof page === PuppeteerPage

digital leaf Mar 2, 2024, 5:45 PM

#

analog notch you don't need the `else if` either, since there are only two possible options a...

but what if there's more to add later?

analog notch Mar 2, 2024, 5:45 PM

#

that's not how the instanceof keyword works

analog notch Mar 2, 2024, 5:46 PM

#

digital leaf but what if there's more to add later?

then you'll get a type error and be forced to add a handler for them

#

with the else you won't get a type error, instead just runtime errors that you'll have to hunt down

#

here's another version with more simplification:

split zephyrBOT Mar 2, 2024, 5:46 PM

#

mkantor#0

Playground Link

Preview:```ts
import {
Page as PuppeteerPage,
GoToOptions,
} from "puppeteer"
import {Page as PlayWrightPage} from "playwright"

export interface INavigator {
page: PuppeteerPage | PlayWrightPage
url: string

waitRandom(
minimum: number,
maximum: number
): Promise<void>
wait(seconds: number): Promise<void>
goto(
url: st
...```

analog notch Mar 2, 2024, 5:47 PM

#

(doesn't seem to be any reason to parameterize PuppeteerNavigator and PlayWrightNavigator)

#

wait sorry

#

lol i have too many versions of this open in tabs

#

1 sec

#

okay this is what i meant to send:

split zephyrBOT Mar 2, 2024, 5:48 PM

#

mkantor#0

Playground Link

Preview:```ts
import {
Page as PuppeteerPage,
GoToOptions,
} from "puppeteer"
import {Page as PlayWrightPage} from "playwright"

export interface INavigator {
page: PuppeteerPage | PlayWrightPage
url: string

waitRandom(
minimum: number,
maximum: number
): Promise<void>
wait(seconds: number): Promise<void>
goto(
url: st
...```

analog notch Mar 2, 2024, 5:48 PM

#

you can try adding a new variant to BrowserPage to see what happens for yourself. you should get a type error that will indicate you need to add another case in createNavigator (which is much better than a runtime error)

digital leaf Mar 2, 2024, 5:50 PM

#

analog notch you can try adding a new variant to `BrowserPage` to see what happens for yourse...

i don't get it

analog notch Mar 2, 2024, 5:52 PM

#

digital leaf i don't get it

here's a simpler example that hopefully explains it:

split zephyrBOT Mar 2, 2024, 5:52 PM

#

mkantor#0

Playground Link

Preview:```ts
type Stuff = string | number // | boolean

declare const stuff: Stuff
if (typeof stuff === "string") {
stuff.toUpperCase()
} else {
// we know for sure stuff is a number here (unless you uncomment | boolean)
stuff.toExponential()
}```

analog notch Mar 2, 2024, 5:53 PM

#

uncomment the | boolean to see what i mean

#

the same kind of thing will happen in your code if you add another variant to the union (at least if you use something structured like the last version i shared)

#

does that make sense?

digital leaf Mar 2, 2024, 5:54 PM

#

yes

#

now it's better to use else instead of adding new handler?

analog notch Mar 2, 2024, 5:55 PM

#

i don't know what you mean by "adding new handler", sorry

digital leaf Mar 2, 2024, 5:55 PM

#

analog notch i don't know what you mean by "adding new handler", sorry

like adding new page object for lets say selenium browser

analog notch Mar 2, 2024, 5:56 PM

#

if your conditions are exhaustive (like you don't have a default else case) then you'll get a type error when you add a new variant, which will tell you that you need to add a new handler

#

in your original version the last else would keep "working" even after adding selenium

#

it probably isn't very important either way for your use case though. you're unlikely to forget to add a case for it there

#

but IMO it's a generally-good habit to try and make conditions like this exhaustive when you can. type errors beat runtime errors every time

digital leaf Mar 2, 2024, 6:00 PM

#

btw can you see any improvements in the code?

#

ik it's really bad rn

#

but i am open to hearing your opinions on how it can be made better

#

removing duplicates, making it more safe etc

analog notch Mar 2, 2024, 6:03 PM

#

i don't understand why you bother to write methods like this:

async bringToFront(): Promise<void>
{
    await this.page.bringToFront();
}

it seems like you're just adding code for no benefit. why can't callers just use x.page.bringtoFront() directly?

#

i think i'd only bother to wrap methods when there is some boilerplate you want to save others from having to repeat

digital leaf Mar 2, 2024, 6:06 PM

#

analog notch i don't understand why you bother to write methods like this: ```ts async bringT...

i just need this function time to time so put it there that's why

digital leaf Mar 2, 2024, 6:06 PM

#

analog notch i think i'd only bother to wrap methods when there is some boilerplate you want ...

ye you're right

#

what others we can remove or add to save us from repetition?

analog notch Mar 2, 2024, 6:12 PM

#

personally i'd probably drop NavigatorFactory too. unless there's more code you want to share in there. just let users directly construct PuppeteerNavigator or PlayWrightNavigator. they'll almost certainly need to know what kind of page they want to use ahead of time anyway

#

that way they'll have a more specific instance to work with afterwards too, not just an INavigator

#

(trying to use INavigator["page"] will have all the same problems we talked about before, because it's not specific. you'll have to narrow it again every time you want to use it)

#

i don't know enough about puppeteer/playwright to give suggestions that are specific to them, sadly

#

i guess another thing is if you can make your logging follow more of a pattern you could probably abstract/centralize that somewhere

#I have no idea how to fix this