fix(types): Fix `overwriteCommand` inconsistent typing by dprevost-LMI · Pull Request #14549 · webdriverio/webdriverio

dprevost-LMI · 2025-06-08T15:31:05Z

Proposed changes

As exposed in this issue, when using overwriteCommand, the typing is not always correctly inferred. We attempt to fix a couple of them and more:

Things done:

Have the this refer to WebdriverIO.Element automatically in the func of overwriteCommand & addCommand
Added the this inference to Browser automatically in the func of overwriteCommand & addCommand (overkill?)
Have the originalCommandFunction infer the correct return type by adding the return type explicitly in existing browser commands
Review documentation to be more verbose and also be more type-safe, and add this in existing examples.
Propose overwriteBrowserCommand and overwriteElementCommand, instead of using the boolean, to ease the API and not worry about missing the boolean or not. Will also reduce the typing of definitions
Propose addBrowserCommand and addElementCommand, instead of using the boolean, to ease the API and not worry about missing the boolean or not. Will also reduce the typing of definitions

Types of changes

Polish (an improvement for an existing feature)
Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update (improvements to the project's docs)
Specification changes (updates to WebDriver command specifications)
Internal updates (everything related to internal scripts, governance documentation and CI files)

Checklist

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have added the necessary documentation (if appropriate)
I have added proper type definitions for new commands (if appropriate)

Backport Request

//: # (The current main branch is the development branch for WebdriverIO v9. If your change should be released to the current major version of WebdriverIO (v8), please raise another PR with the same changes against the v8 branch.)

This change is solely for v9 and doesn't need to be back-ported
Back-ported PR at #XXXXX

Further comments

Please let me know about the new API proposal and whether you would like to deprecate the former or keep it.

Reviewers: @webdriverio/project-committers

- Fix inference having the `this` which was in excess - Fix inference wrapping in a `(...arg) =>` which was in excess - Fix browser command overwrite where the return promise type is wrong because undefined verbose return type is inferred to `Promise<unknown>`

pkg-pr-new · 2025-06-08T17:03:53Z

Open in StackBlitz

eslint-plugin-wdio

npm i https://pkg.pr.new/webdriverio/webdriverio/eslint-plugin-wdio@14549

@wdio/allure-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/allure-reporter@14549

@wdio/appium-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/appium-service@14549

@wdio/browser-runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/browser-runner@14549

@wdio/browserstack-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/browserstack-service@14549

@wdio/cli

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/cli@14549

@wdio/concise-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/concise-reporter@14549

@wdio/config

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/config@14549

@wdio/cucumber-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/cucumber-framework@14549

@wdio/dot-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/dot-reporter@14549

@wdio/firefox-profile-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/firefox-profile-service@14549

@wdio/globals

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/globals@14549

@wdio/jasmine-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/jasmine-framework@14549

@wdio/json-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/json-reporter@14549

@wdio/junit-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/junit-reporter@14549

@wdio/lighthouse-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/lighthouse-service@14549

@wdio/local-runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/local-runner@14549

@wdio/logger

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/logger@14549

@wdio/mocha-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/mocha-framework@14549

@wdio/protocols

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/protocols@14549

@wdio/repl

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/repl@14549

@wdio/reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/reporter@14549

@wdio/runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/runner@14549

@wdio/sauce-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/sauce-service@14549

@wdio/shared-store-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/shared-store-service@14549

@wdio/smoke-test-cjs-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-cjs-service@14549

@wdio/smoke-test-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-reporter@14549

@wdio/smoke-test-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-service@14549

@wdio/spec-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/spec-reporter@14549

@wdio/static-server-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/static-server-service@14549

@wdio/sumologic-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/sumologic-reporter@14549

@wdio/testingbot-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/testingbot-service@14549

@wdio/types

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/types@14549

@wdio/utils

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/utils@14549

@wdio/webdriver-mock-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/webdriver-mock-service@14549

webdriver

npm i https://pkg.pr.new/webdriverio/webdriverio/webdriver@14549

webdriverio

npm i https://pkg.pr.new/webdriverio/webdriverio@14549

commit: 655a3d6

packages/webdriverio/src/types.ts

christian-bromann · 2025-06-09T17:26:51Z

Propose overwriteBrowserCommand and overwriteElementCommand, instead of using the boolean, to ease the API and not worry about missing the boolean or not. Will also reduce the typing of definitions

Propose addBrowserCommand and addElementCommand, instead of using the boolean, to ease the API and not worry about missing the boolean or not. Will also reduce the typing of definitions

I think this makes sense to me. We should deprecate the overwriteCommand then so we can eventually remove it.

christian-bromann

Great cleanup 👏 let's address above comment and get this merged.

dprevost-LMI · 2025-06-09T18:34:01Z

Great cleanup 👏 let's address above comment and get this merged.

Trying in the following days...

dprevost-LMI · 2025-06-10T00:02:37Z

I will open another PR since I'm getting some challenges, so that we can merge the fixes! I reverted the proposition from the PR!

christian-bromann

LGTM 👍

wdio-bot · 2025-06-10T00:35:29Z

Hey dprevost-LMI 👋

Thank you for your contribution to WebdriverIO! Your pull request has been marked as an "Expensable" contribution.

We've sent you an email with further instructions on how to claim your expenses from our development fund.
Please make sure to check your spam folder as well. If you have any questions, feel free to reach out to us at expense@webdriver.io or in the contributing channel on Discord.

We are looking forward to more contributions from you in the future 🙌

Have a nice day,
The WebdriverIO Team 🤖

dprevost-LMI · 2025-06-14T23:07:57Z

Propose overwriteBrowserCommand and overwriteElementCommand, instead of using the boolean, to ease the API and not worry about missing the boolean or not. Will also reduce the typing of definitions
Propose addBrowserCommand and addElementCommand, instead of using the boolean, to ease the API and not worry about missing the boolean or not. Will also reduce the typing of definitions
I think this makes sense to me. We should deprecate the overwriteCommand then so we can eventually remove it.

About the information above, I gave it a try, but I have some doubts, unfortunately.

addCommand is used much more internally than I initially thought.
After seeing more code, it made more sense to use browser.addCommand when not an element than browser.AddBrowserCommand.
I found this code where addCommand is used on an element, which I did not expect and discovered the API also applies to an element. Therefore, having element.addBrowserCommand there makes even less sense.

What would make more sense:

Separate the addCommand API on the browser/driver vs the one on the element
2. On the element, we do not need the attachToElement, proto and instances
3. This way, when adding addElementCommand, it would also not be added by default to the element API
Keep addCommand but deprecated only attachToElement, which would be replaced by driver.addElementCommand

So, as a prototyping start, it would look like the below (utopian thinking since we are currently forgetting for a minute that we need to support a deprecated API)

type BrowserTypeInstances = WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser
export interface BrowserInstanceCommands<T extends BrowserTypeInstances> {
    /**
     * Add a command to `browser`
     */
    addCommand (
        name: string,
        func: AddCommandFn,
        proto?: Record<string, any>,
        instances?: Record<string, T>
    ): void;

    /**
     * Add a command to elements
     */
    addElementCommand<Instance extends BrowserTypeInstances = WebdriverIO.Browser>(
        name: string,
        func: AddCommandFnScoped<T | Instance, true>,
        // Unsure if needed or not, maybe just internally needed if so we could separate from public API
        // proto?: Record<string, any>,
        // instances?: Record<string, Instances>
    ): void;

    /**
     * Overwrite a `browser` command
     */
    overwriteCommand<ElementKey extends keyof $ElementCommands, BrowserKey extends keyof $BrowserCommands>(
        name:  BrowserKey,
        func: OverwriteCommandFn<ElementKey, BrowserKey, false>,
        proto?: Record<string, any>,
        instances?: Record<string, BrowserTypeInstances>
    ): void;

    /**
     * Overwrite an element command
     */
    overwriteElementCommand<ElementKey extends keyof $ElementCommands, BrowserKey extends keyof $BrowserCommands>(
        name:  BrowserKey,
        func: OverwriteCommandFnScoped<ElementKey, BrowserKey, true>,
        // Unsure if needed or not, maybe just internally needed if so we could separate from public API
        // proto?: Record<string, any>,
        // instances?: Record<string, BrowserTypeInstances>
    ): void;

    /**
     * Create a custom selector
     */
    addLocatorStrategy(
        name: string,
        func: ((selector: string, root: HTMLElement) => CustomLocatorReturnValue) | ((selector: string) => CustomLocatorReturnValue)
    ): void
}

export interface ElementInstanceCommands<T extends WebdriverIO.Element> {
    /**
     * add command to `browser` or `element` scope
     */
    addCommand<IsElement extends boolean = false, Instance extends BrowserTypeInstances = WebdriverIO.Browser>(
        name: string,
        func: AddCommandFnScoped<T | Instance, IsElement>,
    ): void;

    // Needed?? Probably not...
    // overwriteCommand(): void;
    // addLocatorStrategy(): void
}

Seeing the complexity, either I just won't do it, or from now on, it will need its dedicated issue, so we can stamp all this properly instead of hijacking this issue 😅

christian-bromann · 2025-06-16T16:03:56Z

@dprevost-LMI thanks for the explanation.

I agree with you. addCommand historically was implemented way before we had different scopes, e.g. browser and element. I hope soon I will finish my work on the new page scope which would mean we have even more scopes.

I have two options in my mind:

only have addCommand and overwriteCommand but apply them on the scope based on which prototype you call it on, e.g. browser.addCommand(...) adds a browser command while $(elem).addCommand(...) adds a command to the element scope - the disadvantage here is that I need an element instance first to modify its prototype which seems less ideal
have dedicated commands for each scope, e.g. addBrowserCommand, addElementCommand or addPageCommand but that seems verbose and requires to add a new command every time we add a new scope?

Let's create a new issue and get more feedback from the community.

dprevost-LMI added 6 commits June 8, 2025 09:13

Fix the debug return type

72b4635

Fix typings

49b8468

Fix UTs

dfdccad

Have more verbose and type safe examples

b8363d4

fix the type reference of the this argument + add UTs

955e07d

dprevost-LMI changed the title ~~fix(types): Fix overwrite command bad typing~~ fix(types): Fix overwriteCommand bad typing Jun 8, 2025

Even better example

5173512

dprevost-LMI mentioned this pull request Jun 8, 2025

[🐛 Bug]: Overwrite command's original function type problem #14548

Closed

3 tasks

More type fixes + more robust UTs

a446477

Simplification of code in index.ts

f578ebb

dprevost-LMI commented Jun 8, 2025

View reviewed changes

packages/webdriverio/src/types.ts Outdated Show resolved Hide resolved

Add this infer type for browser overwritten command + final review

09bc3a8

dprevost-LMI commented Jun 8, 2025

View reviewed changes

packages/webdriverio/src/types.ts Show resolved Hide resolved

dprevost-LMI marked this pull request as ready for review June 8, 2025 19:27

dprevost-LMI changed the title ~~fix(types): Fix overwriteCommand bad typing~~ fix(types): Fix overwriteCommand inconsistent typing Jun 8, 2025

christian-bromann reviewed Jun 9, 2025

View reviewed changes

Remove proposition to revisite later + fix typing issue

655a3d6

christian-bromann approved these changes Jun 10, 2025

View reviewed changes

christian-bromann added the PR: Bug Fix 🐛 PRs that contain bug fixes label Jun 10, 2025

christian-bromann merged commit ce33265 into webdriverio:main Jun 10, 2025
79 of 81 checks passed

wdio-bot added the Expensable $50 💸 label Jun 10, 2025

yashuatla mentioned this pull request Jun 24, 2025

Feature implementation from commits ba8abf1..ce33265 yashuatla/webdriverio#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(types): Fix `overwriteCommand` inconsistent typing#14549

fix(types): Fix `overwriteCommand` inconsistent typing#14549
christian-bromann merged 11 commits intowebdriverio:mainfrom
dprevost-LMI:fix-overwriteCommand-bad-typing

dprevost-LMI commented Jun 8, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Jun 8, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

christian-bromann commented Jun 9, 2025

Uh oh!

christian-bromann left a comment

Uh oh!

dprevost-LMI commented Jun 9, 2025

Uh oh!

dprevost-LMI commented Jun 10, 2025

Uh oh!

christian-bromann left a comment

Uh oh!

Uh oh!

wdio-bot commented Jun 10, 2025

Uh oh!

dprevost-LMI commented Jun 14, 2025 •

edited

Loading

Uh oh!

christian-bromann commented Jun 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dprevost-LMI commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Types of changes

Checklist

Backport Request

Further comments

Reviewers: @webdriverio/project-committers

Uh oh!

pkg-pr-new bot commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christian-bromann commented Jun 9, 2025

Uh oh!

christian-bromann left a comment

Choose a reason for hiding this comment

Uh oh!

dprevost-LMI commented Jun 9, 2025

Uh oh!

dprevost-LMI commented Jun 10, 2025

Uh oh!

christian-bromann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wdio-bot commented Jun 10, 2025

Uh oh!

dprevost-LMI commented Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christian-bromann commented Jun 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dprevost-LMI commented Jun 8, 2025 •

edited

Loading

pkg-pr-new bot commented Jun 8, 2025 •

edited

Loading

dprevost-LMI commented Jun 14, 2025 •

edited

Loading