Skip to content
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

[Bug]: error TS2339: Property 'waitForEvent' does not exist on type 'SuiteFunction' #7857

Closed
VincentDondain opened this issue Jul 27, 2021 · 12 comments

Comments

@VincentDondain
Copy link

VincentDondain commented Jul 27, 2021

Playwright version

1.13.0

Operating system

MacOS

What browsers are you seeing the problem on?

Chromium

Other information

"devDependencies": {
    "@types/jest": "^26.0.24",
    "azure-kusto-ingest": "^2.2.1",
    "jest": "^27.0.6",
    "jest-circus": "^27.0.6",
    "jest-junit": "^12.2.0",
    "jest-playwright-preset": "^1.7.0",
    "otplib": "^12.0.1",
    "playwright": "^1.13.0",
    "playwright-core": "^1.13.0",
    "ts-jest": "^27.0.4",
    "typescript": "^4.3.5",
    "yn": "^4.0.0"
}

What happened? / Describe the bug

When running the tests locally I'm getting these error TS2339: Property 'waitForEvent' does not exist on type 'SuiteFunction' whenever there is this context.waitForEvent('page'), in the Promise.All.

This used to work and is the documented way of handling new pages: https://playwright.dev/docs/multi-pages/#handling-new-pages.

Not sure what's wrong (:

Code snippet to reproduce your bug

const [newPage] = await Promise.all([
      context.waitForEvent('page'),
      page.click('css=[aria-label*="XXX"] >> css=[title="Open in Browser"]'),
    ])

Relevant log output

pw:api <= browserContext.newPage succeeded +840ms
ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information.
  pw:api => browser.close started +4s
  pw:api <= browser.close succeeded +5ms
 FAIL   browser: chromium  e2e/tests/myTest.spec.ts
  ● Test suite failed to run

    e2e/tests/myTest.spec.ts:124:15 - error TS2339: Property 'waitForEvent' does not exist on type 'SuiteFunction'.

    124       context.waitForEvent('page'),
                      ~~~~~~~~~~~~
@dgozman
Copy link
Contributor

dgozman commented Jul 27, 2021

There is something strange with your context variable. Please share the whole snippet, or at least where you initialize the context variable.

@VincentDondain
Copy link
Author

Hey @dgozman I'm doing the following with the context:

