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.

Sign up for GitHub

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

Jump to bottom

feat(ct): react optional declaration merging hooks config #31055

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat(ct): react optional declaration merging hooks config #31055

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd57031
feat(ct): react declaration merging hooks config
sand4rt May 28, 2024
a43d5d6
feat(ct): react explicit hooks config generic type
sand4rt Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/playwright-ct-react/hooks.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
* limitations under the License.
*/

export declare function beforeMount<HooksConfig>(
export interface RegisterHooksConfig {}

export declare function beforeMount<HooksConfig = RegisterHooksConfig>(
callback: (params: { hooksConfig?: HooksConfig; App: () => JSX.Element }) => Promise<void | JSX.Element>
): void;
export declare function afterMount<HooksConfig>(
export declare function afterMount<HooksConfig = RegisterHooksConfig>(
callback: (params: { hooksConfig?: HooksConfig }) => Promise<void>
): void;
7 changes: 4 additions & 3 deletions packages/playwright-ct-react/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@

import type { Locator } from 'playwright/test';
import type { TestType } from '@playwright/experimental-ct-core';
import type { RegisterHooksConfig } from './hooks';

export interface MountOptions<HooksConfig> {
export interface MountOptions<HooksConfig extends RegisterHooksConfig> {
hooksConfig?: HooksConfig;
}

Expand All @@ -27,11 +28,11 @@ export interface MountResult extends Locator {
}

export const test: TestType<{
mount<HooksConfig>(
mount<HooksConfig extends RegisterHooksConfig = RegisterHooksConfig>(
component: JSX.Element,
options?: MountOptions<HooksConfig>
): Promise<MountResult>;
}>;

export { defineConfig, PlaywrightTestConfig } from '@playwright/experimental-ct-core';
export { defineConfig, type PlaywrightTestConfig } from '@playwright/experimental-ct-core';
export { expect, devices } from 'playwright/test';
6 changes: 4 additions & 2 deletions packages/playwright-ct-react17/hooks.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
* limitations under the License.
*/

export declare function beforeMount<HooksConfig>(
export interface RegisterHooksConfig {}

export declare function beforeMount<HooksConfig = RegisterHooksConfig>(
callback: (params: { hooksConfig?: HooksConfig; App: () => JSX.Element }) => Promise<void | JSX.Element>
): void;
export declare function afterMount<HooksConfig>(
export declare function afterMount<HooksConfig = RegisterHooksConfig>(
callback: (params: { hooksConfig?: HooksConfig }) => Promise<void>
): void;
7 changes: 4 additions & 3 deletions packages/playwright-ct-react17/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@

import type { Locator } from 'playwright/test';
import type { TestType } from '@playwright/experimental-ct-core';
import type { RegisterHooksConfig } from './hooks';

export interface MountOptions<HooksConfig> {
export interface MountOptions<HooksConfig extends RegisterHooksConfig> {
hooksConfig?: HooksConfig;
}

Expand All @@ -27,11 +28,11 @@ export interface MountResult extends Locator {
}

export const test: TestType<{
mount<HooksConfig>(
mount<HooksConfig extends RegisterHooksConfig = RegisterHooksConfig>(
component: JSX.Element,
options?: MountOptions<HooksConfig>
): Promise<MountResult>;
}>;

export { defineConfig, PlaywrightTestConfig } from '@playwright/experimental-ct-core';
export { defineConfig, type PlaywrightTestConfig } from '@playwright/experimental-ct-core';
export { expect, devices } from 'playwright/test';
11 changes: 6 additions & 5 deletions tests/components/ct-react-vite/playwright/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ import { beforeMount, afterMount } from '@playwright/experimental-ct-react/hooks
import { BrowserRouter } from 'react-router-dom';
import '../src/assets/index.css';

export type HooksConfig = {
route?: string;
routing?: boolean;
declare module '@playwright/experimental-ct-react/hooks' {
interface RegisterHooksConfig {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to not override types globally in Playwright. Can we help the user in some other way? Perhaps by casting test exported from the package like const test = baseTest as SomeTestDeclaration<SpecificHooksConfig>? Or by extending with a mnt fixture that is mount<SpecificHooksConfig>?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to not override types globally in Playwright.

Normally i wouldn't use declare module in this way, but i think this is a good use case. Especially because the mount() is called so many times. The beforeMount<T>, afterMount<T>() and mount<T>() can also still be specified manually for the people who think it's too much magic.

Can we help the user in some other way?

I don't think there's another way to make the hooksConfig in beforeMount(), afterMount() and mount() automatically type-safe with a single type definition right? type-casting const test would solve a part of the problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's another way to make the hooksConfig in beforeMount(), afterMount() and mount() automatically type-safe with a single type definition right? type-casting const test would solve a part of the problem.

That's right. However, you only write beforeMount() and afterMount() once, so explicitly typing them is not a big deal. It looks like the main goal is to avoid mount<T> every time.

routing?: boolean;
}
}

beforeMount<HooksConfig>(async ({ hooksConfig, App }) => {
beforeMount(async ({ hooksConfig, App }) => {
console.log(`Before mount: ${JSON.stringify(hooksConfig)}`);

if (hooksConfig?.routing)
return <BrowserRouter><App /></BrowserRouter>;
});

afterMount<HooksConfig>(async () => {
afterMount(async () => {
console.log(`After mount`);
});
5 changes: 2 additions & 3 deletions tests/components/ct-react-vite/tests/react-router.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { test, expect } from '@playwright/experimental-ct-react';
import App from '@/App';
import type { HooksConfig } from '../playwright';

test('navigate to a page by clicking a link', async ({ page, mount }) => {
const component = await mount<HooksConfig>(<App />, {
const component = await mount(<App />, {
hooksConfig: { routing: true },
});
await expect(component.getByRole('main')).toHaveText('Login');
Expand All @@ -14,7 +13,7 @@ test('navigate to a page by clicking a link', async ({ page, mount }) => {
});

test('update should not reset mount hooks', async ({ page, mount }) => {
const component = await mount<HooksConfig>(<App title='before'/>, {
const component = await mount(<App title='before'/>, {
hooksConfig: { routing: true },
});
await expect(component.getByRole('heading')).toHaveText('before');
Expand Down
1 change: 0 additions & 1 deletion tests/components/ct-react-vite/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,5 @@
"*": ["_"],
}
},
"include": ["src", "tests"],
"references": [{ "path": "./tsconfig.node.json" }]
}
Loading