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

IJupyterLabPage does not include overridden page.reload() options and the defaults are misspecified #16336

Open
krassowski opened this issue May 16, 2024 · 0 comments
Labels

Comments

@krassowski
Copy link
Member

Description

JupyterLabPage which implements IJupyterLabPage overrides the reload(), and it adds waitForIsReady option:

/**
* Returns the main resource response. In case of multiple redirects, the navigation will resolve with the response of the
* last redirect.
*
* This overrides the standard Playwright `page.reload` method by waiting for:
* - the application to be started (plugins are loaded)
* - the galata in page code to be injected
* - the splash screen to have disappeared
* - the launcher to be visible
*
* @param options
*/
async reload(options?: {
/**
* Maximum operation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be
* changed by using the
* [browserContext.setDefaultNavigationTimeout(timeout)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-navigation-timeout),
* [browserContext.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-timeout),
* [page.setDefaultNavigationTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-navigation-timeout)
* or [page.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-timeout) methods.
*/
timeout?: number;
/**
* When to consider operation succeeded, defaults to `load`. Events can be either:
* - `'domcontentloaded'` - consider operation to be finished when the `DOMContentLoaded` event is fired.
* - `'load'` - consider operation to be finished when the `load` event is fired.
* - `'networkidle'` - consider operation to be finished when there are no network connections for at least `500` ms.
* - `'commit'` - consider operation to be finished when network response is received and the document started loading.
*/
waitUntil?: 'load' | 'domcontentloaded' | 'networkidle' | 'commit';
/**
* Whether to wait for fixture `waitIsReady` or not when reloading.
*
* Default is true.
*/
waitForIsReady?: boolean;
}): Promise<Response | null> {
const response = await this.page.reload({
timeout: options?.timeout,
waitUntil: options?.waitUntil ?? 'domcontentloaded'
});
await this.waitForAppStarted();
await this.hookHelpersUp();
if (options?.waitForIsReady ?? true) {
await this.waitIsReady(this.page, this);
}
return response;
}

This however is not defined on IJupyterLabPage which means that using

await page.reload({ waitForIsReady: false });

leads to:

Argument of type '{ waitForIsReady: boolean; }' is not assignable to parameter of type '{ timeout?: number | undefined; waitUntil?: "load" | "domcontentloaded" | "networkidle" | "commit" | undefined; }'.
  Object literal may only specify known properties, and 'waitForIsReady' does not exist in type '{ timeout?: number | undefined; waitUntil?: "load" | "domcontentloaded" | "networkidle" | "commit" | undefined; }'.

and requires using:

await (page as any as JupyterLabPage).reload({ waitForIsReady: false });

Further, the documentation says that waitUntil "defaults to load." but the code shows that it defaults to domcontentloaded.

  • JupyterLab version: 4.2.0
@krassowski krassowski added bug status:Needs Triage Applied to new issues that need triage labels May 16, 2024
@JasonWeill JasonWeill removed the status:Needs Triage Applied to new issues that need triage label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants