timeout argment to Locator.isVisible, or make the deprecation more obvious to the user #33017Description
🚀 Feature Request
the argument has been deprecated for a few years now, and the current behavior where it just silently ignores the argument is even more confusing than the previous behavior.
Example
there is no visible deprecation warning on code that uses it, which makes it very easy to miss for someone who doesn't already know it's deprecated:
this could be addressed by turning the method into an overload:
// playwright-core/types/types.d.ts
export interface Locator {
/**
* @deprecated the `timeout` option is ignored.
* @param options
*/
isVisible(options: {
timeout: number;
}): Promise<boolean>;
isVisible(): Promise<boolean>;
}which will make vscode display a deprecation hint:
this would also need to be done for all the languages (eg. python using the @deprecated decorator), which i assume would require updates to the code generation scripts. it's probably not worth the effort and imo it's probably time to just delete the argument entirely since it's been several years since it was deprecated.
in python, the deprecation notice is even easier to miss as it's in the middle of a large docstring:
def is_visible(self, *, timeout: typing.Optional[float] = None) -> bool:
"""Locator.is_visible
Returns whether the element is [visible](https://playwright.dev/python/docs/actionability#visible).
**NOTE** If you need to assert that element is visible, prefer `locator_assertions.to_be_visible()` to avoid
flakiness. See [assertions guide](https://playwright.dev/python/docs/test-assertions)
6698
for more details.
**Usage**
```py
visible = page.get_by_role(\"button\").is_visible()
```
Parameters
----------
timeout : Union[float, None]
Deprecated: This option is ignored. `locator.is_visible()` does not wait for the element to become visible and returns immediately.
Returns
-------
bool
"""
return mapping.from_maybe_impl(
self._sync(self._impl_obj.is_visible(timeout=timeout))
)especially considering that so many functions take a timeout argument, it's very unlikely that the user is going to look there for a description of what that argument does
Motivation
i just reviewed someone's pull request where they used hardcoded page.wait_for_timeout calls all over the place because they were relying on locator.is_visible with a timeout, didn't realize it was silently being ignored and assumed that adding an additional wait beforehand was necessary.
i think silently ignoring an argument is a bad way to deprecate something, because it renders code that uses it completely incorrect with no error or warning to the user. if you ask me, this is even worse than just deleting the argument out of the blue with no deprecation phase at all.
IMO when it comes to deprecations, the thing being deprecated should at least still function until its eventual removal, or at the very least provide an IDE or type checker warning to the user.