describe('My Tests', () => {
  let page: Page;
  let pageTabOne: Page;

  beforeAll(async () => {
    const context = await browser.newContext({
      ignoreHTTPSErrors: true,
      bypassCSP: true,
      recordVideo: recordVideo,
    });
    await getContext.contextUtil(context);
    page = await context.newPage();
    pageTabOne = await context.newPage();
  }
);

and

async function contextUtil(context:any) {
    const cookies = process.env.COOKIES;
    const localStorage = process.env.LOCALSTORAGE;
    
    await context.addCookies(JSON.parse(cookies));
    await context.addInitScript(([storageDump]) => {
      if (window.location.hostname === 'mysite.com') {
        console.log(window.location.hostname);
        console.log(storageDump.length);
        const entries = JSON.parse(storageDump);
        Object.keys(entries).forEach(k => {
          window.localStorage.setItem(k, entries[k]);
        });
      }
    }, [localStorage]);
}

module.exports = {contextUtil};

This was all working fine until recently.

@VincentDondain
Copy link
Author

I'm trying the '@playwright/test' runner for the first time now and I'm not running into those issues with it. So it seems related to jest-playwright?

@mxschmitt
Copy link
Member

If you are using jest-playwright, then you can pass the context options in the config, see here: https://github.com/playwright-community/jest-playwright#options

Also a browser, context, and page instance is available in every file, since its on the global scope. Its also persistent through the tests per file, so a single page instance gets shared between the tests.

In comparison to @playwright/test where a new context and by that a new page gets created for each test. For Playwright Test, you can pass the options also in the config, see here: https://playwright.dev/docs/test-configuration#more-browser-and-context-options.

I saw you are migrating, so please let us know if you are still facing the issues, my guess would be that your manual created context conflicts with the global context, so I would either recommend renaming or using the context from Playwright Test. Another nit: instead of using any, Playwright Test exposes the types, so use there the BrowserContext type.

(Feel free to mention me in a migration PR, then I can also review it and give recommendations if you are interested)

@VincentDondain
Copy link
Author

@mxschmitt thanks for those insights, now I have a couple things to try. Will let you know how it goes.

However I'm a bit confused with @playwright/test creating a new context / page for each test. I do not see a config option to change that behavior. I asked that here: #7882.
In our case, sharing the context between all the tests is very useful as we'd have to navigate to the page we're testing in every test otherwise. Those pages are generated and it can take some time to open them so it's more efficient to keep the page open and test a bunch of things in it. Any idea how to solve that with @playwright/test?

@mxschmitt
Copy link
Member

mxschmitt commented Jul 29, 2021

You can do that with Playwright Test too, this would mean:

  • in beforeAll create a new Page
  • in afterAll close the page. (If you use browser.newPage() it will internally create a new context for you.)
  • Also you should then not use the page from the fixtures -> the callback parameter which gets destructured

You can't disable it currently. Keep in mind, that when doing that, you will loose all the Playwright Test goodies, which means, no video, no screenshot on failure, no tracing out of the box etc. (We are working on providing support for that too.)

If you want to do it at more a larger scale, I would recommend to use a worker fixture for your page instance instead, see here.

@VincentDondain
Copy link
Author

VincentDondain commented Jul 29, 2021

@mxschmitt right right, that's actually what I did already and it works, test #1 and #2 can run after each other and share the same page. However the problem is that if any test fails the page closes (even though it's a test.afterAll which shouldn't run after each test...).

However this is sidetracking us, it's really #7882 (:

@VincentDondain
Copy link
Author

@mxschmitt & @dgozman it's really the new page pattern that doesn't work for me (back to jest-playwright)... everything else does.

const [newPage] = await Promise.all([
      context.waitForEvent('page'),
      page.click('css=[aria-label*="XXX"] >> css=[title="Open in Browser"]'),
])

It's not a problem with @playwright/test. I tried following your advice so now I set the context config in jest-playwright.config.js and I skip my beforeAll entirely so no more:

const context = await browser.newContext(browserContextOptions);
await getContext.contextUtil(context);
page = await context.newPage();

Yet whenever playwright gets to my file with the const [newPage] = await Promise.all I get these error TS2339: Property 'waitForEvent' does not exist on type 'SuiteFunction'.

Is there another pattern to get the page of a new tab opened from the main Page of the tests?

@VincentDondain
Copy link
Author

@mxschmitt Ok so with @playwright/test I'm getting the error at runtime (ReferenceError: context is not defined) instead of build time. So it's actually slightly worse from a user perspective than jest-playwright but at least both are consistant in not handling that [newPage] scenario. I am using exactly the same code as https://playwright.dev/docs/multi-pages#handling-new-pages.

1) [chromium] › myTest.spec.ts:109:3 › Network UI Tests

    ReferenceError: context is not defined

      113 |     await page.click('[aria-label*="XXX"]');
      114 |     const [newPage] = await Promise.all([
    > 115 |       context.waitForEvent('page'),
          |       ^
      116 |       page.click('css=[aria-label*="XXX"] >> css=[title="Open in Browser"]'),
      117 |     ])
      118 |     await newPage.waitForLoadState();

        at /Users/vince/Documents/myTest.spec.ts:115:7
        at WorkerRunner._runTestWithBeforeHooks (/Users/vince/Documents/myTest/node_modules/@playwright/test/lib/test/workerRunner.js:390:7)

@VincentDondain
Copy link
Author

image

Ok it's likely because context was scoped to our beforeAll and we did not retrieve it back in the test. Making context a local variable seems to do the trick (:

Though this code hasn't changed and worked before so not sure what happened 🤔

@mxschmitt
Copy link
Member

mxschmitt commented Jul 30, 2021

@VincentDondain so the issues are resolved and we can close it if I understood it correctly?

(In the future we might add page per file to support your use-case better)

@VincentDondain
Copy link
Author

@mxschmitt correct, I verified it and it works. Only #7857 (comment) remains but that's what #7882 is all about (;

Thank for the help and valuable info you provided ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants