Skip to content

Conversation

kettanaito
Copy link
Member

@kettanaito kettanaito commented Aug 25, 2025

While going through the E2E tests, I've noticed the setupWebAuthn() test utility having page: any argument type. Once I updated it to be the correct Page type from Playwright, I noticed that the utility seems to be using the client.send() method incorrectly:

// No "options" key known on the second argument!
-await client.send('WebAuthn.enable', { options: { enableUI: true } })

// The second argument itself is the "options" object.
+await client.send('WebAuthn.enable', { enableUI: true })

Playwright provides correct type autocompletion with this usage, which I believe to be the goal here. I'm not sure how the tests passed given this was allegedly incorrect, but I hope this is a good find.

While Playwright is ambiguous about the type of the params argument in the docs, they do seem to keep a mapped type that ensures their correct type in the implementation:

// node_modules/playwright-core/types/types.d.ts
  send<T extends keyof Protocol.CommandParameters>(
    method: T,
    params?: Protocol.CommandParameters[T]
  ): Promise<Protocol.CommandReturnValues[T]>;
"WebAuthn.enable": WebAuthn.enableParameters;
    export type enableParameters = {
      /**
       * Whether to enable the WebAuthn user interface. Enabling the UI is
recommended for debugging and demo purposes, as it is closer to the real
experience. Disabling the UI is recommended for automated testing.
Supported at the embedder's discretion if UI is available.
Defaults to false.
       */
      enableUI?: boolean;
    }

As you can see, there's no options nesting anywhere in the types.

Test Plan

Checklist

  • Tests updated
  • Docs updated

Screenshots

@kettanaito kettanaito requested a review from kentcdodds August 25, 2025 09:13
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I'm not sure how we ended up with any on that! Shame on me! 😅

@kentcdodds kentcdodds merged commit 9938f6e into main Aug 25, 2025
6 checks passed
@kentcdodds kentcdodds deleted the test/passkey-page-type branch August 25, 2025 17:50
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

Successfully merging this pull request may close these issues.

2 participants