-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Proposal: deprecate waitFor
and add waitForTimeout
#6214
Comments
waitFor
and add waitForTime
waitFor
and add waitForTimer
LGTM. Explicit, dedicated APIs |
LGTM |
Please don't remove |
@supnate |
Thanks @jackfranklin for the suggestion. But you know people don't care warnings much. We can ask teams in different countries to do it but there will be no mechanism to ensure all code are updated before it's finally removed unless we write some script to check it. We've been using puppeteer very smoothly from 1.x to 5.x and we wish it could continue. like you can add a new API but just remove |
BTW, would you please update the issue title using |
Does someone know what the future of the TypeScript Types are? with currently if anyone is using TypeScript they can add the following to extend the definitions import * as puppeteer from 'puppeteer';
declare module 'puppeteer' {
export interface Page {
waitForTimeout(duration: number): Promise<void>;
}
} |
and what about the usage of waitForX is that still same as waitFor original parameter? |
Yeah, I was confused being told it's deprecated but also the only way currently to wait for a specific amount of time is to use waitFor(1000). Hope either an alternate function is created or waitFor() becomes the time exclusive version. |
`waitFor` is deprecated and will be removed in a future release. See puppeteer/puppeteer/issues/6214
`waitFor` is deprecated and will be removed in a future release. See puppeteer/puppeteer/issues/6214
Resolves #785 See puppeteer/puppeteer#6214
You can also wait for a specific time outside the
|
- page.waitFor -> page.waitForTimeout and page.waitForSelector (Got deprication notice. See puppeteer/puppeteer#6214.) - remove removeEventListener (we'd need to change it to removeListener anyway - v4.0.0 and see https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#eventemitterremovelistenerevent-handler). For now we'll count on page close taking care of it, just in case removing it would prevent multiple-file-downloads.
Take a look at #3347 // this test fails with a time out
await page.waitForSelector(buttonSelector)
await page.click(buttonSelector)
// this one passes
await page.waitForSelector(buttonSelector)
await page.waitFor(1000)
await page.click(buttonSelector)
// so does this one
await page.waitForSelector(buttonSelector)
page.$eval(buttonSelector, elem => elem.click()) So if you remove EDIT: wrong copy/paste |
hey, started getting warnings about deprecation. |
Looks like its |
|
If you want to wait for a number of milliseconds to pass, use Please see the documentation for these methods: https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#pagewaitfortimeoutmilliseconds and make sure you use a version that ships |
waitFor
and add waitForTimer
waitFor
and add waitForTimeout
waitFor
and add waitForTimeout
waitFor
and add waitForTimeout
I'm thinking this is just going to be for v4. Not bothering with this for v3 unless we absolutely have to since none of the vulnerabilities are severe. My current rationale is that the more work we do to maintain 3, the less work we can do getting v4 ready for release. Ready to hear opinions. - Close #164, update cucumber to v7 - Prepare for v8 of cucumber because I won't remember it later - Close #394, update puppeteer - Update our version of node (and that of our action that we'll run for other people's libs). [Addresses #393 so we can use the suffolk npm org package.] - Use `npm audit` to fix the remaining vulnerabilities (now 0) - [Remove package.json as discussed in #489 to align our tests' behaviors with those of our users.] * Update action.yml node to v17 * Update from cucumber v6 to v7. See details. See https://github.com/cucumber/cucumber-js/blob/main/docs/migration.md#migrating-to-cucumber-js-7xx Only use cucumber setDefaultTimeout globally and use a shim that replicates the fix in v8 that lets you do custom timeouts more easily so we can still give enough time for steps that may need more time. Use all caps for statuses. Test screenshot step. Btw, the cucumber test output visually looks a bit different now - when a scenario passes, all the steps pass too. Sorry about the little comment changes, etc. Tried to remove a lot of those incidental unrelated changes. * Update puppeteer to latest (13). See details below. - page.waitFor -> page.waitForTimeout and page.waitForSelector (Got deprication notice. See puppeteer/puppeteer#6214.) - remove removeEventListener (we'd need to change it to removeListener anyway - v4.0.0 and see https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#eventemitterremovelistenerevent-handler). For now we'll count on page close taking care of it, just in case removing it would prevent multiple-file-downloads. * Update GitHub worflow node version, tweak changelog item order * Fix npm audit vulnerabilities and update action.yml cucumber * Pin the colors lib in action.yml * Remove package-lock.json #489, use kiln v4 for users, CHANGELOG * Fix custom timeout, remove duplicate report entry, as per review
Created log.txt and git hub artifact for reports. Closes #466. * add log.txt for report messages * create artifact for logs * Update changelog * Update our package's dependencies for v4 (#503) I'm thinking this is just going to be for v4. Not bothering with this for v3 unless we absolutely have to since none of the vulnerabilities are severe. My current rationale is that the more work we do to maintain 3, the less work we can do getting v4 ready for release. Ready to hear opinions. - Close #164, update cucumber to v7 - Prepare for v8 of cucumber because I won't remember it later - Close #394, update puppeteer - Update our version of node (and that of our action that we'll run for other people's libs). [Addresses #393 so we can use the suffolk npm org package.] - Use `npm audit` to fix the remaining vulnerabilities (now 0) - [Remove package.json as discussed in #489 to align our tests' behaviors with those of our users.] * Update action.yml node to v17 * Update from cucumber v6 to v7. See details. See https://github.com/cucumber/cucumber-js/blob/main/docs/migration.md#migrating-to-cucumber-js-7xx Only use cucumber setDefaultTimeout globally and use a shim that replicates the fix in v8 that lets you do custom timeouts more easily so we can still give enough time for steps that may need more time. Use all caps for statuses. Test screenshot step. Btw, the cucumber test output visually looks a bit different now - when a scenario passes, all the steps pass too. Sorry about the little comment changes, etc. Tried to remove a lot of those incidental unrelated changes. * Update puppeteer to latest (13). See details below. - page.waitFor -> page.waitForTimeout and page.waitForSelector (Got deprication notice. See puppeteer/puppeteer#6214.) - remove removeEventListener (we'd need to change it to removeListener anyway - v4.0.0 and see https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#eventemitterremovelistenerevent-handler). For now we'll count on page close taking care of it, just in case removing it would prevent multiple-file-downloads. * Update GitHub worflow node version, tweak changelog item order * Fix npm audit vulnerabilities and update action.yml cucumber * Pin the colors lib in action.yml * Remove package-lock.json #489, use kiln v4 for users, CHANGELOG * Fix custom timeout, remove duplicate report entry, as per review * Allow a developer to wait as a first Step v4. #387. Add test. (#506) Closes #387. Also, generally adds safety measures for when page does not exist. Very similar to PR #459, but moving the responsibility down to a spot that most functions make use of, meaning that it'll be applied to a lot more cases. They mostly won't need it, but it might still be worth being more comprehensive. * Allow a developer to wait as a first Step v4. #387. Add test. Will be able to close once we've added this as an establishing step (in addition to it being a regular step). Also, generally adds safety measures for when page does not exist. * Add test Co-authored-by: Bryce Willey <[email protected]> Co-authored-by: Bryce Willey <[email protected]> * add log to gitnore and cleanup console.logs and typos * add empty string to file Co-authored-by: plocket <[email protected]> Co-authored-by: Bryce Willey <[email protected]>
1. Updates puppeteer to x 2. Fixes deprecations: ``` waitFor is deprecated and will be removed in a future release. See puppeteer/puppeteer#6214 for details and how to migrate your code. ``` 3. Lints/prettyfies the smoke_test.js file
1. Updates puppeteer to x 2. Fixes deprecations: ``` waitFor is deprecated and will be removed in a future release. See puppeteer/puppeteer#6214 for details and how to migrate your code. ``` 3. Lints/prettyfies the smoke_test.js file
ref: puppeteer/puppeteer#6214 Signed-off-by: Outsider <[email protected]>
ref: puppeteer/puppeteer#6214 Signed-off-by: Outsider <[email protected]>
ref: puppeteer/puppeteer#6214 Signed-off-by: Outsider <[email protected]>
- Install percy as a development dependency instead of installing it as a separate build step. - [Replace deprecated `waitFor` with `waitForTimeout`][1]. - Explicitly add `--theme .` argument to the `npx resume` command. [1]: puppeteer/puppeteer#6214
- Install percy as a development dependency instead of installing it as a separate build step. - [Replace deprecated `waitFor` with `waitForTimeout`][1]. - Explicitly add `--theme .` argument to the `npx resume` command. [1]: puppeteer/puppeteer#6214
TypeError: page.waitForTimeout is not a function TypeError: page.waitFor is not a This is on latest 22.3.0 How annoying. What harm is there in keeping older function names, to prevent projects from being nuked on updates? |
|
In case anyone else finds this issue wondering why neither |
Puppeteer's API currently has a few
waitForX
functions:waitForSelector
waitForFunction
waitForXPath
(And others that are less relevant to this proposal).
We also ship
waitFor
, which has the following docs:So if you call
waitFor(someFunc)
, you're just callingwaitForFunction
. Similarly,waitFor('.foo')
callswaitForSelector
, andwaitFor('//div')
callswaitForXPath
. The only unique behaviourwaitFor
provides is calling it with a time:waitFor(1000)
does exactly what you expect.Another downside of this method is that it's harder to type safely with TypeScript (you can do it via overloads, but it's uneccessarily complex, especially when all the standalone
waitForX
functions are already typed).I think it's confusing that most of the API for waiting is explicitly named, e.g.
waitForSelector
, yet the only way to wait for a timeout is to callwaitFor
.I'd propose a nicer, more consistent API would:
waitForTimeout
. This is consistent with the naming of the other wait for methods, which are all namedwaitForX
.waitFor
.Rather than do this as one breaking change we can do this change in two parts:
waitForTimeout
. DeprecatewaitFor
and log a message when it's used, but maintain its current behaviour.waitFor
. The migration path should be straightforward, users will have to swapwaitFor
for the correctwaitForX
method. We'll have awaitForX
for every behaviour of the originalwaitFor
, so this won't be more than a renaming of some method calls.The text was updated successfully, but these errors were encountered: